Skip to content

fix(render): preserve scope chain when 'self:' bound on render tag#2080

Draft
EvilGenius13 wants to merge 1 commit intomainfrom
jf-self-drop-render-bug
Draft

fix(render): preserve scope chain when 'self:' bound on render tag#2080
EvilGenius13 wants to merge 1 commit intomainfrom
jf-self-drop-render-bug

Conversation

@EvilGenius13
Copy link
Copy Markdown
Contributor

@EvilGenius13 EvilGenius13 commented Apr 30, 2026

Summary

  • {% render 'snippet', self: obj %} shadowed the SelfDrop because the render tag wrote self: directly into @scopes[0] under key 'self'. Subsequent self[var] lookups inside the snippet did strict key access on obj instead of walking the scope chain, resolving to nil for any key not in obj.
  • Fix: layered binding. SelfDrop now holds an optional bound_self; [] and key? consult it first and fall through to the existing scope-chain walk on miss. Render#render_tag special-cases attribute key Expression::SELF and routes the value to inner_context.self_drop.bound_self = instead of the generic scope write.
  • Behavior preserved for existing self: users — when the key exists in the bound object, the bound value still wins.

Why

PR #2060 introduced the SelfDrop and the self[var] syntax as the strict-mode replacement for bare-bracket lookups. Its tests exercise self[var] only at the top level via template.render(hash). There was no coverage for the {% render 'snippet', self: obj %} interaction — and the render tag's existing generic attribute mechanism (which writes any attribute key into @scopes[0]) silently shadows the new SelfDrop when the attribute key happens to be 'self'.

This PR adds that coverage and fixes the shadowing.

Implementation

  • lib/liquid/self_drop.rb — added bound_self accessor; [] and key? consult bound first via duck typing (matches the pattern in lib/liquid/variable_lookup.rb:59-61), fall through to @context.find_variable(key) on miss.
  • lib/liquid/context.rb — added a memoized self_drop reader so the SelfDrop returned by find_variable is the same instance the render tag mutates.
  • lib/liquid/tags/render.rb — attribute loop special-cases Expression::SELF (already used at context.rb:215) and routes to inner_context.self_drop.bound_self =.

Lookup order is bound-first, scope-chain-on-miss. Justified because it preserves all currently-passing self: users' hits and only changes previously-nil misses to "now resolves via scope-chain fallthrough."

Each new_isolated_subcontext creates its own Context, its own lazily-created SelfDrop, and its own bound_self — so nested renders, each passing their own self:, stay isolated.

Test plan

  • New test/integration/self_drop_render_test.rb (8 tests, all passing):
    • Baseline: rewritten self[name] resolves correctly when no self: is bound (scope-chain walk works as before)
    • Bound-self shape: self[name] with self: bound resolves the snippet's local assigns when the key is missing from the bound object
    • Forloop-constructed key variant of the bound-self shape
    • Bare-bracket prohibition still raises the expected error in :strict2
    • Two-deep nested render with self: bound at both levels — each snippet sees only its own bound_self
    • Two-deep nested render with inner snippet unbound — leak-detection probes confirm outer self: does NOT leak in
    • Everything-chain composite: two nested renders + top-level self[var] + regular {{ var }} interpolations
    • Bound-key-hit pin: when the lookup key exists in the bound object, the bound value wins (precedence guarantee)
  • No regressions in `test/integration/variable_test.rb`, `test/integration/tags/render_tag_test.rb`, `test/integration/drop_test.rb`, `test/integration/context_test.rb`
  • Mirror change in `Shopify/liquid-vm` — follow-up PR (test scaffolding already exists)

Notes

  • One pre-existing test failure in `test/integration/tags/render_tag_test.rb` (`uninitialized constant RenderTagTest::TestEnumerable`) and one env-dependent flake in `test/integration/context_test.rb` (`stackprof` LoadError). Both verified pre-existing on `main`; not introduced by this PR.
  • `bundle exec rake integration_test` is broken in the local Nix Ruby env (native extension build failures); tests were run individually via `ruby -Itest -Ilib`.

@EvilGenius13 EvilGenius13 force-pushed the jf-self-drop-render-bug branch from 30eb8af to d6a5120 Compare April 30, 2026 19:22
The render tag wrote `self:` attributes directly into @scopes[0] under
key 'self', which shadowed the SelfDrop. `self[var]` lookups inside the
snippet then did strict key access on the bound object instead of
walking the scope chain, resolving to nil for any key not in the object.

SelfDrop now holds an optional `bound_self`; `[]` and `key?` consult it
first via duck typing (matches lib/liquid/variable_lookup.rb), then fall
through to the scope-chain walk on miss. Render#render_tag routes
attribute key Expression::SELF to inner_context.self_drop.bound_self=
instead of the generic scope write. Lookup order is bound-first;
existing self: users' hits stay intact, previously-nil misses now
resolve via fallthrough.

PR #2060 introduced the SelfDrop and bare-bracket prohibition but had
no coverage for the {% render 'snippet', self: obj %} interaction.
Adds 8 tests including two-deep nested renders with leak detection
and a composite chain combining renders, top-level self[var], and
regular variables.

Discovered while investigating SFR strict-parser migration parity
diffs.
@EvilGenius13 EvilGenius13 force-pushed the jf-self-drop-render-bug branch from d6a5120 to bb40962 Compare April 30, 2026 20:46
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.

1 participant