Skip to content

Extract rule: template-no-duplicate-id#2426

Merged
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-duplicate-id
Mar 13, 2026
Merged

Extract rule: template-no-duplicate-id#2426
NullVoxPopuli merged 2 commits intoember-cli:masterfrom
NullVoxPopuli:nvp/template-lint-extract-rule-template-no-duplicate-id

Conversation

@NullVoxPopuli
Copy link
Copy Markdown
Contributor

Split from #2371.

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-no-duplicate-id branch 3 times, most recently from 2978742 to 2a564fb Compare February 28, 2026 23:37
@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-no-duplicate-id branch from 2a564fb to 3b4ba96 Compare March 10, 2026 22:27
@johanrd
Copy link
Copy Markdown
Contributor

johanrd commented Mar 12, 2026

Claude Review: comparing against ember-template-lint no-duplicate-id

blockParams scoping causes false negatives. The PR pushes/pops a scope for elements with blockParams, isolating all IDs inside — including static ones. The original doesn't do element-level scoping.
This is missed:

<MyComponent as |foo|>
  <div id="shared-id"></div>
</MyComponent>
<div id="shared-id"></div>

"shared-id" is added inside the component scope, which gets popped on exit — so the second occurrence isn't detected. Remove the blockParams scope push/pop.

Block-param ID disambiguation is different. The original's getMustacheValue walks up to the yield block ancestor and appends tag + line + column to make block-param-derived IDs unique across different
component invocations (e.g. {{inputProperties.id}} in two separate <MyComponent> blocks). The PR uses sourceCode.getText() which returns identical strings for both → would be a false positive once the
scoping above is removed. resolveIdValue needs the same location-aware keying for paths whose head is a block param.

conditionalReportedDuplicates suppresses valid reports. Not in the original. When an outer-scope ID appears in multiple branches, the original reports each one:

<div id={{this.divId00}}></div>
{{#if this.foo}}                                                                                                                                                                                                   
  <div id={{this.divId00}}></div>   {{!-- both report --}}
{{else}}                                                                                                                                                                                                           
  <div id={{this.divId00}}></div>   {{!-- original reports, PR suppresses --}}
{{/if}}                                                                                                                                                                                                            

Original: 2 errors. PR: 1 error. Remove conditionalReportedDuplicates — the conditional stack already handles branch isolation.

@NullVoxPopuli NullVoxPopuli force-pushed the nvp/template-lint-extract-rule-template-no-duplicate-id branch from 3b4ba96 to 846c541 Compare March 13, 2026 19:26
@NullVoxPopuli NullVoxPopuli merged commit 472dbb0 into ember-cli:master Mar 13, 2026
9 checks passed
@NullVoxPopuli NullVoxPopuli deleted the nvp/template-lint-extract-rule-template-no-duplicate-id branch March 13, 2026 19:55
NullVoxPopuli added a commit that referenced this pull request Mar 14, 2026
Post-merge review of #2426 (`template-no-duplicate-id`)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants