Skip to content

hbs-parser: register block-param scope for as |x|#189

Merged
NullVoxPopuli merged 2 commits intoember-tooling:mainfrom
johanrd:hbs-block-param-scope
Apr 16, 2026
Merged

hbs-parser: register block-param scope for as |x|#189
NullVoxPopuli merged 2 commits intoember-tooling:mainfrom
johanrd:hbs-block-param-scope

Conversation

@johanrd
Copy link
Copy Markdown
Contributor

@johanrd johanrd commented Apr 15, 2026

Summary

Registers block-param scope in the HBS parser so <Foo as |x|> and {{#let ... as |x|}} create proper variable bindings in the scope manager. Previously the HBS parser analyzed an empty stub Program, so no block params landed in any scope.

Why

Came up reviewing ember-cli/eslint-plugin-ember#2689 (template-no-block-params-for-html-elements). The lint rule's scope check is effectively dead code on .hbs files because the HBS parser never populates scope with template-defined variables — only the html-tag allowlist heuristic carries that case. Same gap likely affects any other rule that consults scope for HBS templates.

Approach

  • Adds a new registerHBSScopes in transforms.js that traverses the Glimmer AST and declares only block-param variables. Free identifiers ({{path}}, <Tag>) are intentionally left unregistered so HBS keeps its "all template locals are runtime-defined" policy and no-undef stays quiet. registerGlimmerScopes is unchanged.
  • In hbs-parser.js, rebinds the analyzed global scope from a stub Program onto the real Program. Analyzing the real Program directly causes infinite recursion in esrecurse — the Glimmer subtree carries parent back-links.
  • Drive-by: adds an early-exit to registerBlockParams when there are no block params, avoiding empty BlockScopes that would pollute findParentScope/findVarInParentScopes lookups (also benefits the gjs/gts path).

Test plan

  • <Foo as |x|>{{x}}</Foo> — block scope with x declared
  • {{#let foo as |router|}}{{router}}{{/let}} — block scope with router
  • <Foo as |a b c|> — multiple params each registered
  • <Foo>hi</Foo> — no block scopes created
  • All existing tests still pass (36 total)
  • pnpm lint clean

The HBS parser previously analyzed an empty stub Program, so block
params from `<Foo as |x|>` and `{{#let ... as |x|}}` were never
registered in any scope. Downstream rules that walk scope to resolve
block-param references (or check whether an angle-bracket tag is a
local variable, e.g. template-no-block-params-for-html-elements in
eslint-plugin-ember) had no scope to consult and silently fell back to
heuristics on .hbs files.

Wires the existing `registerGlimmerScopes` helper into the HBS path
with a new `blockParamsOnly` mode, leaving free identifiers
(`{{path}}`, `<Tag>`) unregistered so they remain runtime-defined and
do not surface as no-undef errors. The global scope is rebound from a
stub Program to the real Program — analyzing the real Program directly
hits an esrecurse cycle on the Glimmer subtree's parent back-links.

Also adds an early-exit to `registerBlockParams` when there are no
block params to declare, which avoids creating empty BlockScopes that
slow `findParentScope`/`findVarInParentScopes` lookups (benefits the
gjs/gts path too).
johanrd added a commit to johanrd/eslint-plugin-ember that referenced this pull request Apr 15, 2026
…svg-tags + scope

Replaces the inverse component-detection heuristic with a whitelist of
HTML and SVG tag names plus a scope check. With ember-tooling/ember-eslint-parser#189
populating block-param scope in HBS, lists + scope now works in both modes.
Custom elements and unknown lowercase tags remain accepted false negatives.
johanrd added a commit to johanrd/eslint-plugin-ember that referenced this pull request Apr 15, 2026
…params

Replaces the manual block-param stack with sourceCode.getScope(node.parent)
+ a walk up the scope chain. GTS templates work today (36/37 tests pass).

BLOCKED BY UPSTREAM: ember-tooling/ember-eslint-parser#189. The HBS parser
currently builds an empty scope manager, so Glimmer block params aren't
registered for .hbs files. The failing test
`{{#let ... as |plaintext|}}<plaintext />` documents this gap and will
start passing once PR 189 is merged and consumed here.

Uses getScope(node.parent) rather than getScope(node) so an element's
own `as |x|` params (attached to that element's block scope) don't
shadow its own tag — e.g. `<marquee as |marquee|>` must still flag the
outer <marquee>.
johanrd added a commit to johanrd/eslint-plugin-ember that referenced this pull request Apr 15, 2026
…params

Replaces the manual block-param stack with sourceCode.getScope(node.parent)
+ a walk up the scope chain. GTS templates work today (36/37 tests pass).

BLOCKED BY UPSTREAM: ember-tooling/ember-eslint-parser#189. The HBS parser
currently builds an empty scope manager, so Glimmer block params aren't
registered for .hbs files. The failing test
`{{#let ... as |plaintext|}}<plaintext />` documents this gap and will
start passing once PR 189 is merged and consumed here.

Uses getScope(node.parent) rather than getScope(node) so an element's
own `as |x|` params (attached to that element's block scope) don't
shadow its own tag — e.g. `<marquee as |marquee|>` must still flag the
outer <marquee>.
Comment thread src/parser/transforms.js Outdated
* runtime-defined and must not surface as no-undef errors.
*/
export function registerGlimmerScopes(result) {
export function registerGlimmerScopes(result, { blockParamsOnly = false } = {}) {
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I think it might be better to undo the changes to this function and instead export a registerHBSScopes function

Comment thread tests/hbs-parser.test.js
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thank you!

Copy link
Copy Markdown
Member

@NullVoxPopuli NullVoxPopuli left a comment

Choose a reason for hiding this comment

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

Just one small thing

@johanrd johanrd requested a review from NullVoxPopuli April 15, 2026 21:38
@NullVoxPopuli NullVoxPopuli merged commit d8473f1 into ember-tooling:main Apr 16, 2026
34 checks passed
@github-actions github-actions Bot mentioned this pull request Apr 16, 2026
@NullVoxPopuli NullVoxPopuli added the enhancement New feature or request label Apr 16, 2026
@johanrd
Copy link
Copy Markdown
Contributor Author

johanrd commented Apr 16, 2026

@NullVoxPopuli thanks for the quick review!

Seems like npm publish failed in ci: https://github.com/ember-tooling/ember-eslint-parser/actions/runs/24485077427/job/71557984384

@github-actions github-actions Bot mentioned this pull request Apr 19, 2026
johanrd added a commit to johanrd/eslint-plugin-ember that referenced this pull request Apr 19, 2026
ember-tooling/ember-eslint-parser#189 populates block-param scope for
<foo as |x|> in HBS mode, so the rule's scope check in
lib/rules/template-no-block-params-for-html-elements.js no longer relies
on an empty scope manager in .hbs files. No observable test change on
master (all tests here use <template>...</template> / GJS mode where
scope already worked), but makes the scope-based detection correct in
HBS too.
johanrd added a commit to johanrd/eslint-plugin-ember that referenced this pull request Apr 19, 2026
Unblocks the HBS valid-case test

    {{#let (component 'whatever-here') as |plaintext|}}
      <plaintext />
    {{/let}}

which relies on ember-tooling/ember-eslint-parser#189 populating block-
param scope in the hbs-parser. All 37 tests (GTS + HBS) pass.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants