Skip to content

fix(native): answer an attribute selector the way Selectors §6 defines it - #447

Open
YevheniiKotyrlo wants to merge 1 commit into
nativewind:mainfrom
YevheniiKotyrlo:fix/selectors-attribute-matching
Open

fix(native): answer an attribute selector the way Selectors §6 defines it#447
YevheniiKotyrlo wants to merge 1 commit into
nativewind:mainfrom
YevheniiKotyrlo:fix/selectors-attribute-matching

Conversation

@YevheniiKotyrlo

@YevheniiKotyrlo YevheniiKotyrlo commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Summary

testAttribute disagrees with Selectors §6 on six clauses, and with the DOM about the same dataSet object. This fixes all six and adds 102 tests.

Problem

A component writes one dataSet object and one class string; react-native-web projects it onto data-* attributes for the browser, and this package answers the same selector on native. Measured — web through RN-Web's own View + React's renderer answered by Chromium, native by driving testRule:

dataSet.x DOM attribute [data-x] [data-x="true"] [data-x="1"] [data-x=""]
true data-x="true" Y / Y Y / n n / Y n / n
false data-x="false" Y / n n / n n / n n / Y
0 data-x="0" Y / Y n / n n / n n / Y

web / native; bold is a divergence. = compares with == against an operand lightningcss always hands over as a string, and loose equality between a string and a non-string coerces both to numbersNumber("true") is NaN. So true never matches [data-x="true"], while false matches [data-x="0"] and 0 matches [data-x=""]. Every other operator already coerces with .toString(), so = is the one spelling that disagrees with its five siblings about one element.

Five more, independent of the DOM:

  1. [attr] presence excludes false. createDOMProps skips only a nullish dataSet value, so data-x="false" is written and the browser matches.
  2. [att|=val] implements only its prefix half — [lang|="en"] does not match lang="en", the language it names.
  3. [att|=""] represents nothing. §6 gives this operator no empty-operand exclusion; Chromium matches "", "-" and "-foo".
  4. [att~=val] splits on the space character. §6.1's whitespace links to INFRA — TAB, LF, FF, CR, SPACE — so a newline-separated pair reads as one word.
  5. [att=val i] is case-sensitive. AttributeQuery already declares the fifth tuple element and lightningcss already reports caseSensitivity; neither call site pushed one.

Solution

  • = compares strings. Presence splits by query type — nullish-ness for dataSet, !== false for a plain prop, which is what :disabled depends on.
  • |= gains its exact-match arm and loses an exclusion §6 does not give it. ~= splits on INFRA's five code points, \s being a different set of twenty-five.
  • Both compiler call sites build through one attributeQueryFor — which is how [dir=…] became a third construction site with none of the operator handling. Its operand is now folded, dir being on HTML's ASCII-case-insensitive list. Only i is emitted; s asks for the default, so a query carrying it is a fifth element the runtime ignores. Folding is ASCII-only — toLowerCase folds U+212A KELVIN SIGN onto k.
  • The value-to-string conversion runs after the ! arm returns. :empty is that arm with children as its prop, so coercing first threw on a null-prototype child and walked every element of the ordinary array case on every render.

Test plan

src/__tests__/native/attributes.test.tsx, 4 → 106 cases. Each renders a real element and reads back the width the matched rule sets.

The specs' own examples are encoded in their original spelling — span[hello="Cleveland"][goodbye="Columbus"], a[rel~="copyright"], *[lang|="en"] over en / en-US / en-cockney, object[type^="image/"], a[href$=".html"], p[title*="hello"], [frame=hsides i]. The operator censuses are keyed by AttrSelectorOperator and closed with satisfies, so a seventh operator is a compile error.

Every fix was mutation-proved: reverting one arm at a time turns 1–17 named cases red, and the four pre-existing cases (:disabled, :empty, truthy, equals) stay green in all fifteen runs.

yarn typecheck and yarn lint clean; yarn test 1150 passed, 3 failed, 21 skipped. Two babel suites fail identically on an untouched f70c402 worktree (Windows-only module-specifier rewrites) — this touches no babel file.

Out of scope

Each answers identically before and after this change, measured on main. The first two are now filed:

  • fix(compiler): read [class=…] from the prop the class list arrives on #448[class=…] / [class~=…] never match: the query reads props.class, and the compound-class path already knows to map className.
  • Not filed: an attribute query inside :is() / :where() is stored as a container query and never matched. A single-compound :is() is unaffected — lightningcss flattens it first.
  • fix(compiler): drop a namespace-qualified attribute selector #449component.namespace is discarded, so [ns|att] matches the unqualified set and an undeclared prefix matches rather than being dropped.
  • Not filed: a plain prop's model — presence treats false as absent while the operators compare it as "false", a true boolean attribute is "" in the DOM, and React removes an attribute whose value is a function or symbol.

