Skip to content

fix(native): answer a container query's attribute condition from the container's props - #442

Open
YevheniiKotyrlo wants to merge 5 commits into
nativewind:mainfrom
YevheniiKotyrlo:fix/container-attribute-conditions
Open

fix(native): answer a container query's attribute condition from the container's props#442
YevheniiKotyrlo wants to merge 5 commits into
nativewind:mainfrom
YevheniiKotyrlo:fix/container-attribute-conditions

Conversation

@YevheniiKotyrlo

Copy link
Copy Markdown
Contributor

testContainerQuery leaves a container query's attribute condition unevaluated, so a group-<attribute>: rule applies to every descendant of the container instead of only to the matching ones.

// src/native/conditions/container-query.ts
// if (query.a && !testAttributes(query.a, container.props, guards)) {
//   return false;
// }

#401 found this and takes the safe half — bailing out, so the rule never applies. Its body scoped the real fix and offered it, and this is that: the props are carried, so the condition is answered rather than skipped or assumed. I'd suggest closing #401 in favour of this if you agree with the direction; if you'd rather land the bail first and this on top, I'm happy to rebase onto it.

Why the check could not simply be uncommented

container.props never existed. The container context carries identity alone:

export type ContainerContextValue = Record<string, WeakKey>;   // src/native/reactivity.ts
containers![name] = state.ruleEffectGetter;                     // src/native/react/rules.ts

That is also the reactivity key for hover/active/focus, so it cannot be replaced with a props object, and a prop change on the ancestor has no signal to invalidate a descendant with.

The library already solves that exact shape one export away. containerLayoutFamily is a weakFamily observable keyed on the same identity, written by the container's own component and read through the descendant's getter — which is what subscribes the descendant. This change adds the same thing for props.

The change

File What
native/reactivity.ts containerAttributesFamily — a per-container observable of that container's props, keyed like containerLayoutFamily
native/react/useNativeCss.ts a component that registers a container publishes its props there, after the commit
native/conditions/container-query.ts query.a is evaluated against them, read through get
native/conditions/attributes.ts guards becomes optional

Three details worth calling out, because each is a decision rather than a mechanical step.

The publish effect has no dependency array. The props a descendant queries are not the ones the container's own render guards track — a child writing group-data-[disabled=true]:* reads a key the container itself may render nothing from — so keying the effect on anything the container knows about would miss exactly the changes this channel exists to deliver. The observable's equality is what makes an unchanged republish free.

Equality is narrowed to what a selector can see. Selectors L4 §6.1 compares two strings, so a value that cannot be one is only answerable for presence. children, style and every handler are fresh objects on each render and are indistinguishable to a selector, so they compare as presence; dataSet compares one level deeper, because dataSet={{ open }} is also a fresh object each render and comparing it by identity would defeat the guard for precisely the selectors this serves. Without this a container's re-render notifies every descendant that reads it.

