EPBDS-16211 F1 step 4: vendor the semantic CSS — output byte-identical - #45
Merged
Merged
Conversation
…yet green
NOT FOR MERGE. `css.semantic-parity.test.js` fails, and that is the honest state:
the step's own criterion is byte parity and this does not have it yet. Committed
so the work and the three traps found are not lost.
WHERE IT STANDS. `scripts/vendor-semantic-css.js` compiles `globals/reset` and
`modules/dropdown` into `src/style/vendor/*.less`, and `_semantic.less` imports
those instead of reaching into `node_modules`. The RULE SET is identical —
every rule in the pre-swap fixture is present and nothing new appears. What is
left is ordering, precisely characterised:
- 4 rules emitted TWICE, each as an adjacent pair with identical declarations
(indices 170/171, 208/209, 243/244, 256/257);
- everything from index 244 shifted by one as a consequence.
Both are confined to the tail, where our own `override/**.overrides` rules land.
Splitting the two `@import`s into separate `& { }` blocks, to match the original
shape exactly, does not change it.
THREE TRAPS FOUND, each caught by the gate rather than by reading, which is why
the gate was built first:
1. `.loadUIOverrides()` is NOT just our hook. `theme.less` defines it as two
optional imports, and the FIRST is the theme's own overrides — which for
`globals/reset` is where normalize.css lives. Stripping the mixin produced a
7-rule reset with the normalize body gone.
2. Redefining `@siteFolder` in the definition file to exclude our overrides does
nothing, silently: a LESS mixin resolves variables in the scope where it is
DEFINED, and that mixin is defined in `theme.less`. The gate caught it as a
duplication rather than as an error. Replaced by a LESS file manager that
declines our `override/**.overrides` — which must implement the SYNC path too,
or LESS takes a different route through the whole compile.
3. LESS re-evaluates what it parses, so vendored CSS is not inert. Semantic builds
`calc(100% + 2px)` from escaped strings precisely so LESS leaves it alone; as
a literal in a `.less` file it folded to `calc(102%)` — and 100% + 2px is not
102%. The generator now re-escapes `calc(...)` on the way out. The file has to
stay `.less`, because `:extend` in our own stylesheets must still reach these
rules.
Everything else stays green: 166 of 167 suites, 2,539 of 2,540 tests, and the
published `static/all.css` is unchanged at 307,785 bytes.
`src/style/vendor/semantic-reset.less` and `semantic-dropdown.less`, generated by
`scripts/vendor-semantic-css.js`. `_semantic.less` imports them instead of
reaching into `node_modules/semantic-ui-less`.
PARITY, IN TWO INDEPENDENT FORMS:
- the gate passes on all 285 rules — selector, declarations and cascade order;
- `static/all.css` has the SAME sha256 (dcdb0a40…) built from master and from
this branch. The artifact a consumer downloads does not change at all.
THE :EXTEND THREAD, since that is what this dig was about. Vendoring has to
produce a `.less` file, not `.css`: our own stylesheets extend Semantic's
selectors, and `@import (inline)` would put them out of `:extend`'s reach. That
constraint is what makes the rest of it delicate, because a `.less` file is
re-evaluated rather than copied.
FOUR TRAPS, each caught by the gate rather than by reading:
1. `.loadUIOverrides()` is not just our hook. `theme.less` defines it as two
optional imports and the FIRST is the theme's own overrides — which for
`globals/reset` is where normalize.css lives. Stripping it produced a 7-rule
reset with the normalize body gone.
2. Redefining `@siteFolder` to exclude our overrides does nothing, SILENTLY: a
LESS mixin resolves variables in the scope where it is DEFINED, and that mixin
is defined in `theme.less`. Replaced by a LESS file manager — which must
implement the SYNC path too, or LESS takes a different route through the whole
compile and folds `calc()` differently.
3. LESS re-evaluates what it parses. Semantic builds `calc(100% + 2px)` from
escaped strings precisely so LESS leaves it alone; as a plain literal in a
`.less` file it folded to `calc(102%)` — and 100% + 2px is not 102%. The
generator re-escapes every `calc(...)` on the way out.
4. Our `override/**.overrides` must be imported with `@import (multiple)`, the
form `loadUIOverrides` itself used. With a plain `@import`, the `.outline()`
mixin's nested `.inverted &` block emitted twice and duplicated two rules.
AND TWO FALSE ALARMS FROM THE GATE ITSELF, recorded because both looked exactly
like product bugs and cost real time:
- The generator detects live imports by line shape and did not recognise
`@import (multiple) "..."`. The baseline compile then kept our overrides while
the full compile also had them, so 47 rules subtracted themselves away and the
gate reported them MISSING FROM THE OUTPUT when they were only missing from the
MEASUREMENT. The detector now tolerates import options.
- Two of four apparent "duplicates" occur twice on master as well. Measuring a
suspicious count against the same count on master, before theorising about a
cause, would have saved the detour.
Gates: jsdom 167 suites / 2540 tests / 38 snapshots on React 16.14, 17 and 18.3;
Chrome 41/41; `lint:css`, `css:fixture:check` green; published CSS byte-identical.
CI caught what my own check missed. `lint:css` globs `src/style/**/*.less`, so the vendored files landed in it and reported 2,295 violations of conventions upstream never followed — and which cannot be fixed without breaking the byte-for-byte parity `css.semantic-parity.test.js` exists to enforce. The miss on my side is worth naming: I checked lint with `npm run lint:css | tail -1`, which shows the last line of output and hides the exit code. A pipeline's success is the last command's success, so the check could not fail. Gates are verified by exit code now. `.stylelintignore` rather than a narrower glob, because the reason is about the FILES, not the pattern: generated third-party CSS is not a place where style decisions are made. Ours are, and they stay linted.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
src/style/vendor/semantic-reset.lessandsemantic-dropdown.less, generated byscripts/vendor-semantic-css.js._semantic.lessimports them instead of reaching intonode_modules/semantic-ui-less.Parity, in two independent forms
static/all.csshas the same sha256 (dcdb0a40…) built from master and from this branch. The artifact a consumer downloads does not change at all.The
:extendthread, since that is what this dig was aboutVendoring has to produce a
.lessfile, not.css: our own stylesheets extend Semantic's selectors, and@import (inline)would put them out of:extend's reach. That constraint is what makes the rest delicate — a.lessfile is re-evaluated, not copied.Four traps, each caught by the gate rather than by reading
.loadUIOverrides()is not just our hook.theme.lessdefines it as two optional imports, and the first is the theme's own overrides — which forglobals/resetis where normalize.css lives. Stripping it produced a 7-rule reset with the normalize body gone.@siteFolderto exclude our overrides does nothing, silently. A LESS mixin resolves variables in the scope where it is defined, and that mixin is defined intheme.less. Replaced by a LESS file manager — which must implement the sync path too, or LESS takes a different route through the whole compile and foldscalc()differently.calc(100% + 2px)from escaped strings precisely so LESS leaves it alone; as a plain literal in a.lessfile it folded tocalc(102%)— and 100% + 2px is not 102%. The generator re-escapes everycalc(...)on the way out.override/**.overridesmust use@import (multiple)— the formloadUIOverridesitself used. With a plain@import, the.outline()mixin's nested.inverted &block emitted twice and duplicated two rules.Two false alarms from the gate itself
Both looked exactly like product bugs and cost real time, so both are now comments in the generator:
@import (multiple) "...". The baseline compile then kept our overrides while the full compile also had them — so 47 rules subtracted themselves away and the gate reported them missing from the output when they were only missing from the measurement.A gate that measures by difference needs its difference to be exact. That is the durable lesson here, and it is worth more than either fix.
What is left of step 4
Deleting
semantic-ui-lessand itstheme.configmachinery: three webpack aliases (the plan names two —webpack.watch.config.mjsis the third), thenode_modulescopy inscripts/install-theme-config.jsand its three call sites, and the now-unreferencedoverride/tree.Gates
jsdom 167 suites / 2540 tests / 38 snapshots on React 16.14, 17 and 18.3; Chrome 41/41;
lint:cssandcss:fixture:checkgreen; published CSS byte-identical.🤖 Generated with Claude Code