-
Notifications
You must be signed in to change notification settings - Fork 69
Add smart merging with match_on
option
#234
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
Conversation
ab2ae71
to
6ee52a2
Compare
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.
Pull Request Overview
This PR introduces a match_on
option to enable smart merging on specific object keys when using merge
, deep_merge
, and deferred merge props. It updates the API surfaces, rendering logic, tests, and documentation to support generating a new matchPropsOn
field in the Inertia response.
- Extend
merge
,deep_merge
, anddefer
helpers to accept amatch_on
parameter. - Enhance the renderer to collect and output
matchPropsOn
based on provided keys. - Update specs, dummy controller, and docs to demonstrate and validate smart merging behavior.
Reviewed Changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
File | Description |
---|---|
spec/inertia/rendering_spec.rb | Tests updated to expect match_on props and new matchPropsOn . |
spec/dummy/app/controllers/inertia_render_test_controller.rb | Added examples of merge(match_on:) and defer(..., match_on:) . |
lib/inertia_rails/renderer.rb | Built all_merge_props , collected matchPropsOn , and adjusted existing merge logic. |
lib/inertia_rails/merge_prop.rb | Added match_on attribute to MergeProp . |
lib/inertia_rails/defer_prop.rb | Added match_on attribute to DeferProp . |
lib/inertia_rails/inertia_rails.rb | Updated helper methods (merge , deep_merge , defer ) to pass match_on . |
lib/inertia_rails/generators/helper.rb | Renamed guess_typescript to uses_typescript? . |
lib/inertia_rails/generators/controller_template_base.rb | Updated default for :typescript option to call uses_typescript? . |
docs/guide/merging-props.md | Added documentation and examples for match_on . |
docs/guide/deferred-props.md | Linked merging-props guide under “Combining with mergeable props.” |
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.
Looks good! Gonna wait to merge until I cut a release since the docs update immediately upon merge.
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.
Looks good to me! I assume this logic is all handled in the client side and that's why our test is pretty literal?
@@ -23,7 +23,7 @@ def guess_the_default_framework(package_json_path = DEFAULT_PACKAGE_PATH) | |||
end | |||
end | |||
|
|||
def guess_typescript | |||
def uses_typescript? |
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.
❤️
See inertiajs/inertia-laravel#747 and inertiajs/inertia-laravel#732