Repository navigation
Conversation
Chromium matches min-/max- width features within 1/64px, so the generated `min-width: X.01px` (from `not (max-width: X)` and from `(width > X)` / `(X < width ...)`) still matches at X, and `(width < X)` -> `max-width: X-0.01px` still matches at X too. Use the 0.02px offset that meta#1858 added for the max-width branch for every exclusive px width bound. The adjusted bound never moves past a whole pixel, so a breakpoint written just off one still lands on it: `not (max-width: 599.99px)` stays `min-width: 600px`, and `not (min-width: 600.01px)` now gives `max-width: 600px`. The result is rounded to 4 decimals to drop float noise. Fixes meta#1859 Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Hi @Feldt! Thank you for your pull request and welcome to our community. Action RequiredIn order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you. ProcessIn order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA. Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks! |
|
@Feldt is attempting to deploy a commit to the Meta Open Source Team on Vercel. A member of the Team first needs to authorize it. |
What changed / motivation ?
StyleX turns exclusive width bounds into
min-width/max-widthwith a0.01px offset. Chromium compares
min-/max-features with 1/64px(about 0.0156px) of slack (
CompareDoubleValueinmedia_query_evaluator.ccuses
LayoutUnit::Epsilon()), so these bounds still match at the breakpointthey are meant to exclude:
max-width: 900px, thenmax-width: 600px)generate
(min-width: 600.01px) and (max-width: 900px). That still matchesat 600px and wins over
(max-width: 600px)there.(width > 600px)becomesmin-width: 600.01px, which matches at 600px.(width < 600px)becomesmax-width: 599.99px, which matches at 600px.(620px < width <= 900px)from Strict media range loses its excluded boundary in StyleX unplugin/Vite production CSS #1859 becomes(min-width: 620.01px) and (max-width: 900px), which matches at 620px. The620.01pxin the issue comes from StyleX itself (adjustDimension).#1858 fixed the mirror case (
not (min-width: X)->max-width: X - 0.02px).This PR moves the offset into one helper,
exclusiveBoundValue, used byadjustDimension(range syntax) and by both negated branches ofmergeIntervalsForAnd, so every exclusive px width bound moves by 0.02px.The helper has two more rules, so output that was already right stays the same:
pixel keeps landing on it, as on
main:not (max-width: 599.99px)and(width > 599.99px)givemin-width: 600px, and(width < 600.01px)givesmax-width: 600px. Without this rule, 0.02px would push them to600.01pxand
599.99px.transform-value.jsdoes fornumbers, so
(width > 0.1px)gives0.12px, not0.12000000000000001px.The whole-pixel rule also moves a few bounds that
mainpushed past a pixelback onto it. Mobile-first
min-width: 400pxthenmin-width: 600.01pxnowgives
(min-width: 400px) and (max-width: 600px)instead of599.99px.(width > 599.98px)and(width < 600.02px)now give600pxinstead of599.99pxand600.01px.Heights and non-px units keep 0.01, as in #1858. Chromium has the same slack
for height (
(height >= 400.01px)matches in a 400px-tall viewport). I leftheight out because #1859 and #1858 are both about width, and changing it
would change more generated class names. Covering px heights too means
dropping the
dim === 'width'check and updating the height expectations. Ican add it here if you prefer.
Linked PR/Issues
Fixes #1859
Related: #1763 and #1858 (the same Chromium tolerance, max-width side).
Additional Context
Tests:
media-query-transform-test.js, plus(width < 600px),(width > 600px)and
(620px < width <= 900px)inparse-media-query-test.js. Onmainthey fail (they get
600.01px,599.99pxand620.01px).599.99pxbreakpoint, mobile-first with a
600.01pxbreakpoint,(width > 599.99px)and
(width < 600.01px). One more,(width > 0.1px), covers the rounding.The existing
not (max-width: 99.99px)tests are unchanged and stillexpect
min-width: 100px.X.01pxtoX.02px, and fromX.99pxtoX.98pxfor strict ranges. The class namesin one babel-plugin snapshot change with them.
Browser check, in local headless Chromium 151 and WebKit 26: I compiled the
styles with the built babel plugin and loaded the CSS at widths 599, 600, 601,
619, 620 and 621. Measured against what the spec gives for the authored
queries,
mainis wrong at 4 of these points in each engine. With thischange it is wrong at none.
matchMediaat 600px in Chromium:(min-width: 600.015625px)is true and(min-width: 600.0157px)is false,which is the 1/64px slack.
(min-width: 600.01px)and(max-width: 599.99px)are true, and the
600.02pxand599.98pxversions are false.Breakpoints written just off a pixel (
599.99px,600.01px) are read witheach engine's own slack, so at 600px the engines do not always agree with the
spec. For these, the output in both engines is the same as on
main, withone exception: mobile-first
600.01pxat 600px in WebKit. WebKit matchesmin-width: 600.01pxat 600px, somainshowed the600.01pxrule there.This change shows the
400pxrule, which is what the spec gives.Firefox was not tested (it is not installed here).
Local results on Node 22 with yarn 1.22.22:
yarn install --frozen-lockfile(which builds every package),
yarn flow,yarn test:packages, thedevtools-extension build checks,
yarn prettier:reportandyarn lint:reportpass. The spelling job (typos) was not run locally.
Breaking change: none in the API. The generated CSS for these queries changes
by 0.01px, so their class names change.
Test command:
yarn test. For just the parser tests:cd packages/style-value-parser && npx jest src/at-queries.This PR was prepared with an AI coding agent (Claude Code). The submitter is
responsible for it and will answer review.
Pre-flight checklist
Contribution Guidelines
🤖 Generated with Claude Code