-
Notifications
You must be signed in to change notification settings - Fork 48
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
Support dot notation for :only keys in partial reloads #163
Changes from 8 commits
a42ea77
c5653d2
af8fb91
776da3a
a62b21a
c6df93d
0c566ee
c4606ef
2027984
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -74,25 +74,21 @@ def merge_props(shared_data, props) | |
end | ||
|
||
def computed_props | ||
_props = merge_props(shared_data, props).select do |key, prop| | ||
if rendering_partial_component? | ||
partial_keys.none? || key.in?(partial_keys) || prop.is_a?(AlwaysProp) | ||
else | ||
!prop.is_a?(LazyProp) | ||
end | ||
end | ||
_props = merge_props(shared_data, props) | ||
|
||
drop_partial_except_keys(_props) if rendering_partial_component? | ||
deep_transform_props _props do |prop, path| | ||
next [:dont_keep] unless keep_prop?(prop, path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. nitpick: could we move these symbol values to constants since they're acting as an enum? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. sure! as an aside, |
||
|
||
deep_transform_values _props do |prop| | ||
case prop | ||
transformed_prop = case prop | ||
when BaseProp | ||
prop.call(controller) | ||
when Proc | ||
controller.instance_exec(&prop) | ||
else | ||
prop | ||
end | ||
|
||
[:keep, transformed_prop] | ||
end | ||
end | ||
|
||
|
@@ -105,28 +101,28 @@ def page | |
} | ||
end | ||
|
||
def deep_transform_values(hash, &block) | ||
return block.call(hash) unless hash.is_a? Hash | ||
|
||
hash.transform_values {|value| deep_transform_values(value, &block)} | ||
end | ||
def deep_transform_props(props, parent_path = [], &block) | ||
props.reduce({}) do |transformed_props, (key, prop)| | ||
current_path = parent_path + [key] | ||
|
||
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 | ||
current = initial_keys.any? ? hash.dig(*initial_keys) : hash | ||
if prop.is_a?(Hash) && prop.any? | ||
nested = deep_transform_props(prop, current_path, &block) | ||
transformed_props.merge!(key => nested) unless nested.empty? | ||
else | ||
action, transformed_prop = block.call(prop, current_path) | ||
transformed_props.merge!(key => transformed_prop) if action == :keep | ||
end | ||
|
||
current.delete(last_key) if current.is_a?(Hash) && !current[last_key].is_a?(AlwaysProp) | ||
transformed_props | ||
end | ||
end | ||
|
||
def partial_keys | ||
(@request.headers['X-Inertia-Partial-Data'] || '').split(',').compact.map(&:to_sym) | ||
(@request.headers['X-Inertia-Partial-Data'] || '').split(',').compact | ||
end | ||
|
||
def partial_except_keys | ||
(@request.headers['X-Inertia-Partial-Except'] || '').split(',').filter_map(&:to_sym) | ||
(@request.headers['X-Inertia-Partial-Except'] || '').split(',').compact | ||
end | ||
|
||
def rendering_partial_component? | ||
|
@@ -138,5 +134,34 @@ def resolve_component(component) | |
|
||
configuration.component_path_resolver(path: controller.controller_path, action: controller.action_name) | ||
end | ||
|
||
def keep_prop?(prop, path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. all the prop filtering logic now lives in one concise place. i debated pulling this out into a class, but I don't think the cleanliness is worth the extra indirection. |
||
return true if prop.is_a?(AlwaysProp) | ||
|
||
if rendering_partial_component? | ||
path_with_prefixes = path_prefixes(path) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Maybe worth nothing that if we're in a partial reload, we'll end up calculating this array for every single prop key. That should still be way faster than fully evaluating all of those props, though. |
||
return false if excluded_by_only_partial_keys?(path_with_prefixes) | ||
return false if excluded_by_except_partial_keys?(path_with_prefixes) | ||
end | ||
|
||
# Precedence: Evaluate LazyProp only after partial keys have been checked | ||
return false if prop.is_a?(LazyProp) && !rendering_partial_component? | ||
|
||
true | ||
end | ||
|
||
def path_prefixes(parts) | ||
(0...parts.length).map do |i| | ||
parts[0..i].join('.') | ||
end | ||
end | ||
|
||
def excluded_by_only_partial_keys?(path_with_prefixes) | ||
partial_keys.present? && (path_with_prefixes & partial_keys).empty? | ||
end | ||
|
||
def excluded_by_except_partial_keys?(path_with_prefixes) | ||
partial_except_keys.present? && (path_with_prefixes & partial_except_keys).any? | ||
end | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why return this array from the block now? Well, the block has two jobs 1) to filter out props we don't need and 2) transform the keepers into their final format. We want track those separately so that the transformation can return things that are nil/false-y. (i think the typical pattern here would be
next nil unless some_condition?
)(This block will also be a nice place to rewrite the logic from #154 )