Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for always and except #152

Merged
merged 8 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 6 additions & 10 deletions docs/guide/partial-reloads.md
Original file line number Diff line number Diff line change
Expand Up @@ -46,9 +46,6 @@ router.visit(url, {

## Except certain props

> [!WARNING]
> The `except` option is not yet supported by the Inertia Rails.

:::tabs key:frameworks
== Vue

Expand Down Expand Up @@ -204,13 +201,7 @@ On the inverse, you can use the `InertiaRails.always` method to specify that a p
class UsersController < ApplicationController
def index
render inertia: 'Users/Index', props: {
users: InertiaRails.always(User.all),

# Also works with block:
# users: InertiaRails.always { User.all },

# Also works with a lambda:
# users: InertiaRails.always(-> { User.all }),
users: InertiaRails.always { User.all },
}
end
end
Expand All @@ -236,6 +227,11 @@ class UsersController < ApplicationController
# OPTIONALLY included on partial reloads
# ONLY evaluated when needed
users: InertiaRails.lazy { User.all },

# ALWAYS included on standard visits
# ALWAYS included on partial reloads
# ALWAYS evaluated
users: InertiaRails.always(User.all),
}
end
end
Expand Down
6 changes: 6 additions & 0 deletions lib/inertia_rails/always_prop.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
# frozen_string_literal: true

module InertiaRails
class AlwaysProp < BaseProp
end
end
14 changes: 14 additions & 0 deletions lib/inertia_rails/base_prop.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# frozen_string_literal: true

module InertiaRails
# Base class for all props.
class BaseProp
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this. Half-baked thought: I wonder if we could move more rendering logic into the Prop classes. The computed_props method is doing more and more things in a sort of procedural style. Maybe it could iterate over the props one time and each instance would know what to do with itself.

(Not suggesting changes, more just thinking out loud)

def initialize(&block)
@block = block
end

def call(controller)
controller.instance_exec(&@block)
end
end
end
30 changes: 18 additions & 12 deletions lib/inertia_rails/inertia_rails.rb
Original file line number Diff line number Diff line change
@@ -1,20 +1,26 @@
# Needed for `thread_mattr_accessor`
require 'active_support/core_ext/module/attribute_accessors_per_thread'
require 'inertia_rails/lazy'
require 'inertia_rails/base_prop'
require 'inertia_rails/always_prop'
require 'inertia_rails/lazy_prop'
require 'inertia_rails/configuration'

module InertiaRails
CONFIGURATION = Configuration.default
class << self
CONFIGURATION = Configuration.default

def self.configure
yield(CONFIGURATION)
end
def configure
yield(CONFIGURATION)
end

def self.configuration
CONFIGURATION
end
def configuration
CONFIGURATION
end

def lazy(value = nil, &block)
LazyProp.new(value, &block)
end

def self.lazy(value = nil, &block)
InertiaRails::Lazy.new(value, &block)
def always(&block)
AlwaysProp.new(&block)
end
end
end
28 changes: 0 additions & 28 deletions lib/inertia_rails/lazy.rb

This file was deleted.

20 changes: 20 additions & 0 deletions lib/inertia_rails/lazy_prop.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
# frozen_string_literal: true

module InertiaRails
class LazyProp < BaseProp
def initialize(value = nil, &block)
raise ArgumentError, 'You must provide either a value or a block, not both' if value && block

@value = value
@block = block
end

def call(controller)
value.respond_to?(:call) ? controller.instance_exec(&value) : value
end

def value
@value.nil? ? @block : @value
end
end
end
50 changes: 38 additions & 12 deletions lib/inertia_rails/renderer.rb
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
# frozen_string_literal: true

require 'net/http'
require 'json'
require_relative "inertia_rails"
Expand Down Expand Up @@ -64,24 +66,34 @@ def shared_data
# Functionally, this permits using either string or symbol keys in the controller. Since the results
# is cast to json, we should treat string/symbol keys as identical.
def merge_props(shared_data, props)
shared_data.deep_symbolize_keys.send(@deep_merge ? :deep_merge : :merge, props.deep_symbolize_keys)
if @deep_merge
shared_data.deep_symbolize_keys.deep_merge!(props.deep_symbolize_keys)
else
shared_data.symbolize_keys.merge(props.symbolize_keys)
end
end

def computed_props
_props = merge_props(shared_data, props).select do |key, prop|
if rendering_partial_component?
key.in? partial_keys
partial_keys.none? || key.in?(partial_keys) || prop.is_a?(AlwaysProp)
else
!prop.is_a?(InertiaRails::Lazy)
!prop.is_a?(LazyProp)
end
end

deep_transform_values(
_props,
lambda do |prop|
prop.respond_to?(:call) ? controller.instance_exec(&prop) : prop
drop_partial_except_keys(_props) if rendering_partial_component?
bknoles marked this conversation as resolved.
Show resolved Hide resolved

deep_transform_values _props do |prop|
case prop
when BaseProp
prop.call(controller)
when Proc
controller.instance_exec(&prop)
else
prop
end
)
end
end

def page
Expand All @@ -93,18 +105,32 @@ def page
}
end