…s it

`testAttribute` disagreed with the specification on six clauses, and with
the DOM on the same element a `dataSet` object projects onto.

§6.1 equality compared with `==` against an operand lightningcss always
hands over as a string. Loose equality between a string and a non-string
coerces BOTH to numbers, so `true` never matched `[data-x="true"]` while
`false` matched `[data-x="0"]` and `[data-x=""]`, and `0` matched
`[data-x=""]` — wrong in both directions, and the second needs no boolean
at all. Every other operator already coerced with `.toString()`, so
equality was the one spelling that disagreed with its five siblings about
one element.

§6.1 presence excluded `false` for both query types. react-native-web
writes a `data-*` attribute for every non-nullish value, `false` included,
so `[data-x]` matches on the web; a plain prop is the boolean-attribute
case and keeps `!== false`, which is what `:disabled` depends on.

§6.1 dash-match implemented only its prefix half, so `[lang|="en"]` did
not match `lang="en"` — the language it names — while matching every
subtag of it. It is also the one operator §6 gives NO empty-operand
exclusion: a browser matches `[att|=""]` against `""`, `"-"` and `"-foo"`.

§6.1 includes-match split on the space character. The separator set is
INFRA's ASCII whitespace, which §6.1 links to — TAB, LF, FF, CR and SPACE.
`\s` is a different set, honouring twenty further code points including
NBSP and the BOM, so a value holding one was read as two words.

§6.3's case flags never left the compiler. `AttributeQuery` already
declared the fifth tuple element and lightningcss already reported
`caseSensitivity`, so `[att=val i]` compiled to a case-sensitive query
with no warning. Folding is ASCII-only: `toLowerCase` applies the full
Unicode mapping and folds U+212A KELVIN SIGN onto `k`.

Both compiler call sites now build through one `attributeQueryFor`, which
is what let `[dir=…]` become a third construction site with none of the
operator handling; its operand is folded, because `dir` is on HTML's
ASCII-case-insensitive list. `s` is not emitted — nothing reads a fifth
element unless it is `i`.

The value-to-string conversion runs AFTER the negation arm returns. `:empty`
compiles to that arm and its prop is `children`, so coercing first threw on
a null-prototype child and walked every element of the ordinary array case
on every render.

Tests go from 4 to 106. The specifications' own examples are encoded in
their original spelling — h1[title], the Cleveland/Columbus conjunction,
a[rel~="copyright"], *[lang|="en"] over en / en-US / en-cockney,
object[type^="image/"], a[href$=".html"], p[title*="hello"] and
[frame=hsides i] — and the operator censuses are keyed by
`AttrSelectorOperator` and bound with `satisfies`, so a seventh operator is
a compile error rather than an assertion that silently stays at six.
@YevheniiKotyrlo

YevheniiKotyrlo commented Sep 10, 2026

Copy link
Copy Markdown
Contributor Author

Device evidence — before / after

UNFIXED — The boolean bar is blue. true == "true" coerces both sides to numbers and Number("true") is NaN, so the one value the style was written for is the one it never matches.

FIXED — The boolean bar and the string bar are both red, and the false bar is blue. = compares strings, so a dataSet boolean and the operand lightningcss always hands over as a string agree.

before — stock 3.0.7 after — with this PR

All three bars carry the identical class list — data-[disabled=true]: over a blue base — and differ only in what their own dataSet holds: the boolean true, false, and the string "true".

Read the third bar, not the whole frame. The string bar is red in both frames, and that is what makes the pair mean something: a string compared with a string never reaches the numeric coercion, so it says the rule compiled and matched on the stock build too. Without it, two blue bars on the left are indistinguishable from a build where the rule never applied at all.

Both frames come from the same device in the same run (Android 36 emulator, 1140×2400 @ 480dpi), but before is stock 3.0.7 rather than "this build minus this PR" — so one unrelated difference is visible and worth naming rather than leaving for you to spot. The compiler's inlineRem defaults to 14 and our build sets it to 16, so every rem-derived length in the before frame renders at 87.5% of the after one: smaller type, tighter spacing, a shorter probe box. That is a different fix, not this one.

The build-probe width=<dp> line is that same rem fold used as a build stamp — a 3rem box, so 42 unfixed and 48 fixed. The capture harness reads it off the device and refuses to save a frame whose probe disagrees with the variant it claims, so a before image cannot silently be a second after.

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