fix: Preserve locals when rendering inline partial for object#613
Open
moberegger wants to merge 3 commits intorails:mainfrom
Open
fix: Preserve locals when rendering inline partial for object#613moberegger wants to merge 3 commits intorails:mainfrom
moberegger wants to merge 3 commits intorails:mainfrom
Conversation
moberegger
commented
Apr 3, 2026
| else | ||
| _scope do | ||
| options[:locals] = { options[:as] => object } | ||
| options[options[:as]] = object |
Contributor
Author
There was a problem hiding this comment.
This is the crux of the fix.
moberegger
commented
Apr 3, 2026
Comment on lines
-181
to
-184
| def _render_partial(options) | ||
| options[:locals][:json] = self | ||
| @context.render options | ||
| end |
Contributor
Author
There was a problem hiding this comment.
Since this only had one call site...
moberegger
commented
Apr 3, 2026
Comment on lines
+177
to
+178
| options[:locals][:json] = self | ||
| @context.render options |
Contributor
Author
There was a problem hiding this comment.
... I just inlined it here.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Ran into an odd bug. Given a template like
Rendering to a collection inline like
Would correctly render with the value of
my_localwith the value of'custom'{ "foos": [ "id": 1, "name": "Test", "myLocal": "custom" ] }But rendering a single object wasn't picking up the value for the local
{ "foo": { "id": 1, "name": "Test", "myLocal": "default" } }If the partial didn't have a default value set for strict locals
The following
would raise
The reason is because this render path in
_set_inline_partialwould set a key forlocalsbefore the render is done in_render_partial_with_options._render_partial_with_optionsattempts to pluck key args forlocals, but since the key is already there, they are effectively ignored.This may have been a regression introduced by #591 which made it no longer
reverse_merge!on thelocalskey. I haven't verified this, though.The fix was to not have
_set_inline_partialsetlocals, and instead defer that logic to_render_partial_with_options.I added some extra tests cases to cover this.
I also updated the docs to include an example for rendering an object to an inline partial. This is supported by the library, but was seemingly undocumented. Also documented rendering to partials inline with other locals, which was also supported by the library but seemingly undocumented.
As an aside: With some small refactoring, I think we can drop the need for the intermediary
_set_inline_partialmethod. It doesn't do too much and I think the coordination oflocalscan all be handled within_render_partial_with_options. This would save a bit on indirection and also align everything to the same render path. Outside of the scope of this PR, but perhaps something I can try later.