Skip to content

fix(publisher): order viewport contexts by a total order, not a partial one - #482

Open
borskyj-symph wants to merge 1 commit into
CoreBunch:mainfrom
borskyj-symph:fix/viewport-context-cascade-total-order
Open

fix(publisher): order viewport contexts by a total order, not a partial one#482
borskyj-symph wants to merge 1 commit into
CoreBunch:mainfrom
borskyj-symph:fix/viewport-context-cascade-total-order

Conversation

@borskyj-symph

Copy link
Copy Markdown
Contributor

What

Fixes #464.

Viewport contexts were ordered with compareViewportContextCascade, which compares min-vs-min and max-vs-max by width and falls back to registry index for every other pair. That makes the comparator non-transitive. Array.prototype.sort is only well defined for a consistent comparator, so a registry holding both min-width and max-width contexts produced an order that depended on how the two kinds happened to interleave.

With [min-width: 1024px, max-width: 900px, min-width: 768px] the two min-width contexts are never compared to each other. The 1024px block is emitted before the 768px one, specificity is equal, source order decides, and the 768px rule wins at every viewport above 1024px. Mobile-first CSS silently inverts. The stored rule reads back exactly as authored, so nothing looks wrong until the page renders.

Correction to the issue

The issue reports this for a registry of two min-width contexts alone. That case is already correct on main, in both registry orders. The missing precondition is a max-width context sitting between them, which is what the reporting site's breakpoint set looks like (min-width: 1024px, max-width: 1023px, max-width: 900px). The reported symptom is real, the trigger in the repro steps is incomplete.

How

sortViewportContextCascade replaces the comparator. It partitions by query kind over registry order and sorts each kind group within the slots that group already occupies:

  • Cross-kind order stays registry order, which is what the existing doc comment already promised.
  • min ascending and max descending hold however the kinds interleave.
  • other (mixed or non-pixel queries, where width order is undefined) is never reordered.
  • Sorting by registry index first makes emission independent of contextStyles key order, which is authoring order rather than registry order.

Both call sites (createStyleRuleCssEmitter, resolveAutoSizes) move to the new function. Per CONTRIBUTING, the old export is removed rather than kept as a shim.

Tests

Three regression tests in src/__tests__/publisher/classStyleInjector.test.ts. All three fail on main and pass with the fix:

  • min-width contexts stay narrowest-first with a max-width context between them
  • max-width contexts stay widest-first with a min-width context between them
  • emission depends on the registry, not on contextStyles key order

The two existing cascade-order tests (pure max-width, pure min-width) are unchanged and still pass.

Deliberately not changed

Whether min-width and max-width groups should be ordered relative to each other by convention (mobile-first: all min ascending, then all max descending) rather than by registry order. That is a behaviour change beyond this bug and needs a maintainer call. This PR only makes the documented ordering actually hold.

Checks

bun run lint, bun run build, and bun test src/__tests__/publisher/ src/__tests__/canvas/ src/__tests__/page-tree/ (950 pass, 0 fail) all clean. The full bun test run has ~226 failures on this Windows checkout on main as well, all in collab/socket/SQLite-temp-file areas (EBUSY on cleanup); the set is unchanged by this PR.

…al one

Width order between viewport contexts is only defined within a query kind.
compareViewportContextCascade compared min-vs-min and max-vs-max by width and
fell back to registry index for every other pair, which makes it non-transitive.
Array.prototype.sort is only well defined for a consistent comparator, so a
registry mixing both kinds produced an order that depended on how the kinds
happened to interleave.

With [min-width: 1024px, max-width: 900px, min-width: 768px] the two min-width
contexts are never compared to each other. The 1024px block is emitted before
the 768px one, specificity is equal, and source order decides, so the 768px
rule wins at every viewport above 1024px. Mobile-first CSS silently inverts,
and the stored rule reads back exactly as authored, so nothing looks wrong
until the page renders.

Replace the comparator with sortViewportContextCascade, which partitions by
kind over registry order and sorts each kind group within the slots that group
already occupies. Cross-kind order stays registry order as documented, min and
max groups get their width order however the kinds interleave, and 'other'
(mixed or non-pixel queries, where width order is undefined) is never
reordered. Sorting by registry index first also makes emission independent of
contextStyles key order, which is authoring order rather than registry order.

Both call sites move to the new function.

Fixes CoreBunch#464

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.

[Bug]: min-width media queries are emitted largest-first, so narrower breakpoints override wider ones

2 participants