def deep_transform_values(hash, proc)
return proc.call(hash) unless hash.is_a? Hash
def deep_transform_values(hash, &block)
return block.call(hash) unless hash.is_a? Hash

hash.transform_values {|value| deep_transform_values(value, proc)}
hash.transform_values {|value| deep_transform_values(value, &block)}
end

def drop_partial_except_keys(hash)
partial_except_keys.each do |key|
parts = key.to_s.split('.').map(&:to_sym)
*initial_keys, last_key = parts
Copy link
Collaborator

@bknoles bknoles Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this supports dot notation, yea? so like except: ['users.some_association.expensive_thing_to_compute']? [edit: I see the dot notation in the specs! question below remains]

do the other adapters support that? I don't see it documented anywhere.

either way, if we support it for :except, should we also support it for :only? (genuine question)

Copy link
Contributor Author

@skryukov skryukov Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do the other adapters support that? I don't see it documented anywhere.

yup, Laravel adapter supports that

if we support it for :except, should we also support it for :only?

Laravel adapter does not 😄 To be honest the whole except + only thing is poorly documented, we probably should help the core team make it happen.

Copy link
Contributor Author

@skryukov skryukov Nov 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually they reverted back dot notation for only: inertiajs/inertia-laravel#641

While this would be a nice feature to eventually have, this particular implementation introduced some breaking changes.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh nice find!

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, i dug into this a bit. TL;DR: I don't think those issues will affect our implementation.

Best I can tell (caveat: using my brain's PHP interpreter) is that the original implementation of Laravel's share shallow merged all data. However, dot notation allowed you to deep merge dot-notated shared data. Not sure whether that was intentional or a happy accident since I don't think the feature was ever documented. When dot notation was added to only and except, it changed the execution order and changed the behavior such that all shared data was a shallow merge. So, people relying on the deep merge behavior had breaking changes. (There was another reported but with lazy props that I didn't fully diagnose... looks to be something related to array manipulation quirks in PHP).

InertiaRails 1) has explicit patterns for deep merging shared data and 2) doesn't support dot notation in shared data anyway, so we should be totally fine.

I don't see a reason why we couldn't support dot notation for only as well. Do you think it's worth adding? Seems like it'd be nice if some prop had multiple expensive child props and you only wanted to grab one of them.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, let's try adding support for ’only’ in a separate PR 👍

current = initial_keys.any? ? hash.dig(*initial_keys) : hash

current.delete(last_key) if current.is_a?(Hash) && !current[last_key].is_a?(AlwaysProp)
end
end

def partial_keys
(@request.headers['X-Inertia-Partial-Data'] || '').split(',').compact.map(&:to_sym)
end