No render guard is recorded for an ancestor condition — hence guards becoming optional. A guard is checked against currentProps on the next render, so it can only speak for the component that owns those props. Recording ["d", "disabled", <ancestor's value>] on a descendant makes testGuards compare it against the descendant's own dataSet.disabled, which mismatches on every render for any element that does not happen to carry the same attribute. The subscription is the invalidation channel instead, and it is one the guard system has no way to express.

Ordering, and the first frame

A descendant renders before its ancestor's effect runs, so it reads undefined and every ancestor condition answers false for that frame, then re-evaluates when the publish lands. That is the same shape containerLayoutFamily already has — a container query answers false until the first onLayout — and it is the safe half of the two wrong answers.

Tests

src/__tests__/native/container-queries.test.tsx, +10 cases. Seven over the behaviour: applied when the container matches, withheld when it carries a different value, withheld when it carries none, withheld when the value sits on the element instead of the container, re-evaluated when the container changes and when it changes back, and the presence form both ways.

Three more pin the edges so a change of mind about them is a deliberate edit:

  • Only the nearest same-named group is consulted. That is correct for a real @container — CSS Containment names the query container as the nearest eligible ancestor — and a divergence for the group form, which is a descendant combinator wearing a container query: CSS matches via ANY ancestor, so an outer match behind a non-matching inner one is missed. Carrying every same-named ancestor means an array in the container context, and a fresh array identity per render defeats the ["c", name, container] render guard, which compares by identity — so it is a context-shape decision rather than a local one, and I have left it alone. Happy to attempt it separately if you would like that direction.
  • An element is not its own group ancestor, which agrees with CSS and is the half most easily broken by answering an ancestor condition from the element's own props.

src/__tests__/native/container-attributes.test.ts (new), +5 cases over the equality: a republish that changed nothing a selector can see does not notify; a dataSet value change, a key added or removed, and a prop appearing or disappearing all do; and the pre-publish reading is undefined.

Both are mutation-proven. Reintroducing the unevaluated condition turns 5 of the 12 cases in container-queries.test.tsx red; removing the attribute-visible projection turns 1 of the 5 in container-attributes.test.ts red.

One note on the existing suite, which I have not changed: several cases assert absence as expect(child).toHaveStyle(undefined), and that matcher passes whatever the element's style is. My new negative cases use not.toHaveStyle({ … }) instead, which is what makes them fail under the mutation — the first drafts used the existing idiom and were green with the defect reintroduced.

Two things this does not fix, measured

[data-state="OPEN" i] compiles to ["d","state","=","OPEN","i"] and testAttribute destructures four elements, so the case-insensitive flag is dropped at the runtime rather than at the compiler. And a boolean dataSet value against a string test value does not match, because = compares with ==. Both are element-level defects that an ancestor condition inherits, since both go through the same testAttributes; neither is in scope here, and a fix to either composes with this one.

Quality gates

yarn test       1063 pass (main: 1048), 21 skipped
                3 fail — pre-existing on main, on Windows only
                (babel path-rewrite; #390 fixes them, CI is green)
yarn typecheck  clean
yarn lint       clean
yarn build      module + commonjs + typescript

Happy to reshape this if you would rather review it differently — the four files are independently revertible commits, and the equality narrowing in particular could land separately if you would prefer the simplest possible first version.

An ancestor attribute selector — `group-data-[state=open]:*`, `group-disabled:*` —
compiles to a container query whose attribute condition asks about the CONTAINER's
props. `ContainerContextValue` is `Record<string, WeakKey>`, so the evaluator has
identity and nothing else to read, and a prop change on the ancestor has no signal
to invalidate a descendant with.

`containerAttributesFamily` is that channel, and it is the shape
`containerLayoutFamily` already uses one export above: a weakFamily observable
keyed on the same identity, written by the container's own component and read
through the DESCENDANT's getter — which is what subscribes the descendant.

The effect carries no dependency array, deliberately. The props a descendant
queries are not the ones this component's own render guards track: a child writing
`group-data-[disabled=true]:*` reads a key the container may render nothing from,
so keying the effect on anything the container knows about would miss exactly the
changes the channel exists to deliver. The observable's equality is what makes an
unchanged republish free.

That equality is narrowed to what a selector can see. Selectors L4 6.1 compares
two strings, so a value that cannot be one is answerable only for presence:
`children`, `style` and every handler are fresh objects on each render and
indistinguishable to a selector, so they compare as presence. `dataSet` compares
one level deeper, because it too is a fresh literal each render and comparing it
by identity would defeat the guard for precisely the selectors this serves.
A render guard is checked against `currentProps` on the next render, so it can
only speak for the component that owns those props. A container query's attribute
condition asks about an ANCESTOR's props, and recording a guard for it would
compare the ancestor's value against the descendant's own prop of that name — a
mismatch on every render for any element that does not happen to carry the same
attribute.

The caller that reads an ancestor's props subscribes to the container's props
observable instead, which is a signal the guard system has no way to express. Every
existing caller still passes a ledger and is unaffected.
The check was commented out because `container.props` never existed — the container
context carries identity alone. With the props published on their own channel it
can run, so `group-data-[state=open]:*` and `group-disabled:*` answer from the
container rather than falling through as satisfied.

Falling through is the worse of the two wrong answers: an unchecked condition
applies the rule to EVERY descendant of the container rather than to none.

The read goes through `get`, so this element's rule effect subscribes to the
container's props and re-evaluates when the ancestor changes. No render guard is
recorded, for the reason the previous commit describes.
Seven cases over the group form: applied under a container that carries the value,
withheld under one that carries a different value, withheld under one that carries
none, withheld when the value sits on the element instead of the container,
re-evaluated when the container changes and when it changes back, and the presence
form both ways.

Three more pin what the runtime does at the edges, so a change of mind about any of
them is a deliberate edit here. Only the NEAREST same-named group is consulted,
which is correct for a real `@container` — CSS Containment names the query container
as the nearest eligible ancestor — and a divergence for the group form, which is a
descendant combinator wearing a container query: CSS matches via ANY ancestor.
Carrying every same-named ancestor means an array in the container context, and a
fresh array identity per render defeats the `["c", name, container]` render guard,
so it is a context-shape decision rather than a local one. An element is also not
its own group ancestor, which agrees with CSS and is the half most easily broken by
answering an ancestor condition from the element's own props.

The negatives assert `not.toHaveStyle({ … })` rather than `toHaveStyle(undefined)`.
The latter passes whatever the element's style is, so the first drafts of these
cases were green with the defect reintroduced.
A container publishes after every commit of its own component, so the observable's
equality is what stands between an ancestor re-rendering and every descendant that
reads it re-evaluating its rules.

Five cases: a republish that changed nothing a selector can see does not notify; a
`dataSet` value change, a key added or removed, and a prop appearing or disappearing
all do; and the pre-publish reading is `undefined`, which is what makes a
descendant's first frame answer false rather than true.
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