Skip to content

fix(bindx-react): keep the hook count fixed and subscribe DSL conditions - #78

Closed
matej21 wants to merge 1 commit into
mainfrom
fix/switch-hooks-and-condition-subscriptions
Closed

fix(bindx-react): keep the hook count fixed and subscribe DSL conditions#78
matej21 wants to merge 1 commit into
mainfrom
fix/switch-hooks-and-condition-subscriptions

Conversation

@matej21

@matej21 matej21 commented Aug 20, 2026

Copy link
Copy Markdown
Member

Two defects in the same area, both closed by one missing capability.

1. <Switch> crashes on a conditionally rendered <Case>

SwitchImpl called useField inside a loop over its <Case> children:

for (const entry of entries.cases) {
	// eslint-disable-next-line react-hooks/rules-of-hooks
	accessors.push(useField(resolveTriggerField(entry.props)))
}

The disable carried a comment asserting a "stable count, stable order". Nothing enforced that — SwitchProps.children is ReactNode, so this type-checks:

<Switch>
	{extra ? <Case show={article.status}></Case> : null}
	<Case show={article.title}></Case>
</Switch>

and then:

error: Rendered more hooks than during the previous render.
  at useAccessor (packages/bindx-react/src/hooks/useAccessor.ts:57)
  at SwitchImpl (packages/bindx-react/src/jsx/components/Switch.tsx:147)

A hard crash, not a degradation.

2. cond.* DSL conditions subscribed to nothing

When the condition handed to <If> or <Case if> was a Condition object rather than a bare FieldRef, both components passed null to useField — no subscription at all — while evaluateCondition went on reading the values live.

This is invisible most of the time, because the parent recreates the branch JSX on every render and busts memo. It bites inside a memoized subtree, where the parent's re-render never reaches the child. That is precisely the invariant useEntity.ts:326-329 documents ("memoized children re-render through their own subscription") — the DSL path was the one condition form with no subscription to re-render through.

The regression test pins the structural precondition rather than assuming it: it asserts the memo probe's render count did not increase, that a plain <Field> in the same subtree did update, and only then that the branch flipped.

The fix

Both are the same gap — subscribing to a number of refs not known at build time.

useFields(refs) takes one useSyncExternalStore subscription covering N refs: it reads FIELD_REF_META off anything that has it, ignores nulls and non-refs, dedupes to entityType:entityId, and subscribes to all of them from a single subscribe callback. The hook count no longer depends on the data.

  • <Switch> now calls exactly two hooks regardless of case count. The loop and the eslint-disable are gone.
  • <If> keeps its single useField and adds one unconditional useFields(collectConditionFields(condition)).

Ref→accessor widening goes through overloads, the same way useAccessor already does it — no cast, no any.

Verification

  • bun test tests/react/jsx/ — 74 pass / 0 fail
  • Switch.test.tsx and If.test.tsx are unmodified and green, so existing behaviour (first match wins, show vs if, callback children, the <Default> errors) is preserved without any assertion being touched
  • typecheck clean; full non-browser suite green apart from unrelated in-flight work
  • both new tests confirmed to fail before the fix — Fix/block repeater #2 with Received: null for both components

Worth knowing

The repo has no eslint config at all. Every eslint-disable in the tree is decorative, which is how a rules-of-hooks violation shipped. Wiring up eslint-plugin-react-hooks would be worth a follow-up — componentFactory.ts:80 still has the same shape (stable count today, latent tomorrow), and componentFactory.ts:156 has the same missing-subscription bug as #2 above for createComponent().if().

<Switch> called useField inside a loop over its <Case> children, behind a
rules-of-hooks eslint-disable and a comment asserting a stable count that
nothing enforced. SwitchProps.children is ReactNode, so a conditionally
rendered <Case> type-checks and then crashes React with "Rendered more
hooks than during the previous render".

Separately, a cond.* DSL condition subscribed to nothing: If and Case
passed null to useField whenever the condition was a Condition object
rather than a bare FieldRef, while evaluateCondition kept reading the
values live. Inside a memoized subtree, where the parent's re-render does
not reach the child, the branch never re-evaluated.

Both are the same missing capability — subscribing to a number of refs
that is not known at build time. useFields(refs) takes a single
useSyncExternalStore subscription over N refs, so the hook count stays
constant, and <Switch> and <If> now feed it their condition fields. It
widens ref to accessor through overloads exactly as useAccessor already
does, so no cast is involved.

The repo has no eslint config, so the rules-of-hooks disable removed here
was never enforced by anything — which is how the crash survived.
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