Skip to content

fix(bindx): materialize an embedded has-many before reading or mutating it - #76

Merged
matej21 merged 1 commit into
mainfrom
fix/hasmany-embedded-materialization
Aug 20, 2026
Merged

fix(bindx): materialize an embedded has-many before reading or mutating it#76
matej21 merged 1 commit into
mainfrom
fix/hasmany-embedded-materialization

Conversation

@matej21

@matej21 matej21 commented Aug 19, 2026

Copy link
Copy Markdown
Member

Fixes the one test that has been failing on main.

The bug

A has-many loaded as embedded data on its parent — what a query with a nested selection produces — has no state in the store until something materializes it. HasManyListHandle.items and getById call materializeEmbeddedItems(); isDirty and every mutator did not.

So a consumer that never iterated the list:

  • read isDirty === false no matter what, and
  • had its disconnect / connect / add / remove / move / reset land on a state nobody reads.

Silently. No error, no warning.

// embedded list, nothing has touched `items` yet
tags.isDirty        // false
tags.disconnect('tag-1')
tags.isDirty        // still false — the write went nowhere

The irony is that the method's own doc comment already anticipated exactly this class of mistake:

Materialisation must NOT be a side effect of iterating items only: the block editor (and any consumer) resolves a single child via getById() without ever reading items, and that path must also see populated data.

getById was added to the guard; the mutators and isDirty were not.

The victim in this repo

FormHasManyRelationScope renders dirty={relation.isDirty} and never touches items, so packages/bindx-form/tests/formRelations.test.tsx > tracks dirty state from has-many relation has been failing on main. I bisected it back to 3c2fd0d — it predates the recent store work and is not fallout from #49/#50/#56.

CI never reported it, because the test script covers only tests/unit, tests/react, tests/cases and tests/*.test.* — not packages/*/tests, tests/bindx-client, tests/repeater or tests/shared. Worth closing that gap separately.

The fix

Every entry point that depends on the state existing now goes through the same idempotent guard, and the doc comment says so, so a new entry point has a rule to follow rather than a precedent to guess at.

Tests

tests/unit/handles/hasManyEmbeddedMaterialization.test.ts — deliberately under tests/unit so CI actually runs it. Four cases; two fail without the change (verified by reverting the source file):

  • disconnect marks the relation dirty without reading items first — fails without
  • the disconnect is visible to a later items read — fails without
  • connect marks the relation dirty without reading items first — passes either way, kept as coverage
  • reading items first still works — the idempotency control

packages/bindx-form/tests/formRelations.test.tsx goes 2 pass / 1 fail → 3 pass.

Verification

Branch gated in isolation with its own bun install: typecheck clean, bun run test 1531 pass / 0 fail, bun test packages/ 170 pass / 0 fail (was 169 pass / 1 fail).

Not verified

Only the embedded-list shape is exercised. A has-many loaded by its own separate query already has store state, so it was never affected; that path is covered by the existing suites rather than by anything new here. Whether the same gap exists on the has-one side is not addressed by this PR.

🤖 Generated with Claude Code

https://claude.ai/code/session_01GiniFfaE4gb5EuQpQ3Ncee

…ng it

A has-many loaded as embedded data on its parent has no state in the store
until something materializes it. `items` and `getById` do that; isDirty and
every mutator did not. So a consumer that never iterated the list read
isDirty === false and had its disconnect/connect/add/remove land on a state
nobody reads — silently, with no error.

FormHasManyRelationScope is the case in this repo: it renders
`dirty={relation.isDirty}` without ever touching `items`, so its test
"tracks dirty state from has-many relation" has been failing on main. That
failure was invisible to CI, because the `test` script covers only
tests/unit, tests/react and tests/cases — not packages/*/tests.

Every entry point that depends on the state existing now goes through the
same guard, which is idempotent. The regression test lives under tests/unit
so CI actually runs it; two of its four cases fail without this change.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01GiniFfaE4gb5EuQpQ3Ncee
@matej21
matej21 force-pushed the fix/hasmany-embedded-materialization branch from 427127f to 18d10e4 Compare August 20, 2026 09:27
@matej21
matej21 merged commit 78b2cc2 into main Aug 20, 2026
3 checks passed
@matej21
matej21 deleted the fix/hasmany-embedded-materialization branch August 20, 2026 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant