Make entity references unambiguous: ids are written #id - #433
Merged
Conversation
The reference grammar now says which form a value takes instead of guessing. A reference beginning with '#' is an element id (or any id-rooted selector), resolved document-wide and authoritative - it bypasses the name lookup entirely, so an entity named '#body' can never shadow the element whose id is 'body'. Any other reference is the name of an entity-fronting element, resolved lexically through the entity hierarchy and then against the document, with explicitly written CSS selectors as the document-wide last interpretation. The bare getElementById fallback is gone: a bare value denotes a name, never an id, so adding an in-scope entity can no longer silently retarget a reference that used to resolve by id - and the same spelling no longer means two things. A bare reference that names nothing but matches an element id gets a migration pointer in the unresolved warning - write '#id' - on every resolving surface, script attributes included. The document name lookup now also runs ahead of the selector interpretation, so a name that collides with an HTML tag (head, body, main) resolves the entity rather than the page element. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Review fixes, in three parts. A bare reference is now a name and nothing else: the trailing interpretation as an arbitrary CSS selector is gone, so entity-a set to a value like 'pc-entity' can only ever mean the entity named that - it can no longer silently become a type selector when no such name exists, which was the same retargeting hazard in another form. Selector power lives entirely behind '#': a '#' reference is any document-wide selector rooted in an id, so compound selection stays expressible. The grammar now reaches the published metadata: the CEM analyzer takes member descriptions from getters, so the eight reference getters carry the same grammar parenthetical as their setters, the pc-script-instance docs state that entity: ids need the '#' form, and the wrong-target advice names all three entity-fronting kinds instead of pc-entity alone. The migration hint is now correct and honest: the suggested form is CSS.escape'd (an id like 'a:b' must be written '#a\:b' to parse as a selector), it is offered only when the id belongs to an entity-fronting element - anything else would just trade this warning for the wrong-target one - and the script surface shares the same helper, suggesting the prefixed 'entity:#id' spelling. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The CEM member and the derived 'attributes' HTML attribute take their descriptions from the scriptAttributes getter - and the attribute reducer keeps only the getter's first sentence - so the grammar the setter and class doc explain never reached custom-elements.json or the editor data. The getter now states it inside its first sentence (an entity: value is a name or a document-wide '#' selector, never a bare element id), and the element summary that VS Code shows on hover carries the concise form. Verified in the rebuilt manifest and VS Code custom data. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
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.
Fixes #432
After #430/#431 a bare reference had two possible meanings — a lexically scoped entity name, or a document-wide element id via the legacy
getElementByIdfallback — so adding an in-scope entity could silently retarget a reference that previously resolved by id, and neither authors nor tooling could tell which form markup intended. The grammar is now closed: every reference has exactly one interpretation, and no change elsewhere in the document can flip which form it takes.body#body,#hud pc-entityWhat changed
getElementByIdfallback is removed. A bare value denotes a name, never an id.querySelector(ref)pass meantentity-a="pc-entity"was the entity namedpc-entityif one existed, and otherwise silently the first<pc-entity>via type selector — the same retargeting hazard in another spelling. Selector power now lives entirely behind#: any selector rooted in an id works (#hud pc-entity), which also closes the old trap where a name colliding with an HTML tag (head,body) matched the page element. An explicitselector:form can be added later, non-breakingly, if arbitrary un-rooted selectors ever prove needed.#...references bypass the name lookup entirely — resolved document-wide before any scoped phase runs, so an unusually named entity (name="#body") can never shadow the element whose id isbody.write '#a\:b' to reference the element with that id(CSS.escape, so the suggestion actually parses as a selector;getElementByIdaccepts ids no unescaped selector can express). A same-spelled id on a non-entity element keeps the generic advice — suggesting#there would only trade this warning for the wrong-target one. Theentity:script surface shares the helper and suggests the prefixed spelling (entity:#id).custom-elements.jsonandvscode.html-custom-data.json.pc-script-instance's class andscriptAttributesdocs state the id rule forentity:values, and the wrong-target advice names all three entity-fronting kinds (Point x at a pc-entity, pc-model or pc-node instead.).Compatibility
Pre-1.0 hard change, per the issue's preference. Markup relying on a bare id stops resolving and warns with the exact replacement to write; markup relying on bare selector interpretation stops resolving with the standard diagnostics. All shipped examples were audited for both: every reference is a name or a
#id— nothing relied on either removed fallback. The repo's own fixtures that used bare ids (button/scrollbar/scroll-view/joint/script suites) migrated to#id, keeping the id form covered through every element.Tests
Following the issue's list, plus the closure pins:
pc-entityandpc-entity[name="Cube"]resolve nothing even with matching elements present;#idresolves the document element, is not shadowed by an in-scope entity literally named#id, stays document-wide from inside a scope, and any#-rooted compound selector works (#wrap pc-entity);resolveEntityandentity:script attributes, including the escaped suggestion for an id likea:band the non-entity id case keeping generic advice;Full suite: 1045 tests across 49 files; lint, type-check and build (CEM validation) clean. Live smoke of the AR Wiener Storm example (
?sim): clones still self-wire by name with zero resolution warnings.🤖 Generated with Claude Code