From 5924ea4a705e898e1d32d80ec53636fd86f09f38 Mon Sep 17 00:00:00 2001 From: borskyj Date: Wed, 2 Sep 2026 19:46:00 +0200 Subject: [PATCH] fix(publisher): order viewport contexts by a total order, not a partial 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 #464 Co-Authored-By: Claude Opus 5 --- .../publisher/classStyleInjector.test.ts | 78 +++++++++++++++++++ src/core/publisher/classCss.ts | 62 ++++++++++++--- src/core/publisher/sizesResolver.ts | 9 +-- 3 files changed, 132 insertions(+), 17 deletions(-) diff --git a/src/__tests__/publisher/classStyleInjector.test.ts b/src/__tests__/publisher/classStyleInjector.test.ts index 4b5f5efc2..f7620ee81 100644 --- a/src/__tests__/publisher/classStyleInjector.test.ts +++ b/src/__tests__/publisher/classStyleInjector.test.ts @@ -662,6 +662,84 @@ describe('generateClassCSS', () => { expect(desktopIdx).toBeGreaterThan(tabletIdx) }) + it('keeps min-width contexts narrowest-first when a max-width context sits between them', () => { + // Regression: width order is only defined between two contexts of the same + // query kind, so ordering used to fall back to registry index for a + // min-vs-max pair. That made the comparison non-transitive — the two + // min-width contexts below are never compared to each other, and the + // 1024px block was emitted before the 768px one. Both match above 1024px, + // specificity is equal, so the 768px block won and mobile-first inverted. + const breakpoints = [ + { id: 'desktop', width: 1024, mediaQuery: '(min-width: 1024px)' }, + { id: 'mobile', width: 900, mediaQuery: '(max-width: 900px)' }, + { id: 'tablet', width: 768, mediaQuery: '(min-width: 768px)' }, + ] + const classes = { + hero: makeClass('hero', { color: 'rgb(1, 2, 3)' }, { + desktop: { color: 'rgb(13, 14, 15)' }, + mobile: { color: 'rgb(7, 8, 9)' }, + tablet: { color: 'rgb(10, 11, 12)' }, + }), + } + const css = generateClassCSS(classes, breakpoints) + const tabletIdx = css.indexOf('@media (min-width: 768px)') + const desktopIdx = css.indexOf('@media (min-width: 1024px)') + expect(tabletIdx).toBeGreaterThanOrEqual(0) + expect(desktopIdx).toBeGreaterThan(tabletIdx) + }) + + it('keeps max-width contexts widest-first when a min-width context sits between them', () => { + const breakpoints = [ + { id: 'narrow', width: 600, mediaQuery: '(max-width: 600px)' }, + { id: 'desktop', width: 1024, mediaQuery: '(min-width: 1024px)' }, + { id: 'wide', width: 900, mediaQuery: '(max-width: 900px)' }, + ] + const classes = { + hero: makeClass('hero', { color: 'rgb(1, 2, 3)' }, { + narrow: { color: 'rgb(4, 5, 6)' }, + desktop: { color: 'rgb(13, 14, 15)' }, + wide: { color: 'rgb(7, 8, 9)' }, + }), + } + const css = generateClassCSS(classes, breakpoints) + const wideIdx = css.indexOf('@media (max-width: 900px)') + const narrowIdx = css.indexOf('@media (max-width: 600px)') + expect(wideIdx).toBeGreaterThanOrEqual(0) + expect(narrowIdx).toBeGreaterThan(wideIdx) + }) + + it('orders viewport contexts by the registry, not by contextStyles key order', () => { + // `contextStyles` is keyed in authoring order, which is not registry order. + // Emission must not depend on it: the same registry and the same overrides + // produce the same CSS whichever order the keys were written in. + const breakpoints = [ + { id: 'desktop', width: 1024, mediaQuery: '(min-width: 1024px)' }, + { id: 'mid', width: 1023, mediaQuery: '(max-width: 1023px)' }, + { id: 'tablet', width: 768, mediaQuery: '(min-width: 768px)' }, + ] + const overrides = { + desktop: { color: 'rgb(13, 14, 15)' }, + mid: { color: 'rgb(7, 8, 9)' }, + tablet: { color: 'rgb(10, 11, 12)' }, + } + const authoredOneWay = generateClassCSS( + { hero: makeClass('hero', { color: 'rgb(1, 2, 3)' }, overrides) }, + breakpoints, + ) + const authoredAnother = generateClassCSS( + { hero: makeClass('hero', { color: 'rgb(1, 2, 3)' }, { + tablet: overrides.tablet, + desktop: overrides.desktop, + mid: overrides.mid, + }) }, + breakpoints, + ) + expect(authoredAnother).toBe(authoredOneWay) + expect(authoredOneWay.indexOf('@media (min-width: 1024px)')).toBeGreaterThan( + authoredOneWay.indexOf('@media (min-width: 768px)'), + ) + }) + it('rewrites class background images with responsive image-set candidates', () => { const classes = { hero: makeClass('hero', { backgroundImage: "url('/uploads/hero.png')" }), diff --git a/src/core/publisher/classCss.ts b/src/core/publisher/classCss.ts index 87321ea25..6c2bbaa7e 100644 --- a/src/core/publisher/classCss.ts +++ b/src/core/publisher/classCss.ts @@ -270,6 +270,11 @@ export function bagToReactStyle( * narrowest matching query wins. Pure min-width contexts emit narrowest first * so the widest matching query wins. Mixed/custom viewport queries keep the * user's registry order. + * + * Those two width orderings are *within* a query kind: a registry holding both + * `min-width` and `max-width` contexts keeps the two groups in registry order + * and sorts each group by width in its own slots. See + * `sortViewportContextCascade`. */ export interface ViewportContext { id: string @@ -296,15 +301,49 @@ function viewportQuerySort(breakpoint: ViewportContext): ViewportQuerySort { return { kind: 'other', width: breakpoint.width } } -export function compareViewportContextCascade( - a: { breakpoint: ViewportContext; index: number }, - b: { breakpoint: ViewportContext; index: number }, -): number { - const aQuery = viewportQuerySort(a.breakpoint) - const bQuery = viewportQuerySort(b.breakpoint) - if (aQuery.kind === 'max' && bQuery.kind === 'max') return bQuery.width - aQuery.width - if (aQuery.kind === 'min' && bQuery.kind === 'min') return aQuery.width - bQuery.width - return a.index - b.index +/** + * Order viewport contexts for emission. + * + * This cannot be a plain `Array.prototype.sort` comparator. Width order is only + * meaningful between two contexts of the same query kind — comparing a + * `min-width` against a `max-width` has no answer, and falling back to registry + * index for those pairs makes the comparator non-transitive: with + * `[min-width: 1024px, max-width: 900px, min-width: 768px]` the two min-width + * contexts never get compared to each other, so the 1024px block is emitted + * before the 768px one and wins the equal-specificity tie at every viewport + * above 1024px. Mobile-first CSS silently inverts. + * + * Instead, partition by kind over registry order and sort each kind group + * within the slots that group already occupies. Cross-kind order stays registry + * order, `min`/`max` groups get their width order regardless of how the kinds + * interleave, and `other` (mixed or non-pixel queries, where width order is not + * defined) is never reordered. + */ +export function sortViewportContextCascade( + entries: readonly T[], +): T[] { + // Registry order first, so the result never depends on the caller's array + // order (`contextStyles` key order is authoring order, not registry order). + const ordered = entries.slice().sort((a, b) => a.index - b.index) + + for (const kind of ['min', 'max'] as const) { + const slots: number[] = [] + for (let i = 0; i < ordered.length; i++) { + if (viewportQuerySort(ordered[i].breakpoint).kind === kind) slots.push(i) + } + if (slots.length < 2) continue + + const group = slots.map((slot) => ordered[slot]) + group.sort((a, b) => { + const aWidth = viewportQuerySort(a.breakpoint).width + const bWidth = viewportQuerySort(b.breakpoint).width + if (aWidth !== bWidth) return kind === 'min' ? aWidth - bWidth : bWidth - aWidth + return a.index - b.index + }) + slots.forEach((slot, i) => { ordered[slot] = group[i] }) + } + + return ordered } /** @@ -318,7 +357,7 @@ export function compareViewportContextCascade( * between what the editor shows and what a publish ships. * * Cascade order (precedence Q-A): base → custom conditions (registry order) → - * viewport @media contexts (see `compareViewportContextCascade`). Context keys + * viewport @media contexts (see `sortViewportContextCascade`). Context keys * matching neither registry are skipped (orphaned overrides). */ export interface StyleRuleDeclarationLayers { @@ -390,8 +429,7 @@ export function createStyleRuleCssEmitter( blocks.push(`${prelude} {\n ${selector} {\n${decls}\n }\n}`) } - bpEntries.sort(compareViewportContextCascade) - for (const { contextId, bag, breakpoint } of bpEntries) { + for (const { contextId, bag, breakpoint } of sortViewportContextCascade(bpEntries)) { const decls = bagToCSS(bag, options, layers.contextStylePriorities?.[contextId]) if (!decls) continue const prelude = conditionPrelude({ kind: 'media', query: breakpointMediaQuery(breakpoint) }) diff --git a/src/core/publisher/sizesResolver.ts b/src/core/publisher/sizesResolver.ts index 5856b2482..5c1cfd412 100644 --- a/src/core/publisher/sizesResolver.ts +++ b/src/core/publisher/sizesResolver.ts @@ -62,7 +62,7 @@ * tier — the caller falls back to `100vw`. */ import { breakpointMediaQuery, type Page, type PageNode, type SiteDocument } from '@core/page-tree' -import { compareViewportContextCascade } from './classCss' +import { sortViewportContextCascade } from './classCss' // --------------------------------------------------------------------------- // Linear width candidates @@ -491,10 +491,9 @@ export function resolveAutoSizes( // Viewport tiers in `sizes` first-match order — the reverse of the CSS // cascade, so the candidate that would win in CSS is hit first. - const tiers = site.breakpoints - .map((breakpoint, index) => ({ breakpoint, index })) - .sort(compareViewportContextCascade) - .reverse() + const tiers = sortViewportContextCascade( + site.breakpoints.map((breakpoint, index) => ({ breakpoint, index })), + ).reverse() const entries: Array<{ query: string | null; value: string }> = [] for (const { breakpoint } of tiers) {