Conversation
Contributor
Coverage Report
File CoverageNo changed files found. |
enyo
added this pull request to stack #2364
September 12, 2026 17:56
enyo
force-pushed
the
plain-css
branch
2 times, most recently
from
September 13, 2026 10:16
1cc3d3f to
64393d4
Compare
The two stylesheets used sass for nesting, a handful of variables and one math.div call. None of that needs a compiler: the sources are now the flattened CSS sass was producing anyway, and sass is gone from the package. Flattened rather than nested. Native CSS nesting only reached browsers in 2023, well above the range a library targeting es2017 and supporting everything except Internet Explorer can assume. Lightning CSS minifies them. It is the more thorough of the minifiers tried: it drops a `background: #999` fallback that no browser in range needs, shortens `transition: opacity .3s ease` to `.3s` because ease is the default, and removes a dead `display: block` that was being overridden on the very next line -- which is in the scss too, and is removed at the source here. The supported browsers are declared once, as a browserslist field in package.json, and the build reads them with --browserslist rather than repeating a list of versions inside a script. That range is the one the es2017 JavaScript target implies, and it is load-bearing: given a modern range instead, Lightning CSS rewrites every rgba() into hex-with-alpha, which is newer syntax than this library claims to support. Checked by widening the field and watching the output change. The output was compared declaration by declaration against what sass produced: 175 effective declarations before and after in dropzone.css, 33 in basic.css, no difference once value-level equivalences are normalised. watch-css is removed rather than ported. It existed because sass had a --watch flag; lightningcss has none, nothing referenced the script, and a script called watch-css that does not watch is worse than no script. src/.gitignore is deleted with the stylesheets. It ignored *.css and *.map next to the sources, which was harmless while they were sass -- and silently swallowed both new files the moment .css became the source, including hiding them from the formatter.
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
1 participant
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.
Stacked on #2361. The two stylesheets used sass for nesting, a handful of variables and one
math.divcall. None of that needs a compiler.sassis gone from the package. The sources are now the flattened CSS sass was producing anyway.Flattened, not nested
Native CSS nesting only reached browsers in 2023 — well above the range a library targeting
es2017and supporting everything except Internet Explorer can assume. So the nesting is resolved rather than translated.esbuild minifies, with an explicit target
It was already in the tree as vite's own dependency, so this trades one direct dependency for another rather than adding one — and
sassis by far the larger install.The target is pinned to
chrome58,firefox54,safari11,edge16, matching what thees2017JS build implies. That is not cosmetic: without it esbuild rewrites everyrgba()to hex-with-alpha (rgba(0,0,0,.9)→#000000e6), which is syntax newer than the library claims to support. With the target set,rgba()survives and the only hex left is a plain six-digit one.Verified declaration by declaration
Not eyeballed — I parsed both the old sass output and the new esbuild output into
(selector, property, value)sets and compared them.dropzone.csshas the same 177 declarations before and after. Every difference is a value-level equivalence esbuild applies:padding: 20px 20pxpadding: 20pxtranslateY(0px)translateY(0)rgba(0, 0, 0, 0)transparent300ms.3sscale(1.05, 1.05)scale(1.05)100% { }in keyframesto { }basic.cssgoes from 35 declarations to 33. Both losses are aleft: 30pxthat aleft: 50%three lines later in the same block already overrode — dead in the original too.252 unit tests, 3 library end-to-end specs and the 7 website specs all pass; the website consumes
dist/dropzone.cssdirectly, so it exercises the new output.One thing I got wrong first
The first push of this branch deleted both
.scssfiles and added neither.cssfile.packages/dropzone/src/.gitignorecontained*.cssand*.map, left from when sass compiled next to the sources — harmless while the stylesheets were sass, and silently swallowing both new files the moment.cssbecame the source.git statusshowed the deletions and simply never mentioned the additions.That file is deleted here too. I checked the fix by extracting the commit with
git archiveinto a clean directory and building from that alone: the stylesheets are present and the output is byte-identical to the working-tree build.Not done here
Moving the variables to CSS custom properties would let users retheme without rebuilding — a genuine improvement, but a feature and a new public surface, so it does not belong in a conversion whose whole claim is that nothing changed.