Align WithElements typing with runtime naming - #1
Merged
Conversation
WithElements typing with runtime naming
There was a problem hiding this comment.
Pull request overview
This PR centralizes the static elements naming rule (runtime + TypeScript) into a single module, aligns WithElements typing with the runtime camelization behavior, and adds CI/typecheck coverage plus characterization tests around collisions and acronym dasherization.
Changes:
- Introduces
src/element-definition.tsas the single owner for element accessor naming (runtime) andCamelize/WithElements(types). - Updates the blessing implementation to consume merged element definitions and warn on cross-key generated-name collisions.
- Adds type-level assertions + CI
typecheck, expands tests, and updates docs/changeset accordingly.
Reviewed changes
Copilot reviewed 14 out of 14 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| test/types.test-d.ts | Adds tsc-checked type-level assertions for Camelize/WithElements. |
| test/helpers.test.ts | Removes tests for string helpers now made private/relocated. |
| test/element-definition.test.ts | Adds runtime tests for naming, sample-table lockstep, and collision warnings. |
| test/blessing.test.ts | Adds integration/characterization tests for acronym overrides and collision behavior. |
| src/types.ts | Removes old WithElements typing (moved/rewritten). |
| src/helpers.ts | Removes exported string transforms; keeps readInheritableStaticObjectPairs. |
| src/element-definition.ts | New module implementing runtime naming + type-level Camelize/WithElements. |
| src/blessing.ts | Switches to mergeElementDefinitions + ElementDefinition to generate properties. |
| README.md | Updates documentation to reflect runtime-aligned camelization in WithElements. |
| package.json | Cleans dist/ before build; adds typecheck script. |
| index.ts | Re-exports WithElements from the new module. |
| CONTEXT.md | Records glossary + decisions (acronym lock-in, collision policy, type/runtime lockstep). |
| .github/workflows/test.yml | Adds bun run typecheck step in CI. |
| .changeset/warm-planes-refactor.md | Documents the typing alignment + collision warning behavior as a minor change. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| "sideEffects": false, | ||
| "scripts": { | ||
| "build": "bun build ./index.ts --outdir dist --format esm --external @hotwired/stimulus && tsc -p tsconfig.build.json", | ||
| "build": "rm -rf dist && bun build ./index.ts --outdir dist --format esm --external @hotwired/stimulus && tsc -p tsconfig.build.json", |
myabc
force-pushed
the
fix/element-naming-rule
branch
from
August 10, 2026 18:19
952045c to
e5dc783
Compare
This was referenced Aug 10, 2026
This was referenced Aug 10, 2026
myabc
force-pushed
the
fix/element-naming-rule
branch
from
August 10, 2026 19:39
e5dc783 to
0c23166
Compare
The element-naming rule (key -> xElement/xElements/hasXElement +
attribute suffix) was smeared across helpers.ts, blessing.ts, and
types.ts, with no single owner. types.ts re-encoded it by hand and
already disagreed with the runtime: WithElements<{menu_item}> produced
menu_itemElement while the runtime generated menuItemElement.
- add src/element-definition.ts as single owner of the naming rule,
runtime (elementDefinition, mergeElementDefinitions) and type level
(Camelize mirroring the camelize regex exactly; ASCII-only,
tail-recursive to avoid TS2589 on long keys)
- warn when different raw keys collide on a generated property name
(foo/_foo share hasFooElement; foo's predicate vs hasFoo's getter);
later definition wins per property, as before. Same-key subclass
overrides stay silent
- type each generated name as the union of every role producing it:
non-colliding keys keep exact accessor types, colliding names (e.g.
hasFooElement from foo + hasFoo) force callers to narrow, since
runtime key order decides. Numeric keys are stringified before
Camelize, matching Object.keys
- lock in acronym dasherization (htmlURL -> html-u-r-l) with tests
- gate type assertions (test/types.test-d.ts) via new typecheck
script; clean dist before build so stale declarations cannot ship
- record domain glossary and decisions in CONTEXT.md
myabc
force-pushed
the
fix/element-naming-rule
branch
from
August 10, 2026 20:48
0c23166 to
5d606a3
Compare
Open
3 tasks
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.
Ticket
N/A — internal architecture review of this library.
Stacked on #6 (vitest browser-mode migration): the branch is rebased onto
code-maintenance/vitest, so this PR's file list shows #6's commit until #6 lands onmain(GitHub's stack feature blocks retargeting the base). Chain: #6 ← this ← #2 ← #4.What are you trying to accomplish?
Give the element-naming rule (
static elementskey →xElement/xElements/hasXElement+ attribute-override suffix) a single owner. It was spread acrosshelpers.ts,blessing.ts, andtypes.ts— and the type already disagreed with the runtime:WithElements<{menu_item: string}>producedmenu_itemElementvs runtimemenuItemElement(documented as a README caveat instead of fixed). Different keys could also silently collide on generated names (foo/_foo→hasFooElement).What approach did you choose and why?
src/element-definition.tsowns the rule at runtime (elementDefinition,mergeElementDefinitions) and type level (Camelize<K>mirroring thecamelizeregex exactly). String helpers become private;types.tsfolded in.WithElementsnow camelizes like the runtime — keys pass verbatim. Compile-surface breaking for snake_case-typed consumers (changeset: minor).Object.assignsemantics). Colliding names get a union type, forcing callers to narrow — an intersection would hide the pathology.htmlURL→html-u-r-l) locked in by tests, Stimulus-compatible.typecheckscript gatestest/types.test-d.tsin CI; build cleansdist/first.CONTEXT.mdrecords glossary + decisions.Verification:
bun test40 pass,bun run typecheckclean,bun run buildclean.Merge checklist
Lookbook docsn/a — npm library; README + CONTEXT.md updatedTested major browsersn/a — no browser-dependent behavior