Skip to content

[types] Accept revert and revert-layer, and fix six properties that accept no CSS-wide keywords - #1879

Closed
henryqdineen wants to merge 4 commits into
facebook:mainfrom
henryqdineen:hqd-stylex-css-types-keywords
Closed

[types] Accept revert and revert-layer, and fix six properties that accept no CSS-wide keywords#1879
henryqdineen wants to merge 4 commits into
facebook:mainfrom
henryqdineen:hqd-stylex-css-types-keywords

Conversation

@henryqdineen

@henryqdineen henryqdineen commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator

What changed / motivation ?

Stacked on #1878. GitHub will not let a pull request from a fork target a branch on that fork, so this targets main and its diff therefore includes #1877 and #1878. Review the last two commits only; the diff will shrink to just those once the earlier PRs merge.

Two bugs, both found by the CSS-wide keyword test added in #1878.

revert and revert-layer were missing from every property. The all alias that 516 properties compose stopped at unset. revert has been Baseline since 2021 and revert-layer since 2022; csstype has carried both for years. Fixed in one place, since all is the single definition.

Six properties accepted none of the CSS-wide keywords — not even inherit. colorScheme: 'inherit' was a type error, as were marginTrim: 'unset', paintOrder, textJustify, WebkitBackgroundClip and WebkitBoxOrient. They were written null | 'a' | 'b' rather than all | 'a' | 'b'.

Linked PR/Issues

Stacked on #1878.

Additional Context

The cause of the second one is worth recording, because it explains why nobody noticed. Those six are the only properties in the file with no named type alias. Everything else declares type foo = … and writes foo?: all | foo; these inline their union at the property instead — and reaching for null, which is what all happens to start with, rather than composing all, is the slip that inlining hid. Five date to the initial commit; WebkitBackgroundClip was added later by "Chore: Allow additional -webkit- properties (#28)" and copied the shape.

Both fixes are pure widenings: no value that used to typecheck stops doing so. Confirmed after the change that color: 'revert-layer', colorScheme: 'inherit', marginTrim: 'unset' and paintOrder: 'revert' all compile.

The NO_GLOBAL_KEYWORDS allowlist that #1878 introduced for these six is deleted rather than shortened, so the pattern cannot come back.

Pre-flight checklist

🤖 Generated with Claude Code

henryqdineen and others added 4 commits September 10, 2026 21:34
@henryqdineen @claude
[types] Hand-maintain StyleXCSSTypes.d.ts for autocompletable unions
98ae19c 
`gen-types` translates the Flow types into TypeScript, which loses the most
useful thing these types can do, because a Flow union containing `string`
collapses to `string` and discards every literal member:

    Flow:       type Display = 'block' | 'flex' | string;
    TypeScript: type Display = 'block' | 'flex' | (string & {});

`string & {}` is mutually assignable with `string`, so any string is still
accepted, but the literals survive and editors keep suggesting them. There is
no way to express this in Flow.

`gen-types` copies a `.d.ts` verbatim when one sits beside the Flow source, so
checking this file in is enough to take over that translation -- no build
changes. Verified byte-identical to what `gen-types` produced, once the 249
`string` -> `(string & {})` substitutions are reversed.

100 of the 522 properties were typed as bare `string` or `number | string` and
so offered no completions at all; another ~340 lost their literals to a
`string` member. Those now autocomplete.

`packages/typescript-tests/src/open-unions.ts` covers the behaviour: arbitrary
strings and `var()` are still accepted, `null` still unsets a property, the
CSS-wide keywords still resolve, literals survive, and closed unions stay
closed. Note `NonNullable` cannot be used to inspect these unions -- it is
`T & {}` in TypeScript >=4.9, and that intersection flattens them.

The two files must now be kept in sync by hand. Adding a css-tree-based test to
enforce that, and to check the literals against the CSS grammar, is the
intended follow-up.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@henryqdineen @claude
[types] Test CSSProperties against the CSS grammar, Baseline, and Flow
8b7f11f 
Five things about `CSSProperties` are hand-maintained and can drift silently.

`StyleXCSSTypes.d.ts` duplicates `StyleXCSSTypes.js` so that TypeScript can use
`(string & {})` where Flow only has `string`. Nothing stops the two files
diverging, so the first test reads both, reduces each property to the keywords,
numbers and strings it accepts, and asserts the two agree. `string & {}` reads
as an arbitrary string, which is exactly what it is.

`@babel/parser` reads both dialects -- the Flow types with the `flow` plugin,
the definitions with `typescript` in declaration mode -- so one reader covers
both files. The two ASTs use disjoint node names, so a single switch handles
either.

The CSS-wide keywords are valid on every property and appear in no grammar, so
the second test is the only thing that can check them. Properties compose them
from the `all` alias.

The property values are written by hand, so `css-tree`'s value definition
grammar checks them in both directions. The third test asserts every keyword in
the types is one the grammar accepts, catching typos and invented values. The
fourth asserts the reverse for closed unions -- that every keyword the grammar
allows is typed -- catching values a user simply cannot write. Both follow
`<'other-property'>` references, which is how logical properties and shorthands
are defined.

The reverse check is restricted to closed unions on purpose. Where a property
accepts an arbitrary string, every keyword is already accepted and completeness
means nothing; asserting it everywhere demands over ten thousand additions,
because shorthands like `background` transitively pull in every named colour.
Restricted, it finds 125 real omissions across 33 properties.

The set of properties is also hand-maintained, so the fifth test asserts every
Baseline-available property is typed. `web-features` is the data behind MDN's
"Widely available" / "Newly available" banners; css-tree carries no support
data at all, and mdn-data's `status` describes the spec rather than browsers --
it calls `font-stretch` obsolete though every browser has supported it since
2020, while marking its replacement `font-width` experimental. Using Baseline
rather than "css-tree knows about it" keeps the exclusion list to real
decisions: 25 entries rather than 82.

All five pass as-is. Each list entry carries a reason, and each failure says
what to do about it:

- 6 properties accepting none of the CSS-wide keywords, written
  `null | 'a' | 'b'` rather than composing `all`. They are also the only
  properties in the file with no named type alias -- inlining the union at the
  property is what hid the slip.
- 17 properties css-tree has no grammar for: abandoned drafts (`motion-*` became
  `offset-*`, the CSS Display Level 3 longhands were dropped), `@font-face`
  descriptors that were never properties (`src`, `unicode-range`), and names
  that never shipped unprefixed.
- 18 properties keeping a keyword the grammar has dropped, where real
  stylesheets still use it. `marginTrim` is the clearest: the spec was rewritten
  to `none | in-flow | all`, but Safari ships the original `block`/`inline`
  syntax.
- 33 properties whose closed union is missing keywords the grammar allows --
  `display` cannot take `flow` or `table-caption`, `overflow` cannot take
  `overlay`, `mixBlendMode` cannot take `plus-darker`. `marginTrim` appears in
  both keyword lists, which is honest: it keeps values the spec dropped and is
  missing values the spec added.
- 25 Baseline-available properties that are not typed: 7 that should be, 17 SVG
  presentation attributes, and `all`.

`web-features` is pinned exactly rather than by range, because its whole
purpose is to change over time: a floating version would let a routine install
flip a property to Baseline and fail the build with no code change. Pinned, the
test only fires on a deliberate bump -- which is when the news is wanted.

Verified by injecting each failure: a typo'd keyword trips the grammar and
parity tests and is reported by name; editing one type file without the other
trips only parity; removing an allowlist entry reports what to add, and
satisfying one reports the entry as stale.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@henryqdineen @claude
[types] Accept revert and revert-layer
d520c2c 
Worked example of the CSS-wide keyword loop: add the two missing keywords to
`GLOBAL_KEYWORDS`, and the test names every property that cannot take them.

    display: does not accept 'revert', 'revert-layer'. Either compose the
    `all` alias, or add it to NO_GLOBAL_KEYWORDS with a reason.

`revert` and `revert-layer` are CSS-wide keywords, valid on every property, but
the `all` alias that all 516 properties compose stopped at `unset`. `revert`
has been Baseline since 2021 and `revert-layer` since 2022; csstype has carried
both for years.

Fixed in one place, since `all` is the single definition. Pure widening -- no
value that used to typecheck stops doing so.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@henryqdineen @claude
[types] Compose all in the six properties that inlined their union
a0dd0cd 
Worked example of emptying an allowlist: delete `NO_GLOBAL_KEYWORDS` and the
test names each property and what it is missing.

    colorScheme: does not accept 'inherit', 'initial', 'unset', 'revert',
    'revert-layer'. Every property should compose the `all` alias.

These six were written `null | 'a' | 'b'` rather than `all | 'a' | 'b'`, so
they accepted none of the CSS-wide keywords -- `colorScheme: 'inherit'` was a
type error, as were `marginTrim: 'unset'`, `paintOrder`, `textJustify`,
`WebkitBackgroundClip` and `WebkitBoxOrient`.

They are also the only six properties in the file with no named type alias:
everything else declares `type foo = ...` and writes `foo?: all | foo`, while
these inline their union at the property. Reaching for `null` -- what `all`
happens to start with -- instead of composing `all` is the slip that inlining
hid. Five date to the initial commit; `WebkitBackgroundClip` was added later by
"Chore: Allow additional -webkit- properties (facebook#28)" and copied the shape.

Pure widening. The allowlist is now gone rather than shortened, so nothing can
reintroduce the pattern.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@vercel

vercel Bot commented Sep 11, 2026

Copy link
Copy Markdown

@henryqdineen is attempting to deploy a commit to the Meta Open Source Team on Vercel.

A member of the Team first needs to authorize it.

@henryqdineen henryqdineen mentioned this pull request Sep 11, 2026
Closed
2 tasks
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Sep 11, 2026
@henryqdineen

henryqdineen commented Sep 11, 2026

Copy link
Copy Markdown
Collaborator Author

Superseded by #1884. Reopened from branches in this repository rather than my fork, so the five PRs can be a proper stack — cross-fork stacks aren't supported, and each PR here was showing the earlier ones' commits in its diff. Same commits, unchanged.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mellyeliu mellyeliu Awaiting requested review from mellyeliu mellyeliu will be requested when the pull request is marked ready for review mellyeliu is a code owner
@nmn nmn Awaiting requested review from nmn nmn will be requested when the pull request is marked ready for review nmn is a code owner

Assignees

No one assigned

Labels

CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

1 participant

@henryqdineen