Skip to content

fix: make processStylexRules sort comparator deterministic#1513

Merged
mellyeliu merged 2 commits intofacebook:mainfrom
matclayton:fix/deterministic-css-sort
Mar 13, 2026
Merged

fix: make processStylexRules sort comparator deterministic#1513
mellyeliu merged 2 commits intofacebook:mainfrom
matclayton:fix/deterministic-css-sort

Conversation

@matclayton
Copy link
Copy Markdown
Contributor

@matclayton matclayton commented Mar 6, 2026

Sorry for what is clearly an Claude/AI generated patch, but we've been seeing issues with StyleX (across multiple recent versions) and it creating non-deterministic ordering of the CSS Output. The same code compiled multiple times, gives different outputs. After having Claude work through the problem, we've manually validated the patch below against our code base, and observed the output to now be deterministic when its applied against 0.18.1 using the unplugin plugin with vite. On the surface the below fits with our understanding of the issue at hand, however I'm not familiar with the detailed inner workings of the plugin.

Summary

The sort comparator in processStylexRules has two bugs that cause non-deterministic CSS rule ordering across builds. When a parallel bundler (Vite, Rolldown, Rollup) transforms modules in a different order between builds, identical source code produces different CSS output — breaking content hashes and causing unnecessary cache invalidation.

Bug 1: Asymmetric @-rule check

// Line 514: only enters query comparison when the FIRST rule has @
if (rule1.startsWith('@') && !rule2.startsWith('@')) {

When rule2 has @ but rule1 doesn't, it falls through to property-only comparison. This means compare(A, B) and compare(B, A) can use different comparison logic — an antisymmetry violation that makes the sort unstable.

Bug 2: Transitivity violation

Three categories of rules use different comparison paths:

  • @media / @container / @starting-style rules -> query-string comparison
  • var(--...)-wrapped rules -> property-string comparison (no @ prefix)
  • Plain / pseudo-element rules -> property-string comparison

Comparing across categories uses different data for the same transitive relationship. For example, given rules A (pseudo-element), B (@starting-style), C (var(--...)-wrapped):

  • A vs B -> query comparison (B starts with @)
  • B vs C -> query comparison (B starts with @)
  • A vs C -> property comparison (neither starts with @)

This violates transitivity (A < B and B < C does not guarantee A < C), causing Array.prototype.sort to produce different results depending on which pairs it compares — which depends on input order.

Fix

Replace the split query/property comparison with a simple, transitive total order:

// Before (buggy):
if (rule1.startsWith('@') && !rule2.startsWith('@')) {
    const query1 = rule1.slice(0, rule1.indexOf('{'));
    const query2 = rule2.slice(0, rule2.indexOf('{'));
    if (query1 !== query2) {
        return query1.localeCompare(query2);
    }
}
const property1 = rule1.slice(rule1.lastIndexOf('{'));
const property2 = rule2.slice(rule2.lastIndexOf('{'));
return property1.localeCompare(property2);

// After (deterministic):
const property1 = rule1.slice(rule1.lastIndexOf('{'));
const property2 = rule2.slice(rule2.lastIndexOf('{'));
const propertyComparison = property1.localeCompare(property2);
if (propertyComparison !== 0) return propertyComparison;
return rule1.localeCompare(rule2);

Sort order: priority -> CSS property -> full rule string. The full ltr rule string as final tiebreaker guarantees a total order (no two distinct rules compare as equal), eliminating all input-order dependence.

Impact

  • CSS file hashes are now stable across builds with identical source code
  • No change to the semantic ordering — rules at the same priority level were never order-dependent for correctness; this only affects the arbitrary ordering of rules within the same specificity layer
  • The useLegacyClassnamesSort path is unchanged
  • All existing snapshot tests pass without changes

Test Plan

Added two new tests in transform-process-test.js:

  1. sort is deterministic regardless of input order — constructs rules mixing @media, @container, @starting-style, var()-wrapped, and pseudo-element rules at the same priority, then processes them in original, reversed, and shuffled orders — asserts all produce identical output
  2. sort is deterministic with duplicate rules in different input orders — verifies determinism when duplicate rules appear (as happens when multiple modules import the same component)

Both tests fail on the current code and pass with this fix.

Discovered and validated in a production Vite + Rolldown build environment where the non-determinism caused CSS file hashes to change on every build despite no code changes.

The sort comparator in `processStylexRules` had two bugs causing
non-deterministic CSS rule ordering when input order varies (which
happens in parallel bundlers like Vite/Rolldown where module transform
order is non-deterministic):

1. Asymmetric @-rule check: the condition
   `rule1.startsWith('@') && !rule2.startsWith('@')` only entered the
   query-string comparison path when the first rule had an @ prefix,
   not the second. This made compare(A, B) use different logic than
   compare(B, A).

2. Transitivity violation: @media/@container/@starting-style rules used
   query-string comparison, but var()-wrapped and plain/pseudo-element
   rules used property-string comparison. Comparing across categories
   used different data for transitive pairs, violating the requirement
   that if A < B and B < C then A < C.

Replace the multi-path comparison with a simple total order:
priority -> CSS property -> full rule string as tiebreaker.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 6, 2026

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

A member of the Team first needs to authorize it.

@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 Mar 6, 2026
…tests

Makes the expected CSS output visible in the test file, which is more
useful for reviewers and makes regressions easier to diagnose.
Copy link
Copy Markdown
Member

@mellyeliu mellyeliu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cleaned up the test assertions a bit. Thanks so much for the fix!

@vercel
Copy link
Copy Markdown

vercel Bot commented Mar 13, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
stylex Ready Ready Preview, Comment Mar 13, 2026 5:55am

Request Review

@mellyeliu mellyeliu merged commit 12f64f2 into facebook:main Mar 13, 2026
6 of 9 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

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

Development

Successfully merging this pull request may close these issues.

2 participants