Support dot notation for :only keys in partial reloads #163
Conversation
lib/inertia_rails/renderer.rb
Outdated
|
|
||
| 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.
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 )
lib/inertia_rails/renderer.rb
Outdated
|
|
||
| hash.transform_values {|value| deep_transform_values(value, &block)} | ||
| end | ||
| def deep_transform_props(props, parent_path = '', &block) |
There was a problem hiding this comment.
Similar logic as the previous method, but we're now also dealing with the results of the filtering within the hash we're building up and conditionally setting keys.
| @@ -138,5 +134,35 @@ 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.
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.
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.
lib/inertia_rails/renderer.rb
Outdated
| end | ||
|
|
||
| def excluded_by_except_partial_keys?(path_with_prefixes) | ||
| partial_except_keys.present? && (path_with_prefixes & partial_except_keys.map(&:to_s)).any? |
There was a problem hiding this comment.
I think we can replace .filter_map(&:to_sym) in the partial_except_keys and then stop calling .map(&:to_s) here
And same for partial_keys
| partial_except_keys.present? && (path_with_prefixes & partial_except_keys.map(&:to_s)).any? | |
| partial_except_keys.present? && (path_with_prefixes & partial_except_keys).any? |
lib/inertia_rails/renderer.rb
Outdated
| 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) |
There was a problem hiding this comment.
I noticed that 'what_about_empty_hash' => {}, example will fail, here's a fix:
| if prop.is_a?(Hash) | |
| if prop.is_a?(Hash) && prop.any? |
lib/inertia_rails/renderer.rb
Outdated
| end | ||
| def deep_transform_props(props, parent_path = '', &block) | ||
| props.reduce({}) do |transformed_props, (key, prop)| | ||
| current_path = [parent_path, key].reject(&:empty?).join('.') |
There was a problem hiding this comment.
we can simplify this by sticking with arrays:
| current_path = [parent_path, key].reject(&:empty?).join('.') | |
| current_path = path + [key] |
Deploying inertia-rails with
|
| Latest commit: |
c4606ef
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://8d1fa703.inertia-rails.pages.dev |
| Branch Preview URL: | https://dot-notation-for-only-props.inertia-rails.pages.dev |
lib/inertia_rails/renderer.rb
Outdated
|
|
||
| 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.
nitpick: could we move these symbol values to constants since they're acting as an enum?
There was a problem hiding this comment.
sure! as an aside, :dont_keep doesn't do anything at all... it's just there to reveal intention
Resolves #157
I also refactored the rendering logic a bit, to:
Note: the renderer diff isn't very helpful; better to view the entire file to see how it looks now.
TODO: