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

feat: Partial Except #1

Conversation

PedroAugustoRamalhoDuarte
Copy link

@PedroAugustoRamalhoDuarte PedroAugustoRamalhoDuarte commented Oct 12, 2024

This PR introduces support for the X-Inertia-Partial-Except header to the Inertia Rails Renderer. This allows developers to exclude specific props from partial updates when using the Inertia.

Key Changes:

  • Added logic to handle X-Inertia-Partial-Except within the computed_props method.
  • Implemented new method partial_except_keys to retrieve the excluded props from the request headers and ensure these are omitted from partial updates.
  • Updated the computed_props method to exclude props that are listed in X-Inertia-Partial-Except while still allowing props defined in X-Inertia-Partial-Data to be included in the partial update.
  • Added unit test to validate that props listed in the X-Inertia-Partial-Except header are not included in the response.

Laravel implementation:

@@ -72,7 +72,7 @@ def merge_props(shared_data, props)
def computed_props
_props = merge_props(shared_data, props).select do |key, prop|
if rendering_partial_component?
key.in?(partial_keys) || prop.is_a?(AlwaysProp)
(key.in?(partial_keys) || prop.is_a?(AlwaysProp)) && !key.in?(partial_except_keys)
Copy link
Owner

Choose a reason for hiding this comment

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

I believe except should not affect always params:

Resolve always properties that should always be included on all visits, regardless of "only" or "except" requests.

https://github.com/inertiajs/inertia-laravel/blob/2.x/src/Response.php#L219

Copy link
Owner

@skryukov skryukov Oct 12, 2024

Choose a reason for hiding this comment

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

Another thing I noticed is that we should support alternative to Arr::forget which supports dot notation (i.e. partial can return a nested hash {auth: {user: '', additional: 'data'}}, so we pass partial: 'auth', except: 'auth.additional' and Inertia Rails returns just {auth: {user: ''}).

This is a snippet (untested 😂) for a Arr::forget alternative in Ruby that also ignores AlwaysProp:

  def drop_partial_except_keys(hash)
    partial_except_keys.each do |key|
      parts = key.split('.').map(:to_sym)
      *initial_keys, last_key = parts
      current = hash.dig(*initial_keys)

      # !current[last_key].is_a?(AlwaysProp) isn't pretty
      # but it allows us to use just one loop over merged props on the prev step
      current.delete(last_key) if current.is_a?(Hash) && !current[last_key].is_a?(AlwaysProp)
    end
  end

I guess we can just call this method on _props:

# _props = ...

drop_partial_except_keys(_props) if rendering_partial_component?

# rendering_partial_component(...

@PedroAugustoRamalhoDuarte wdyt?

Choose a reason for hiding this comment

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

Thanks for the review @skryukov, makes total sense

Choose a reason for hiding this comment

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

I think we can ship a v2 beta without this feature like you said, I have never reached a use case to use this one

Copy link
Owner

@skryukov skryukov Oct 13, 2024

Choose a reason for hiding this comment

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

Thanks @PedroAugustoRamalhoDuarte! I played a bit with the partial except stuff and decided to add them to inertiajs#132, would love to hear your thoughts!

@PedroAugustoRamalhoDuarte
Copy link
Author

@skryukov I have checked the PR, beautiful implementation, congratulations!!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants