feat(data)!: drop id from ECS reads and archetype value types - #173
Merged
Conversation
id is the entity's identity (the key), not a component value: `read(entity)`, FromArchetype/EntityReadValues/Store.EntityValues, and the typed archetype row no longer include it. The id column stays internal — still present on every archetype and typed for swap-remove and manual per-row traversal — and id is now implicit in ensureArchetype (callers never name it). getRowData is no longer exported from @adobe/data/table; public reads go through an id-excluding reader. Also removes the now-obsolete id-ignore special case from @adobe/data-testing Match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…he id name - Throw if a component schema defines a reserved built-in name (id, nonPersistent, nonShared) — at createCore and at store.extend — instead of silently clobbering the built-in (which post-id-removal corrupts the entity-id column, since resolveArchetype seeds it from componentSchemas[ID]). Covered by a unit test that runs for both the core and store factories. - Abstract the "id" component name to a single source of truth: `ID` (runtime) and `IdComponent` (type) in required-components.ts, with `RequiredComponents = Record<IdComponent, Entity>`. All ECS id-column access (data + data-persistence) now goes through these, so the name could be changed in one place. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
State spec pattern for the feature architecture, aligned with the id-removal: - State = named singletons (→ resources) + one `entities: ReadonlyMap<number, V>` keyed by a plain numeric id; entity value types carry NO id (identity is the key), are structural with a composing `is` guard, and property names are unique feature-wide (one name → one component); queries return ids (Set unordered / Array ordered by an `order` component). Rules: state.md (rewritten), flipped data-modelling.md's identity-collection guidance, fixed conformance.md. - @adobe/data-testing: `Match.ref` re-typed to fit numeric/map-key positions so a case's `after`/`samples` entity map keys use distinct `Match.ref` labels (`Match.anyNumber` is a shared singleton that would collapse duplicate keys). - Migrated every entity-bearing sample to the pattern (todo, pixie, space-rock, gpu-hopper): entity types lose id, State uses one `entities` map, transitions + conformance projections + derivations updated; all suites green. Singleton features (tictactoe, dashboard, p2p-tictactoe) need no change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…trip
Objective review of the remove-id branch surfaced two defects:
- The public `extend` path guarded reserved names (id/nonPersistent/nonShared)
on components but NOT on resources. Since createStore constructs the core with
an empty schema, the extend loop is the only guard, so a reserved-named
resource would silently clobber the built-in quadrant marker / identity column.
Add the same throw to the resources loop; cover both paths with a create-store
test (component + resource).
- The transactional-store delete path still destructured `{ [ID]: _ignore, ... }`
off the read values, but reads no longer return id, so it was a no-op masked by
an `as any`. Read values directly; bridge the all-optional read type to the
insert type with one commented runtime-invariant cast.
Also harden a sample spec against a latent stack risk: resolve-bullet-hits minted
ids via `Math.max(0, ...keys)` (spreads every key as an argument) — use a running
max instead.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Building the expected side of an entities map (a `samples` entry, a created-entity
`after`) meant hand-rolling `new Map([[Match.ref("a"), v], ...])` in every sample —
one distinct label per entry so the open keys don't collapse the way a shared
`anyNumber` would. Factor that into `Match.refMap(values)`: an iterable of id-less
values → `ReadonlyMap<number, V>` keyed by process-unique open refs. Reserve a
hand-written `Match.ref(label)` for keys that must CORRESPOND to a reference
elsewhere in the case.
Conform all four entity samples to it (drops gpu-hopper's per-package `entities`
helper and the inline `Match.ref` bookkeeping in todo/pixie/space-rock); document
in state.md + conformance.md. All suites green; clean monorepo typecheck.
Co-Authored-By: Claude Opus 4.8 <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
1 participant
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.
Summary
idis the entity's identity (the key), not a component value. This removes it from the ECS read/value surface while keeping it as an always-present internal column.read(entity),FromArchetype,EntityReadValues/EntityValues/Store.EntityValues,ArchetypeRowOf, the typed archetype row, and thequeryArchetypes/ensureArchetype/store.archetypesgenerics no longer carryid.readgoes through a single-pass id-excluding reader;getRowData(which keeps id, for the migration copy) is no longer exported from@adobe/data/table.columns.idand keepsidin its component set — swap-remove, serialization, and manual per-row traversal are unchanged.idis now implicit:ensureArchetype([...])never namesid;resolveArchetypeadds the id column structurally.@adobe/data-testing: removed the now-obsoleteMatchspecial case that ignored an unmentioned numericid.Semantics
Matches Bevy/Flecs: the entity id is stored per-archetype internally (required for swap-remove) but is not part of the component value set — exposed as a distinct column, never echoed back from a read you already keyed by id.
Tests
@adobe/data(2957) and@adobe/data-testing(12) suites pass; full monorepo typechecks clean.