def partial_except_keys
(@request.headers['X-Inertia-Partial-Except'] || '').split(',').filter_map(&:to_sym)
end

def rendering_partial_component?
@request.inertia_partial? && @request.headers['X-Inertia-Partial-Component'] == component
@request.headers['X-Inertia-Partial-Component'] == component
end

def resolve_component(component)
Expand Down
2 changes: 1 addition & 1 deletion lib/patches/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,6 @@ def inertia?
end

def inertia_partial?
key? 'HTTP_X_INERTIA_PARTIAL_DATA'
key?('HTTP_X_INERTIA_PARTIAL_COMPONENT')
end
end
3 changes: 3 additions & 0 deletions spec/dummy/app/controllers/application_controller.rb
Original file line number Diff line number Diff line change
@@ -1,2 +1,5 @@
class ApplicationController < ActionController::Base
def controller_method
'controller_method value'
end
end
28 changes: 28 additions & 0 deletions spec/dummy/app/controllers/inertia_render_test_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,23 @@ def props
}
end

def except_props
render inertia: 'TestComponent', props: {
flat: 'flat param',
lazy: InertiaRails.lazy('lazy param'),
nested_lazy: InertiaRails.lazy do
{
first: 'first nested lazy param',
}
end,
nested: {
first: 'first nested param',
second: 'second nested param'
},
always: InertiaRails.always { 'always prop' }
}
end

def view_data
render inertia: 'TestComponent', view_data: {
name: 'Brian',
Expand Down Expand Up @@ -34,4 +51,15 @@ def lazy_props
grit: InertiaRails.lazy(->{ 'intense' })
}
end

def always_props
render inertia: 'TestComponent', props: {
always: InertiaRails.always { 'always prop' },
regular: 'regular prop',
lazy: InertiaRails.lazy do
'lazy prop'
end,
another_lazy: InertiaRails.lazy(->{ 'another lazy prop' })
}
end
end
2 changes: 2 additions & 0 deletions spec/dummy/config/routes.rb
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,8 @@
get 'error_500' => 'inertia_test#error_500'
get 'content_type_test' => 'inertia_test#content_type_test'
get 'lazy_props' => 'inertia_render_test#lazy_props'
get 'always_props' => 'inertia_render_test#always_props'
get 'except_props' => 'inertia_render_test#except_props'
get 'non_inertiafied' => 'inertia_test#non_inertiafied'

get 'instance_props_test' => 'inertia_rails_mimic#instance_props_test'
Expand Down
3 changes: 3 additions & 0 deletions spec/inertia/always_prop_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
RSpec.describe InertiaRails::AlwaysProp do
it_behaves_like 'base prop'
end
3 changes: 3 additions & 0 deletions spec/inertia/base_prop_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
RSpec.describe InertiaRails::BaseProp do
it_behaves_like 'base prop'
end
35 changes: 35 additions & 0 deletions spec/inertia/lazy_prop_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
RSpec.describe InertiaRails::LazyProp do
it_behaves_like 'base prop'

describe '#call' do
subject(:call) { prop.call(controller) }
let(:prop) { described_class.new('value') }
let(:controller) { ApplicationController.new }

it { is_expected.to eq('value') }

context 'with false as value' do
let(:prop) { described_class.new(false) }

it { is_expected.to eq(false) }
end

context 'with nil as value' do
let(:prop) { described_class.new(nil) }

it { is_expected.to eq(nil) }
end

context 'with a callable value' do
let(:prop) { described_class.new(-> { 'callable' }) }

it { is_expected.to eq('callable') }

context 'with dependency on the context of a controller' do
let(:prop) { described_class.new(-> { controller_method }) }

it { is_expected.to eq('controller_method value') }
end
end
end
end
21 changes: 0 additions & 21 deletions spec/inertia/lazy_spec.rb

This file was deleted.

Loading