diff --git a/CHANGELOG.md b/CHANGELOG.md index f7f2c4f..ccd4b69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,17 @@ All notable changes to CSS Dedup are documented in this file, which is (mostly) The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), and the project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [1.13.0] - 2026-08-24 + +### Fixed + +* Corrected the `--fix` example output in the README + +### Changed + +* Changed `--savings-only` (`savingsOnly`) to decide per merge instead of per file, so a style sheet keeps the consolidations that save bytes even when others in it would cost more than those save + - Adjusted `withheld: { count, bytes }` to report the merges that were declined and what applying them, too, would have cost, rather than the whole file’s consolidation + ## [1.12.1] - 2026-08-13 ### Changed diff --git a/README.md b/README.md index 74811a9..de1e35e 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ Findings -f (-a) Savings with: -f -f -s -f -a -f -a -s Legend: -f: --fix, -a: --aggressive, -s: --savings-only ``` -(A cell reads `n/a` when no findings would be safe to apply, or if `--savings-only` would decline the merge for growing the file.) +(A cell reads `n/a` when no findings would be safe to apply, or if `--savings-only` would decline every merge on offer for growing the file.) Running with `--fix` folds `.a` and `.b` into a single rule for the shared declaration: @@ -50,15 +50,15 @@ Running with `--fix` folds `.a` and `.b` into a single rule for the shared decla ```shell $ npx css-dedup --fix default.css -1 consolidated, 0 skipped -63 → 53 bytes (-10 B, -15.9%) -Wrote default.css +Summary: +* 1 declaration consolidated: Reduced duplication and saved 10 bytes (64 → 54 bytes, -15.6%) +* Wrote default.css ``` Since duplicate declarations cost bytes wherever they live—in the style sheet itself, and (uncompressed) over the wire—the byte counts reflect two payoffs at once: less to maintain, and less to transfer. -The two aren’t always aligned, though: Folding a declaration into a shared selector list adds that list’s bytes back, so consolidating one that only has a couple of long, otherwise-unrelated selectors in common can end up costing more than it removes. CSS Dedup’s two modes call this out, so it’s a decision you can make consciously—it’s worth it if you value using declarations once for maintainability, yet not if you’re optimizing purely for transfer size. (`--fix --savings-only` automates that call: A file whose consolidation would grow it is left untouched.) +The two aren’t always aligned, though: Folding a declaration into a shared selector list adds that list’s bytes back, so consolidating one that only has a couple of long, otherwise-unrelated selectors in common can end up costing more than it removes. CSS Dedup’s two modes call this out, so it’s a decision you can make consciously—it’s worth it if you value using declarations once for maintainability, yet not if you’re optimizing purely for transfer size. (`--fix --savings-only` automates that call, merge by merge: Every consolidation that would grow the style sheet is left out, and the ones that pay for themselves still apply.) ## Usage @@ -76,7 +76,7 @@ The input is CSS. A preprocessor source named as an argument (.scss, .sass, .les | --- | --- | | `--fix`, `-f` | Consolidate declarations that are safe to merge automatically, rewriting each file in place (or printing to STDOUT for `-`) | | `--aggressive`, `-a` | Also apply merges that are probably—but not provably—safe (see [aggressive mode](#aggressive-mode)); only applies together with `--fix`, since report mode’s table already previews both variants automatically | -| `--savings-only`, `-s` | Leave a file untouched when its consolidation would make it bigger, not smaller (checked per file); only applies together with `--fix`, since report mode doesn’t write | +| `--savings-only`, `-s` | Leave out each consolidation that would make the file bigger rather than smaller, keeping the ones that save bytes (checked per merge); only applies together with `--fix`, since report mode doesn’t write | | `--ignore-selector `, `-i` | Regular expression for selectors to exclude from analysis (repeatable) | | `--no-ignore-selectors-defaults`, `-n` | Disable the built-in selector hack ignore list | | `--ignore-path `, `-p` | Regular expression tested against each file’s path, relative to the working directory; a match excludes the file (repeatable) | @@ -112,7 +112,7 @@ These are the supported options with their defaults (each can be omitted): // css-dedup.config.js export default { aggressive: false, // set to `true` to also allow probably-safe merges - savingsOnly: false, // set to `true` to skip files whose consolidation would grow them (`--fix` runs only) + savingsOnly: false, // set to `true` to leave out the individual merges that would grow the file (`--fix` runs only) ignoreSelectors: [], // additional selector patterns to exclude, e.g., `[/^\.legacy-/]` ignoreSelectorsDefaults: true, // set to `false` to disable the built-in hack list ignorePaths: [], // file paths to exclude, matched relative to the working directory, e.g., `[/dist\//]` @@ -141,7 +141,7 @@ Both functions accept an options object: { from: 'path/to/file.css', // forwarded to PostCSS; names the file in syntax-error messages aggressive: false, // set to `true` to also allow probably-safe merges - savingsOnly: false, // set to `true` to withhold a consolidation that would grow the style sheet (`dedup()` only) + savingsOnly: false, // set to `true` to withhold each merge that would grow the style sheet (`dedup()` only) ignoreSelectors: [/^\.legacy-/], // additional selector patterns to exclude ignoreSelectorsDefaults: true // set to `false` to disable the built-in hack list } @@ -162,9 +162,9 @@ Both functions accept an options object: `dedup()` returns `{ css, applied, skipped, bytes }`: `css` is the rewritten style sheet; `applied` lists what it did—each entry has `redundant: true` if it just dropped a same-rule (or same-at-rule-block) duplicate, `folded: true` if it folded a rule repeating the same selector into a later one, absent if it folded selectors from separate rules into one; `skipped` lists duplicate groups (and blocked same-selector folds) it left untouched along with why; and `bytes` is `{ before, after, saved }`—UTF-8 byte counts of the style sheet before and after, since that’s what changes over the wire, not the character count, covering everything `--fix` did as one net figure. -`saved` is `before - after`, so it’s negative on the rare file where the added selector-list text outweighs the removed declarations—dropping a same-rule duplicate never costs bytes, only folding selectors from separate rules can. With `savingsOnly: true`, a consolidation whose net `saved` would be negative is withheld: `css` comes back untouched, `applied` is empty, `bytes` reports no change (that’s what actually happened), and the declined outcome arrives as `withheld: { count, bytes }`—the number of merges and the byte counts the consolidation would have had (`withheld` is absent whenever nothing was withheld). `dedupRoot()` (the same function, operating on an already-parsed PostCSS root instead of a CSS string) returns the same shape minus `css`. +`saved` is `before - after`, so it’s negative on the rare file where the added selector-list text outweighs the removed declarations—dropping a same-rule duplicate never costs bytes, only folding selectors from separate rules can. With `savingsOnly: true`, each merge that would make `saved` more negative is withheld on its own, leaving the rest in place: `css` comes back with whatever paid for itself applied, `bytes` reports what actually happened, and the declined merges arrive as `withheld: { count, bytes }`—how many, and the byte counts the style sheet would have had with them applied too (`withheld` is absent whenever nothing was withheld). A file offering nothing but growing merges therefore still comes back untouched, with `applied` empty; one that mixes them keeps its savings. `dedupRoot()` (the same function, operating on an already-parsed PostCSS root instead of a CSS string) returns the same shape minus `css`. -`dedup()` additionally returns `sourceMapStale: true` when the style sheet passed in carried a `/*# sourceMappingURL=… */` comment _and_ the run actually rewrote it—the CLI’s source map caveat in machine-readable form, so a wrapper can surface it instead of shipping a broken map unnoticed. It’s absent otherwise, including when nothing was applied or the consolidation was withheld, since the style sheet (and hence the map’s accuracy) is then unchanged. `dedupRoot()` doesn’t report it, deliberately: It’s what [the PostCSS plugin](#postcss-plugin) calls, and that path stringifies through PostCSS, which emits a correct map of its own. +`dedup()` additionally returns `sourceMapStale: true` when the style sheet passed in carried a `/*# sourceMappingURL=… */` comment _and_ the run actually rewrote it—the CLI’s source map caveat in machine-readable form, so a wrapper can surface it instead of shipping a broken map unnoticed. It’s absent otherwise, including when nothing was applied or every merge was withheld, since the style sheet (and hence the map’s accuracy) is then unchanged. `dedupRoot()` doesn’t report it, deliberately: It’s what [the PostCSS plugin](#postcss-plugin) calls, and that path stringifies through PostCSS, which emits a correct map of its own. ### PostCSS Plugin @@ -183,7 +183,7 @@ const fixed = await postcss([cssdedup({ fix: true })]).process(css, { from: 'def console.log(fixed.css); ``` -The plugin takes the same options as `analyze()`/`dedup()`, plus `fix: true` to switch it into consolidation mode (`aggressive: true` and `savingsOnly: true` work here, too—a withheld consolidation leaves the root untouched and surfaces as a warning). Since CSS Dedup is a source-hygiene tool—more like `stylelint --fix` than a bundle optimizer—it fits well early in a pipeline, on hand-authored CSS, before Autoprefixer and before minification; running it after either may overlap with work those tools do. (CSS Dedup also works as a last step, however, as with applying `--savings-only` optimizations to minified files.) +The plugin takes the same options as `analyze()`/`dedup()`, plus `fix: true` to switch it into consolidation mode (`aggressive: true` and `savingsOnly: true` work here, too—withheld merges are left out of the root and surface as a warning). Since CSS Dedup is a source-hygiene tool—more like `stylelint --fix` than a bundle optimizer—it fits well early in a pipeline, on hand-authored CSS, before Autoprefixer and before minification; running it after either may overlap with work those tools do. (CSS Dedup also works as a last step, however, as with applying `--savings-only` optimizations to minified files.) ## How It Works @@ -227,6 +227,7 @@ CSS Dedup: - If a merged rule (including the last occurrence itself) also carries a declaration for an overlapping property, that declaration is split out into its own small rule—keeping that occurrence’s own, original selector—placed right after the merged rule, rather than blocking the merge outright: Folding every selector onto one shared declaration block would otherwise hand that overlapping extra to selectors that never had it. Exception: If that extra is itself duplicated elsewhere in the same scope, it’s left alone and the whole merge is skipped instead, since splitting it here would orphan that other duplicate’s own merge. - If something does block it, the merge is skipped and reported rather than risking a cascade change. A blocker fences, though—it doesn’t forbid: Occurrences on the same side of it still consolidate among themselves (their own spans are clean, so the same safety argument applies), and the group is reported as skipped either way, since the duplicate keeps existing across the blocker. - Consolidation runs to a fixed point: One merge can unblock or create another (a fresh merged rule may repeat an existing rule’s selector list, an emptied rule stops fencing a span), so the passes repeat until nothing changes. + - With `--savings-only`, each merge is weighed before it stands: The consolidation is performed, the style sheet measured, and the merge undone again if it turned out to cost bytes. The unit is the smallest one that can be applied alone—usually a single duplicate group, each run separately where a blocker splits one, and a whole cluster where groups share a rule. The strategies that can’t cost bytes (collapsing a repeat within one rule, folding two same-selector rules) aren’t weighed. Measuring the real thing beats predicting it—a merge that empties a rule or a conditional block saves far more than its declaration arithmetic suggests. Overall, CSS Dedup is conservative by design and will leave some safe merges for manual review. @@ -282,7 +283,7 @@ src/ index.js Public API (re-exports only) plugin.js PostCSS plugin wrapper analyze.js Read-only detection (step 5 above) - consolidate.js The `savingsOnly` gate and the fixed-point loop + consolidate.js The fixed-point loop and the result shape merge.js The merge strategies (step 6 above) lib/ Engine internals @@ -294,6 +295,7 @@ src/ shorthands.js Shorthand/longhand overlap hacks.js The default selector ignore list (step 3) caches.js Per-run memoization lifecycle + transaction.js Snapshot/rollback, for weighing one merge’s bytes util.js Shared helpers cli/ CLI internals diff --git a/package-lock.json b/package-lock.json index 336c6b6..d55638a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "css-dedup", - "version": "1.12.1", + "version": "1.13.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "css-dedup", - "version": "1.12.1", + "version": "1.13.0", "license": "MIT", "dependencies": { "postcss": "^8.5.26" @@ -16,7 +16,7 @@ }, "devDependencies": { "@eslint/js": "^10.0.1", - "eslint": "^10.8.1", + "eslint": "^10.9.0", "globals": "^17.11.0", "typescript": "^7.0.2" }, @@ -678,9 +678,9 @@ } }, "node_modules/eslint": { - "version": "10.8.1", - "resolved": "https://registry.npmjs.org/eslint/-/eslint-10.8.1.tgz", - "integrity": "sha512-wqA7W2jbsC/BnV9Iv1UZpKVFkO1AdNoSmYW8NWG4HNOBbkAMvIqDZ27pI2f07dqn583NcIC44ckjAcOXDL1QbQ==", + "version": "10.9.0", + "resolved": "https://registry.npmjs.org/eslint/-/eslint-10.9.0.tgz", + "integrity": "sha512-5KeEOJZBfEVA47boFiBsf+6MmmJpffM7qEBg4pLla2e4nlKgdKlqCW0oSLOGsT8Wl5uCGJptLV1bkaiShj90Gw==", "dev": true, "license": "MIT", "workspaces": [ diff --git a/package.json b/package.json index db0f58c..5b38974 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "description": "CSS declaration deduplicator for maintainability and performance optimization", "devDependencies": { "@eslint/js": "^10.0.1", - "eslint": "^10.8.1", + "eslint": "^10.9.0", "globals": "^17.11.0", "typescript": "^7.0.2" }, @@ -60,5 +60,5 @@ }, "type": "module", "types": "src/index.d.ts", - "version": "1.12.1" + "version": "1.13.0" } diff --git a/src/cli/file-pass.js b/src/cli/file-pass.js index fdd7804..a9c5969 100644 --- a/src/cli/file-pass.js +++ b/src/cli/file-pass.js @@ -54,33 +54,20 @@ function slimPass(pass) { return { bytes: pass.bytes, unavailable: pass.applied.length === 0 }; } -// Mirrors `dedupRoot()`’s `savingsOnly` gate against an already-computed plain -// pass, rather than running `dedup()` again just to reapply a rule that only -// looks at the first pass’s own `bytes.saved` -function applySavingsOnlyGate(pass) { - if (pass.bytes.saved >= 0) return pass; - return { - bytes: { before: pass.bytes.before, after: pass.bytes.before, saved: 0 }, - applied: [], - }; -} - // The report table’s four savings columns, in the order they’re rendered. // Named once here, where they’re produced, so the consumers in `report.js` // can’t drift out of sync with them. export const PASS_KEYS = ['passDefault', 'passDefaultS', 'passAgg', 'passAggS']; -// Only two of the four actually run `dedup()`; the `-s` variants are derived -// from those, since a second full pass would just reproduce the first one’s -// `bytes` before the gate looks at them. +// All four run `dedup()`. The `-s` variants can’t be derived from the ungated +// ones: The gate decides per merge, so a file mixing merges that save with +// merges that cost lands somewhere neither ungated figure predicts. function computeReportPasses(css, targetOptions) { - const passDefault = dedup(css, { ...targetOptions, aggressive: false, savingsOnly: false }); - const passAgg = dedup(css, { ...targetOptions, aggressive: true, savingsOnly: false }); return { - passDefault, - passDefaultS: applySavingsOnlyGate(passDefault), - passAgg, - passAggS: applySavingsOnlyGate(passAgg), + passDefault: dedup(css, { ...targetOptions, aggressive: false, savingsOnly: false }), + passDefaultS: dedup(css, { ...targetOptions, aggressive: false, savingsOnly: true }), + passAgg: dedup(css, { ...targetOptions, aggressive: true, savingsOnly: false }), + passAggS: dedup(css, { ...targetOptions, aggressive: true, savingsOnly: true }), }; } diff --git a/src/cli/render.js b/src/cli/render.js index 55e599b..38b9428 100644 --- a/src/cli/render.js +++ b/src/cli/render.js @@ -117,8 +117,10 @@ function renderFixPass(payload, { isStdin, label, multi, flags }) { // The outcome first, then this run’s footnotes on it (growth caveat, // aggressive-only warning, where it was written), then the two - // forward-looking items (what was skipped, what `--aggressive` adds) - if (withheld) { + // forward-looking items (what was skipped, what `--aggressive` adds). + // The gate decides per merge, so a file can come out partly consolidated— + // “left this file untouched” only fits the run where nothing survived it. + if (withheld && !applied) { log(`* 0 declarations consolidated, ${withheld.count} withheld: \`savingsOnly\` left this file untouched—consolidating would ${formatByteDeltaClause(withheld.bytes.saved, withheld.bytes.before)}`); } else { log(`* ${applied} declaration${plural(applied)} consolidated${applied ? `: ${formatAppliedReduceClause(bytes)}` : ''}`); @@ -128,6 +130,9 @@ function renderFixPass(payload, { isStdin, label, multi, flags }) { if (bytes.saved < 0) { log('* Worth it for maintainability (each declaration used once); skip `--fix` here if you care more about transfer size.'); } + if (withheld) { + log(`* ${withheld.count} further merge${plural(withheld.count)} withheld by \`savingsOnly\`—applying ${withheld.count !== 1 ? 'them' : 'it'} too would ${formatByteDeltaClause(withheld.bytes.saved, withheld.bytes.before)}`); + } if (aggressiveDiffers) { const share = aggressiveOnly > 0 ? `${aggressiveOnly} of these merges ${aggressiveOnly !== 1 ? 'are' : 'is'}` @@ -267,8 +272,12 @@ function printOverallFixSummary(ok) { const shrinkTotal = sumBy(filesShrink, result => result.stats.bytesSaved); const growTotal = Math.abs(sumBy(filesGrow, result => result.stats.bytesSaved)); - const withheldFiles = ok.filter(result => result.stats.withheldCount > 0); + // Split by what the gate left behind—a file where nothing survived it, and + // one that kept the merges that paid for themselves, warrant different words + const withheldFiles = ok.filter(result => result.stats.withheldCount > 0 && !result.stats.applied); const withheldGrowthTotal = sumBy(withheldFiles, result => result.stats.withheldGrowth); + const partialFiles = ok.filter(result => result.stats.withheldCount > 0 && result.stats.applied); + const partialGrowthTotal = sumBy(partialFiles, result => result.stats.withheldGrowth); const aggFiles = ok.filter(result => result.stats.aggDiffers); const aggFilesShrink = aggFiles.filter(result => result.stats.aggExtraSaved > 0); @@ -299,6 +308,9 @@ function printOverallFixSummary(ok) { if (withheldFiles.length) { console.log(`* ${withheldFiles.length} file${plural(withheldFiles.length)} left untouched by \`--savings-only\`—consolidating would have made ${withheldFiles.length !== 1 ? 'them' : 'it'} ${formatBytesShareOfTotal(withheldGrowthTotal, totalBeforeAll)} bigger in total`); } + if (partialFiles.length) { + console.log(`* ${partialFiles.length} file${plural(partialFiles.length)} had further merges withheld by \`--savings-only\`—applying those too would have added ${formatBytesShareOfTotal(partialGrowthTotal, totalBeforeAll)} in total`); + } if (!aggFiles.length) return; diff --git a/src/consolidate.js b/src/consolidate.js index 190cd66..b6ec3af 100644 --- a/src/consolidate.js +++ b/src/consolidate.js @@ -16,6 +16,7 @@ import { ownSelectors, } from './lib/scopes.js'; import { usesMultilineSelectors, usesSpacedCommas } from './lib/style.js'; +import { resetByteCache } from './lib/transaction.js'; // Conditional group rules whose empty block is inert. `@layer` is deliberately // absent: a layer’s position in the layer order is set by its first @@ -23,28 +24,40 @@ import { usesMultilineSelectors, usesSpacedCommas } from './lib/style.js'; // the cascade. const INERT_WHEN_EMPTY_ATRULES = new Set(['media', 'supports', 'container']); -// Aggressive mode’s cross-block merges can drain the earlier of two -// same-condition blocks completely. This removes such blocks—only ones this -// run emptied, and only where emptiness is provably inert. The walk collects -// candidates parents-first, so the reverse pass sees each inner block before -// its parent and a parent emptied by its child’s removal is caught in the same -// sweep. -function removeEmptiedConditionBlocks(root, initiallyEmpty) { +// Every block aggressive mode could later find emptied, innermost first—so one +// sweep sees each inner block before its parent, and a parent emptied by its +// child’s removal is caught in the same pass. Collected once per run. +function collectConditionBlocks(root) { const candidates = []; root.walkAtRules(atrule => { if (INERT_WHEN_EMPTY_ATRULES.has(atrule.name.toLowerCase())) candidates.push(atrule); }); + return candidates.reverse(); +} - for (const atrule of candidates.reverse()) { - if (atrule.nodes && !atrule.nodes.length && !initiallyEmpty.has(atrule)) atrule.remove(); +// Aggressive mode’s cross-block merges can drain the earlier of two +// same-condition blocks completely. This removes such blocks—only ones this +// run emptied, and only where emptiness is provably inert. +function removeEmptiedConditionBlocks(candidates, initiallyEmpty) { + const removed = []; + for (const atrule of candidates) { + // Gone already, in this sweep or an earlier one + if (!atrule.parent) continue; + if (atrule.nodes && !atrule.nodes.length && !initiallyEmpty.has(atrule)) { + // Measured before it goes: standing empty, all that’s left of it is the + // wrapper the `savingsOnly` gate needs to price + removed.push(atrule); + atrule.remove(); + } } + return removed; } // Everything the merge strategies need from this run. The normalization mode // is bound once here, so no call site can fall back to default-mode // normalization by forgetting a flag—which would silently give one declaration // two different keys in different phases of an aggressive run. -function createContext(root, options) { +function createContext(root, options, settle) { const aggressive = options.aggressive ?? false; return { aggressive, @@ -55,6 +68,13 @@ function createContext(root, options) { spacedCommas: usesSpacedCommas(root), applied: [], skipped: [], + // The `savingsOnly` gate in `merge.js` needs the whole style sheet to + // weigh one merge (a cross-block merge’s saving lands outside the scope it + // happens in) and `settle()` to see the state it would actually ship + savingsOnly: options.savingsOnly ?? false, + root, + settle, + declined: [], }; } @@ -77,6 +97,10 @@ function runPass(root, ctx) { ctx.applied.push(...removeRedundantDuplicates(ctx, atrule, describeScope(atrule), [atRuleLabel(atrule)])); } for (const scope of scopes) foldSameSelectorRules(ctx, scope); + + // The phases above rewrite rules the gate would otherwise measure against a + // stale cache; from here on the gate maintains it itself + resetByteCache(root); for (const scope of scopes) mergeDuplicateGroups(ctx, scope); } @@ -87,17 +111,25 @@ function consolidateRoot(root, options = {}) { // Bytes, not characters—the effectiveness this measures (fewer bytes over // the wire) is a transfer-size concern. const before = Buffer.byteLength(root.toString(), 'utf8'); - const ctx = createContext(root, options); // Blocks already empty in the source, so the cleanup at the end only ever // removes what this run emptied const initiallyEmpty = new Set(); - if (ctx.aggressive) { - root.walkAtRules(atrule => { + const aggressive = options.aggressive ?? false; + const conditionBlocks = aggressive ? collectConditionBlocks(root) : []; + if (aggressive) { + for (const atrule of conditionBlocks) { if (atrule.nodes && !atrule.nodes.length) initiallyEmpty.add(atrule); - }); + } } + // Bringing the style sheet to the state it would ship in. Idempotent, so the + // gate can call it around every merge it weighs and the run can call it once + // more at the end—which is why the candidate list is gathered once rather + // than re-walked on each of those calls. + const settle = () => (aggressive ? removeEmptiedConditionBlocks(conditionBlocks, initiallyEmpty) : []); + const ctx = createContext(root, options, settle); + // One merge can unblock or create another: a fresh merged rule may twin with // an existing one, and an emptied rule stops fencing the spans it sat in—so // passes repeat until nothing changes. Termination is guaranteed, since @@ -108,42 +140,46 @@ function consolidateRoot(root, options = {}) { while (ctx.applied.length !== appliedCount) { appliedCount = ctx.applied.length; ctx.skipped.length = 0; + ctx.declined.length = 0; runPass(root, ctx); } - if (ctx.aggressive) removeEmptiedConditionBlocks(root, initiallyEmpty); + settle(); const after = Buffer.byteLength(root.toString(), 'utf8'); - return { applied: ctx.applied, skipped: ctx.skipped, bytes: { before, after, saved: before - after } }; + return { + applied: ctx.applied, + skipped: ctx.skipped, + declined: ctx.declined, + bytes: { before, after, saved: before - after }, + }; } -// The `savingsOnly` gate: consolidation runs on a detached clone first, and -// only a result that doesn’t grow the style sheet is grafted back onto the -// real root—which is what lets the PostCSS plugin and the CLI share one -// implementation of the policy. A withheld result reports `applied: []` and -// unchanged bytes (what actually happened), with the would-be outcome under -// `withheld` so callers can explain what was declined. A net-zero result still -// applies (deduplicated at no byte cost). +// The `savingsOnly` gate is decided per cluster, inside `merge.js`: Every +// merge that would grow the style sheet is performed, measured, and undone, +// leaving the ones that pay for themselves in place. A file therefore keeps +// its savings even when other merges in it would have cost more than the whole +// consolidation gains—the case that used to sink every merge in the file with +// it. A net-zero merge still applies (deduplicated at no byte cost). +// +// What was declined arrives as `withheld: { count, bytes }`: How many merges, +// and the byte counts the style sheet would have had with them applied, too. +// Absent when nothing was declined. export function dedupRoot(root, options = {}) { - if (!options.savingsOnly) return consolidateRoot(root, options); - - const clone = root.clone(); - const result = consolidateRoot(clone, options); - if (result.bytes.saved < 0) { - return { - applied: [], - skipped: result.skipped, - bytes: { before: result.bytes.before, after: result.bytes.before, saved: 0 }, - withheld: { count: result.applied.length, bytes: result.bytes }, - }; - } + const { declined, ...result } = consolidateRoot(root, options); + if (!declined.length) return result; - if (result.applied.length) { - root.raws = clone.raws; - root.removeAll(); - root.append(clone.nodes); - } - return result; + // Clusters are independent by construction, so the cost of applying the + // declined merges as well is the sum of what each was measured to cost + const cost = declined.reduce((total, entry) => total + entry.cost, 0); + const count = declined.reduce((total, entry) => total + entry.count, 0); + return { + ...result, + withheld: { + count, + bytes: { before: result.bytes.after, after: result.bytes.after + cost, saved: -cost }, + }, + }; } // A `/*# sourceMappingURL=… */` comment means a build tool generated this diff --git a/src/lib/style.js b/src/lib/style.js index d8b22a5..aaf4d3e 100644 --- a/src/lib/style.js +++ b/src/lib/style.js @@ -18,6 +18,13 @@ export function resetSeparatorCache() { separatorCache = new WeakMap(); } +// A speculative merge that gets rolled back may already have had its +// separator computed—off a container holding residuals that no longer exist. +// Dropping the entry lets the next caller tally the restored tree. +export function forgetSeparator(container) { + separatorCache.delete(container); +} + // The gap most sibling nodes carry—the container’s prevailing “normal” // separation between rules. A majority vote, not just whichever neighbor is // handy, since that neighbor can be the anomaly. A gap straight after a diff --git a/src/lib/transaction.js b/src/lib/transaction.js new file mode 100644 index 0000000..28d487d --- /dev/null +++ b/src/lib/transaction.js @@ -0,0 +1,290 @@ +// Snapshot and rollback around one speculative merge, for the `savingsOnly` +// gate. The gate measures a merge by performing it and undoing it again when +// it turns out to cost bytes, rather than by predicting its cost up front—so +// no merge strategy has to carry a second, parallel implementation of itself +// in byte arithmetic, and the figure the gate decides on is the real one. + +import { forgetSeparator } from './style.js'; + +// Every container whose child list a merge can touch: the rules’ own parents, +// plus their ancestors up to the root—an aggressive cross-block merge can +// drain a conditional block, whose removal changes *its* parent’s child list. +function affectedContainers(rules) { + const containers = new Set(); + for (const rule of rules) { + for (let node = rule.parent; node; node = node.parent) containers.add(node); + } + return [...containers]; +} + +// A merge is priced from the few rules it actually rewrites, +// and everything around them cancels out untouched +let byteCache = new WeakMap(); +// Nodes already in the tree when the pass began, so a rule a merge inserts can +// be told from one that was always there +let knownNodes = new WeakSet(); +// Where the merge strategies report the rules they insert. Finding those any +// other way means scanning a container, which is the cost this exists to +// avoid—but the accounting below still reconciles against the container’s +// child count, so a strategy that forgets to report is slow, never wrong. +let insertions = null; + +export function resetByteCache(root) { + byteCache = new WeakMap(); + knownNodes = new WeakSet(); + root.walk(node => knownNodes.add(node)); +} + +export function recordInsertion(node) { + if (insertions) insertions.push(node); +} + +// How much text the gate has serialized to price merges; +// exported for the scaling test +let measuredBytes = 0; + +export function measuredByteTotal() { + return measuredBytes; +} + +function textBytes(text) { + return text ? Buffer.byteLength(text, 'utf8') : 0; +} + +// `toString()` does not include a node’s own leading whitespace, so it counts +// separately—the two together are exactly what the node adds to the output +function measureNode(node) { + const bytes = textBytes(node.raws.before) + Buffer.byteLength(node.toString(), 'utf8'); + measuredBytes += bytes; + return bytes; +} + +function cachedBytes(node) { + let bytes = byteCache.get(node); + if (bytes === undefined) { + bytes = measureNode(node); + byteCache.set(node, bytes); + } + return bytes; +} + +function remeasure(node) { + const bytes = measureNode(node); + byteCache.set(node, bytes); + return bytes; +} + +// The root-level subtrees this merge could remove: its rules, or the blocks +// holding them, which `settle()` clears away once drained +function removableAtRoot(root, rules) { + const removable = new Set(); + for (const rule of rules) { + let node = rule; + while (node.parent && node.parent !== root) node = node.parent; + if (node.parent === root) removable.add(node); + } + return removable; +} + +// The leading root children whose spacing a removal can rewrite. PostCSS hands +// each removed first child’s `raws.before` down the line, so a run of removals +// at the front reaches the first child that outlives them—no further. +function captureFront(root, removable) { + const front = new Map(); + for (const node of root.nodes) { + front.set(node, node.raws.before); + if (!removable.has(node)) break; + } + return front; +} + +// A node’s own mutable state plus its child list, recorded by identity rather +// than by cloning. Identity is the whole point: `runPass()` collects every +// scope up front, so a rule nested inside one this rollback touches is already +// spoken for by a later scope in the same pass. Hand that scope a clone and it +// would go on merging a subtree detached from the style sheet. +function captureNode(node) { + return { + node, + raws: { ...node.raws }, + selector: node.selector, + prop: node.prop, + value: node.value, + important: node.important, + name: node.name, + params: node.params, + children: node.nodes ? node.nodes.map(captureNode) : null, + }; +} + +function restoreNode(entry) { + const { node } = entry; + + // Scalars before `raws`: PostCSS drops a value’s raw spelling when the value + // itself is assigned, so restoring `raws` last is what puts it back + if (entry.selector !== undefined) node.selector = entry.selector; + if (entry.prop !== undefined) node.prop = entry.prop; + if (entry.value !== undefined) node.value = entry.value; + if (entry.important !== undefined) node.important = entry.important; + if (entry.name !== undefined) node.name = entry.name; + if (entry.params !== undefined) node.params = entry.params; + node.raws = { ...entry.raws }; + + if (!entry.children) return; + node.nodes = entry.children.map(child => child.node); + for (const child of node.nodes) child.parent = node; + for (const child of entry.children) restoreNode(child); +} + +/** + * Everything needed to put a scope back exactly as it stands right now, and to + * price what happens in between. Cheap enough to take per cluster—it walks one + * cluster’s rules, not the style sheet. + */ +export function snapshot(root, scope, rules) { + // Per rule, not just the sum: a rollback has to put these back into the + // cache, or the speculative measurement would outlive the tree it described + const ruleBefore = new Map(); + let ruleBytes = 0; + for (const rule of rules) { + const bytes = cachedBytes(rule); + ruleBefore.set(rule, bytes); + ruleBytes += bytes; + } + + // Where each rule started, and the child counts of those containers, so the + // reconciliation below can tell a genuine gap from an expected one + const ruleParent = new Map(); + const counts = new Map(); + for (const rule of rules) { + if (!rule.parent) continue; + ruleParent.set(rule, rule.parent); + counts.set(rule.parent, rule.parent.nodes.length); + } + + insertions = []; + + return { + root, + ruleBefore, + ruleBytes, + ruleParent, + counts, + // PostCSS hands a removed first child’s `raws.before` to its successor, so + // a node this merge never touched can still change size. That override + // lives on `Root` alone—a nested container just splices—so only the root’s + // own leading children need recording. + frontBefores: captureFront(root, removableAtRoot(root, rules)), + // Residual rules are appended here as merges create them, so the array + // itself is state a rollback has to restore + scopeRules: scope.rules.slice(), + containers: affectedContainers(rules).map(container => ({ container, nodes: container.nodes.slice() })), + rules: rules.map(rule => ({ entry: captureNode(rule), parent: rule.parent })), + }; +} + +// Rules the merge reported inserting, plus—only where a container’s child +// count disagrees with what those reports account for—whatever else turned up +// in it. The scan is the safety net: It makes an unreported insertion cost a +// container walk rather than a wrong answer. +function insertedNodes(snap) { + const found = new Set(); + const perContainer = new Map(); + + for (const node of insertions) { + if (!node.parent || knownNodes.has(node) || found.has(node)) continue; + found.add(node); + perContainer.set(node.parent, (perContainer.get(node.parent) ?? 0) + 1); + } + + for (const [container, before] of snap.counts) { + // A container the merge removed outright is priced as a whole, not by its + // children + if (!container.parent && container !== snap.root) continue; + + let gone = 0; + for (const [rule, parent] of snap.ruleParent) { + if (parent === container && rule.parent !== container) gone++; + } + if (container.nodes.length === before - gone + (perContainer.get(container) ?? 0)) continue; + + for (const node of container.nodes) { + if (!knownNodes.has(node)) found.add(node); + } + } + + return [...found]; +} + +/** + * What the merge just performed cost, in bytes of the whole style sheet. + * Negative means it paid for itself. + */ +export function costSince(snap, removedContainers) { + const { root, ruleBefore, frontBefores } = snap; + let after = 0; + + // A rule the merge removed contributes nothing, and its old size stays + // counted in `ruleBytes` + for (const rule of ruleBefore.keys()) { + if (rule.parent) after += remeasure(rule); + } + + snap.inserted = insertedNodes(snap); + for (const node of snap.inserted) { + after += remeasure(node); + knownNodes.add(node); + } + insertions = null; + + // A conditional block `settle()` cleared away. Its rules are already + // accounted for above; what goes with it is the block’s own wrapper, which + // is all that is left to measure now that it stands empty. + let wrappers = 0; + for (const container of removedContainers) wrappers += measureNode(container); + + // A node whose leading whitespace PostCSS rewrote when it promoted a new + // first child, and that nothing above has re-measured + let respaced = 0; + for (const [node, before] of frontBefores) { + if (ruleBefore.has(node) || node.parent !== root || before === node.raws.before) continue; + respaced += textBytes(node.raws.before) - textBytes(before); + byteCache.delete(node); + } + + return after - snap.ruleBytes - wrappers + respaced; +} + +// Restores the child lists first (which un-removes emptied rules and drops +// inserted residuals), then each rule’s own contents. Every node object is the +// one that was there before, so references held elsewhere—`scope.rules`, a +// cluster’s `distinctRules`, a nested scope collected earlier this pass—stay +// live across a rollback. +export function rollback(snap, scope) { + // The speculative pass remeasured these; put the real figures back + for (const [node, bytes] of snap.ruleBefore) byteCache.set(node, bytes); + // Residuals it inserted are about to be dropped—they were never really here + if (snap.inserted) for (const node of snap.inserted) knownNodes.delete(node); + + // …and undo any re-spacing PostCSS did when it promoted a new first child + for (const [node, before] of snap.frontBefores) { + if (node.raws.before !== before) { + node.raws.before = before; + byteCache.delete(node); + } + } + + for (const { container, nodes } of snap.containers) { + container.nodes = nodes; + for (const node of nodes) node.parent = container; + forgetSeparator(container); + } + + for (const { entry, parent } of snap.rules) { + restoreNode(entry); + entry.node.parent = parent; + } + + scope.rules.length = 0; + scope.rules.push(...snap.scopeRules); +} \ No newline at end of file diff --git a/src/merge.js b/src/merge.js index 60b123f..8cec8d0 100644 --- a/src/merge.js +++ b/src/merge.js @@ -11,6 +11,7 @@ import { splitSelectors, selectorsAreMutuallyExclusive, selectorsLikelyDisjoint import { propertiesOverlap } from './lib/shorthands.js'; import { insertAfter, joinSelectors, typicalSeparator } from './lib/style.js'; import { declsOf, pushTo } from './lib/util.js'; +import { costSince, recordInsertion, rollback, snapshot } from './lib/transaction.js'; // All occurrences of a key are equivalent by our own normalization rules, so // the merge keeps whichever raw spelling is shortest rather than whatever the @@ -251,6 +252,7 @@ export function mergeSoloGroup(ctx, scope, group) { if (targetBeforeExtras) { beforeResidual = makeResidual(target, targetOriginalSelector, targetBeforeExtras); target.before(beforeResidual); + recordInsertion(beforeResidual); target.raws.before = interPieceSeparator; } @@ -262,6 +264,7 @@ export function mergeSoloGroup(ctx, scope, group) { const residual = makeResidual(target, rule === target ? targetOriginalSelector : rule.selector, extras); insertAfter(insertPoint, residual, interPieceSeparator); + recordInsertion(residual); insertPoint = residual; afterResiduals.push(residual); } @@ -305,15 +308,17 @@ export function mergePartialGroup(ctx, scope, group, reason) { runs.at(-1).push(distinctRules[i]); } + // A blocking rule separates the runs, so each touches a disjoint set of + // rules and carries its own byte outcome—`savingsOnly` weighs them one by one for (const runRules of runs) { if (runRules.length < 2) continue; const runSet = new Set(runRules); - mergeSoloGroup(ctx, scope, { + gated(ctx, scope, [key], runRules, () => mergeSoloGroup(ctx, scope, { key, occurrences: occurrences.filter(occ => runSet.has(occ.rule)), distinctRules: runRules, propNormalized, - }); + })); } // Whatever merged above resurfaces as a smaller group on the next @@ -431,6 +436,7 @@ function mergeClusterGroupRuns(ctx, scope, group) { if (lastDecl.value !== value) lastDecl.value = value; mergedRule.append(lastDecl); lastRule.after(mergedRule); + recordInsertion(mergedRule); scope.rules.splice(scope.rules.indexOf(lastRule) + 1, 0, mergedRule); // The clone inherited `lastRule`’s own `raws.before`—right if `lastRule` @@ -535,8 +541,10 @@ function splitStarHub(ctx, scope, cluster, hub) { if (trailingGap.length) finalRules.push(makeResidual(hub, hubOriginalSelector, trailingGap)); hub.before(finalRules[0]); + recordInsertion(finalRules[0]); for (let i = 1; i < finalRules.length; i++) { insertAfter(finalRules[i - 1], finalRules[i], interPieceSeparator); + recordInsertion(finalRules[i]); } hub.remove(); @@ -632,6 +640,41 @@ function clusterGroups(groups) { return [...clusters.values()]; } +// The `savingsOnly` gate, applied per cluster. Clusters are exactly the units +// that can be decided independently—union-find has already put every group +// sharing a rule into the same one—so accepting one and declining another +// cannot leave a half-applied merge behind. +// +// Only declaration merges are gated. The other two strategies don’t cost +// bytes: Collapsing a repeat within one rule removes text and adds none, and +// folding two same-selector rules removes a whole selector and its braces. +function gated(ctx, scope, keys, rules, apply) { + if (!ctx.savingsOnly) { + apply(); + return; + } + + // Measurements are taken with emptied conditional blocks already cleared + // away, so a block drained by an *earlier* cluster is not counted as this + // one’s saving. `settle()` is idempotent, so this only ever does work on the + // first cluster of a pass—the phases before it are the ones that can leave a + // block behind. + ctx.settle(); + + const snap = snapshot(ctx.root, scope, [...new Set(rules)]); + const appliedBefore = ctx.applied.length; + + apply(); + const cost = costSince(snap, ctx.settle()); + if (cost <= 0) return; + + rollback(snap, scope); + // `skipped` is left as it stands—whatever the safety checks concluded about + // this cluster is still true, and still worth reporting + ctx.declined.push({ scope: scope.label, keys, count: ctx.applied.length - appliedBefore, cost }); + ctx.applied.length = appliedBefore; +} + export function mergeDuplicateGroups(ctx, scope) { const byKey = new Map(); for (const rule of eligibleRules(scope, ctx.ignorePatterns)) { @@ -661,8 +704,10 @@ export function mergeDuplicateGroups(ctx, scope) { } if (!outsideBlocker) { - if (cluster.length === 1) mergeSoloGroup(ctx, scope, cluster[0]); - else mergeCluster(ctx, scope, cluster); + gated(ctx, scope, cluster.map(group => group.key), [...clusterRules], () => { + if (cluster.length === 1) mergeSoloGroup(ctx, scope, cluster[0]); + else mergeCluster(ctx, scope, cluster); + }); continue; } @@ -675,8 +720,12 @@ export function mergeDuplicateGroups(ctx, scope) { continue; } + // Entangled members share rules, so their runs can only be weighed as one + gated(ctx, scope, cluster.map(member => member.key), [...clusterRules], () => { + for (const member of cluster) mergeClusterGroupRuns(ctx, scope, member); + }); + for (const member of cluster) { - mergeClusterGroupRuns(ctx, scope, member); ctx.skipped.push({ scope: scope.label, key: member.key, diff --git a/src/plugin.js b/src/plugin.js index 80308b2..6022216 100644 --- a/src/plugin.js +++ b/src/plugin.js @@ -12,7 +12,7 @@ export default function cssdedup(options = {}) { if (options.fix) { const { skipped, withheld } = dedupRoot(root, options); if (withheld) { - root.warn(result, `Consolidation withheld (\`savingsOnly\`): ${withheld.count} merge${withheld.count !== 1 ? 's' : ''} would make the style sheet ${Math.abs(withheld.bytes.saved)} bytes bigger`); + root.warn(result, `${withheld.count} merge${withheld.count !== 1 ? 's' : ''} withheld (\`savingsOnly\`): applying ${withheld.count !== 1 ? 'them' : 'it'} would make the style sheet ${Math.abs(withheld.bytes.saved)} bytes bigger`); } for (const item of skipped) { root.warn(result, `Duplicate \`${item.key}\` left unmerged (${item.scope === 'root' ? 'root' : item.scope}): ${item.reason}`); diff --git a/test/cli.test.js b/test/cli.test.js index e4165f4..b8db58d 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -5,7 +5,7 @@ import path from 'node:path'; import { availableParallelism } from 'node:os'; import { poolSize, shouldParallelize } from '../src/cli/pool.js'; import { dedup } from '../src/index.js'; -import { BEST_CELL, RE_MERGED_AB, RE_MERGED_AC, RE_PAYOFF_FIX, RE_SYNTAX_ERROR, RE_SYNTAX_ERROR_UNCLOSED, RE_WITHHELD_ONE, cssGrowing, cssGrowingAggressive, cssShrinkingAggressive, dirTest, findingsRow, fixturesDir, makeTempDir, run, runColor, runTty } from './helpers.js'; +import { BEST_CELL, RE_MERGED_AB, RE_MERGED_AC, RE_PAYOFF_FIX, RE_SYNTAX_ERROR, RE_SYNTAX_ERROR_UNCLOSED, RE_WITHHELD_ONE, cssGrowing, cssGrowingAggressive, cssMixed, cssShrinkingAggressive, dirTest, findingsRow, fixturesDir, makeTempDir, run, runColor, runTty } from './helpers.js'; describe('CLI', () => { test('Shows help with `--help`', () => { @@ -214,13 +214,53 @@ describe('CLI', () => { assert.ok(detailIndex < countsIndex); // The counts line—the run’s conclusion—must be among the last things // printed, not stranded above the skipped-group detail - assert.ok(stdout.includes('more declarations in aggressive mode')); + assert.match(stdout, /\d+ more declarations? in aggressive mode/); assert.match(stdout, /\* 0 declarations consolidated, 1 withheld:.*\n\* 1 finding skipped \(considered unsafe to auto-merge\)/); } finally { fs.rmSync(dirTemp, { recursive: true, force: true }); } }); + test('`--fix --savings-only` keeps the merges that pay for themselves and reports the rest as withheld', () => { + const dirTemp = makeTempDir('temp_savings_partial'); + const file = path.join(dirTemp, 'mixed.css'); + fs.writeFileSync(file, cssMixed); + + try { + const { stdout } = run(['--fix', '--savings-only', file]); + // The outcome line states what was consolidated, not that the file was + // left untouched—the gate decides per merge + assert.match(stdout, /\* \d+ declarations? consolidated: Reduced duplication and saved \d+ bytes \(\d+ → \d+ bytes, -\d+\.\d%\)/); + assert.doesNotMatch(stdout, /left this file untouched/); + assert.match(stdout, /\* 1 further merge withheld by `savingsOnly`—applying it too would grow by \d+ bytes \(\+\d+\.\d%\)/); + + const output = fs.readFileSync(file, 'utf8'); + assert.match(output, /\.p,\s*\.q,\s*\.r\s*{\s*margin: 0;\s*}/); + assert.match(output, /\.b\s*{\s*color: red;\s*}/); + assert.ok(Buffer.byteLength(output, 'utf8') < Buffer.byteLength(cssMixed, 'utf8')); + } finally { + fs.rmSync(dirTemp, { recursive: true, force: true }); + } + }); + + test('`--fix --savings-only` distinguishes a partly consolidated file from an untouched one in the overall summary', () => { + const dirTemp = makeTempDir('temp_multi_summary_partial'); + const filePartial = path.join(dirTemp, 'partial.css'); + const fileGrow = path.join(dirTemp, 'grow.css'); + fs.writeFileSync(filePartial, cssMixed); + fs.writeFileSync(fileGrow, cssGrowing); + + try { + const { stdout } = run(['--fix', '--savings-only', filePartial, fileGrow]); + assert.match(stdout, /\* 1 file left untouched by `--savings-only`/); + assert.match(stdout, /\* 1 file had further merges withheld by `--savings-only`—applying those too would have added \d+ bytes \(\d+\.\d% overall\) in total/); + assert.strictEqual(fs.readFileSync(fileGrow, 'utf8'), cssGrowing); + assert.notStrictEqual(fs.readFileSync(filePartial, 'utf8'), cssMixed); + } finally { + fs.rmSync(dirTemp, { recursive: true, force: true }); + } + }); + test('`--fix --savings-only -` still writes the untouched style sheet to STDOUT when withholding', () => { const source = cssGrowing; const { stdout, stderr, status } = run(['--fix', '-s', '-'], { input: source }); diff --git a/test/dedup.test.js b/test/dedup.test.js index 290e5ca..b46276b 100644 --- a/test/dedup.test.js +++ b/test/dedup.test.js @@ -3,7 +3,8 @@ import assert from 'node:assert'; import { analyze, dedup } from '../src/index.js'; import { normalizeValue } from '../src/lib/normalization.js'; import { selectorsLikelyDisjoint } from '../src/lib/selectors.js'; -import { RE_MERGED_AB, RE_MERGED_AC, cssGrowing, cssGrowingAggressive } from './helpers.js'; +import { measuredByteTotal } from '../src/lib/transaction.js'; +import { RE_MERGED_AB, RE_MERGED_AC, cssGrowing, cssGrowingAggressive, cssEntangledGrowing, cssEntangledShrinking, cssMixed, cssNestedHost, cssTwoLeadingRemovals } from './helpers.js'; describe('Deduplication', () => { test('Treats a `)` inside a `/* … */` comment as text when scanning a `min()` call', () => { @@ -757,4 +758,172 @@ describe('Savings only', () => { assert.strictEqual(output, css); assert.strictEqual(withheld.count, 1); }); + + test('Applies the merges that pay for themselves and declines only the ones that don\u2019t', () => { + const { css: output, applied, bytes, withheld } = dedup(cssMixed, { savingsOnly: true }); + + // The shrinking cluster merges… + assert.ok(applied.length > 0); + assert.match(output, /\.p,\s*\.q,\s*\.r\s*{\s*margin: 0;\s*}/); + // …while the growing one stays exactly as it was written + assert.match(output, /\.very-long-selector-name-one\s*{\s*color: red;\s*font-weight: bold;\s*}/); + assert.match(output, /\.b\s*{\s*color: red;\s*}/); + assert.doesNotMatch(output, /\.very-long-selector-name-one,/); + + assert.strictEqual(withheld.count, 1); + assert.ok(withheld.bytes.saved < 0, 'the declined merge is reported with what it would have cost'); + assert.ok(bytes.saved > 0, 'the run as a whole still shrinks the style sheet'); + assert.strictEqual(bytes.after, Buffer.byteLength(output, 'utf8')); + }); + + test('Never grows the style sheet, whichever mix of merges a file offers', () => { + const cases = [cssGrowing, cssGrowingAggressive, cssMixed, '.a { color: red; }\n.b { color: red; }\n']; + for (const aggressive of [false, true]) { + for (const css of cases) { + const { css: output, bytes } = dedup(css, { savingsOnly: true, aggressive }); + assert.ok( + Buffer.byteLength(output, 'utf8') <= Buffer.byteLength(css, 'utf8'), + `\`savingsOnly\` grew a style sheet (aggressive: ${aggressive})` + ); + assert.ok(bytes.saved >= 0); + } + } + }); + + test('Leaves a file untouched when every merge it offers would grow it', () => { + const { css: output, applied, bytes, withheld } = dedup(cssGrowing, { savingsOnly: true }); + assert.strictEqual(output, cssGrowing); + assert.strictEqual(applied.length, 0); + assert.strictEqual(bytes.saved, 0); + assert.strictEqual(withheld.count, 1); + }); + + test('Reports no `withheld` when every merge pays for itself', () => { + const css = '.a { color: red; top: 0; }\n.b { color: red; left: 0; }\n.c { color: red; right: 0; }\n'; + const { withheld, applied } = dedup(css, { savingsOnly: true }); + assert.ok(applied.length > 0); + assert.strictEqual(withheld, undefined); + }); + + test('Declining a merge on a nesting host still merges the rules nested inside it', () => { + // The inner scope is collected before the outer merge runs, so a rollback + // that replaced `.a`’s children with copies would leave that scope + // merging a subtree no longer attached to the style sheet + const { css: output, applied, bytes } = dedup(cssNestedHost, { savingsOnly: true }); + + assert.match(output, /&:hover,\s*&:focus\s*{\s*top: 0;\s*}/); + assert.match(output, /\.very-long-selector-name-one\s*{\s*color: red;/); + assert.doesNotMatch(output, /\.very-long-selector-name-one,/); + assert.strictEqual(applied.length, 1); + assert.strictEqual(bytes.saved, Buffer.byteLength(cssNestedHost, 'utf8') - Buffer.byteLength(output, 'utf8')); + assert.ok(bytes.saved > 0); + }); + + test('Weighs each safe run of a blocked group on its own', () => { + // `background` is blocked in the middle, leaving two independent runs: a + // shrinking one (short selectors) and a growing one (long selectors). + // Gating them together would let either drag the other along. + const css = [ + '.s1 { background: red; }', + '.s2 { background: red; }', + '.mid { background: blue; }', + '.a-very-long-selector-name-here { background: red; color: #fff; }', + '.another-very-long-selector-name { background: red; color: #000; }', + '', + ].join('\n'); + + const { css: output, bytes, withheld } = dedup(css, { savingsOnly: true }); + assert.match(output, /\.s1,\s*\.s2\s*{\s*background: red;\s*}/); + assert.doesNotMatch(output, /\.a-very-long-selector-name-here,/); + assert.ok(bytes.saved > 0); + assert.strictEqual(withheld.count, 1); + }); + + test('Declines an entangled cluster as a whole, leaving the style sheet untouched', () => { + // Its groups all share the hub rule, so they merge as one coordinated + // whole or not at all—and that whole grows the file + const ungated = dedup(cssEntangledGrowing); + assert.ok(ungated.bytes.saved < 0, 'fixture should grow the file when ungated'); + + const { css: output, applied, skipped, bytes, withheld } = dedup(cssEntangledGrowing, { savingsOnly: true }); + assert.strictEqual(output, cssEntangledGrowing); + assert.strictEqual(applied.length, 0); + assert.strictEqual(skipped.length, 0); + assert.strictEqual(bytes.saved, 0); + assert.strictEqual(withheld.count, ungated.applied.length); + assert.strictEqual(withheld.bytes.saved, ungated.bytes.saved); + }); + + test('Applies an entangled cluster that pays for itself, exactly as an ungated run does', () => { + const ungated = dedup(cssEntangledShrinking); + assert.ok(ungated.bytes.saved > 0); + + const gated = dedup(cssEntangledShrinking, { savingsOnly: true }); + assert.strictEqual(gated.css, ungated.css); + assert.strictEqual(gated.applied.length, ungated.applied.length); + assert.strictEqual(gated.withheld, undefined); + }); + + test('Restores leading whitespace when a declined merge emptied two rules at the top of the file', () => { + const ungated = dedup(cssTwoLeadingRemovals); + assert.ok(ungated.bytes.saved < 0, 'fixture should grow the file when ungated'); + // The merge empties both leading rules before it is undone + assert.ok(!ungated.css.startsWith('.a {') && !ungated.css.includes('\n.b {')); + + for (const aggressive of [false, true]) { + const { css: output, applied, bytes } = dedup(cssTwoLeadingRemovals, { savingsOnly: true, aggressive }); + assert.strictEqual(output, cssTwoLeadingRemovals, `output should be restored byte for byte (aggressive: ${aggressive})`); + assert.strictEqual(applied.length, 0); + assert.strictEqual(bytes.saved, 0); + } + }); + + test('Prices a merge from the rules it touches, not from the whole style sheet', () => { + // Doubling the number of duplicate groups should roughly double the work + // the gate does. If pricing one merge re-serializes everything around it, + // the work grows with the file instead—which is how three separate + // quadratic regressions in this accounting first showed up. + const build = (groups, indent) => { + let css = ''; + for (let index = 0; index < groups; index++) { + const one = `.a${index}`.padEnd(28, 'x'); + const two = `.b${index}`.padEnd(28, 'y'); + css += `${indent}${one} { color: #${index % 900 + 100}; z-index: ${index}; }\n`; + css += `${indent}${two} { color: #${index % 900 + 100}; top: ${index}px; }\n`; + } + return css; + }; + // Both at the root and inside one shared block, where every group's + // enclosing subtree is the whole rest of the file + const shapes = { + root: groups => build(groups, ''), + block: groups => `@media (min-width: 40em) {\n${build(groups, ' ')}}\n`, + }; + + for (const [shape, make] of Object.entries(shapes)) { + const work = groups => { + const before = measuredByteTotal(); + dedup(make(groups), { savingsOnly: true }); + return measuredByteTotal() - before; + }; + const small = work(50); + const large = work(200); + // Four times the groups, so linear pricing lands near four times the + // work; the bound is loose enough for the fixed-point loop's extra + // passes, and far under the sixteen-fold a quadratic pass would cost + assert.ok( + large < small * 8, + `pricing scales with the file rather than the merge (${shape}): ${small} → ${large} bytes measured` + ); + } + }); + + test('Declining a merge leaves the merges around it byte-identical to an ungated run', () => { + // The gate must not perturb what it does not decline: The shrinking + // cluster has to come out exactly as it would on its own + const shrinkingAlone = '.p { margin: 0; padding: 0; }\n.q { margin: 0; top: 0; }\n.r { margin: 0; left: 0; }\n'; + const alone = dedup(shrinkingAlone).css; + const mixed = dedup(cssMixed, { savingsOnly: true }).css; + assert.ok(mixed.includes(alone.trim()), `the shrinking cluster should consolidate exactly as it does on its own, but got:\n${mixed}`); + }); }); diff --git a/test/helpers.js b/test/helpers.js index 9ef5974..785d48b 100644 --- a/test/helpers.js +++ b/test/helpers.js @@ -37,6 +37,74 @@ export const cssGrowingAggressive = [ '', ].join('\n'); +// One cluster that grows the file (the long selector list costs more than the +// removed `color` saves) and one that shrinks it (three short selectors share +// `margin`), in a single style sheet. Clusters are independent, so a per-merge +// gate applies the second and declines the first. +export const cssMixed = [ + '.very-long-selector-name-one { color: red; font-weight: bold; }', + '.b { color: red; }', + '.p { margin: 0; padding: 0; }', + '.q { margin: 0; top: 0; }', + '.r { margin: 0; left: 0; }', + '', +].join('\n'); + +// A growing merge on a *nesting host*, plus a shrinking one among the rules +// nested inside it. Declining the outer merge must leave the nested rules the +// inner scope already holds references to intact, not swap in copies. +export const cssNestedHost = [ + '.very-long-selector-name-one {', + ' color: red;', + '', + ' &:hover {', + ' top: 0;', + ' }', + '', + ' &:focus {', + ' top: 0;', + ' }', + '}', + '', + '.b { color: red; }', + '', +].join('\n'); + +// An entangled cluster: Three duplicate groups all sharing the hub rule, so +// they can only be merged as one coordinated whole. The selectors are long +// enough that the whole thing costs bytes, so `savingsOnly` must decline it +// together—and its identity-preserving rollback has to survive the several +// replacement rules the split creates. +export const cssEntangledGrowing = [ + '.hub-selector-that-is-long-here { color: red; top: 0; left: 0; }', + '.spoke-selector-number-one-here { color: red; }', + '.spoke-selector-number-two-here { top: 0; }', + '.spoke-selector-number-six-here { left: 0; }', + '', +].join('\n'); + +// The same shape where the merge pays off: Identical rules fold whole, so the +// coordinated merge removes rules instead of lengthening selector lists +export const cssEntangledShrinking = [ + '.a-really-quite-long-selector-one { color: red; top: 0; }', + '.a-really-quite-long-selector-two { color: red; top: 0; }', + '.a-really-quite-long-selector-six { color: red; top: 0; }', + '', +].join('\n'); + +// A declined merge that first empties the *two* leading root rules: PostCSS +// hands each removed first child’s `raws.before` to its successor, so the blank +// line before `.mid` travels down the chain and has to be handed back on +// rollback—byte for byte, or the file comes out reformatted. +export const cssTwoLeadingRemovals = [ + '.a { color: red; }', + '.b { color: red; }', + '', + '.mid { top: 0; }', + '.c-with-an-extremely-long-selector-name { color: red; padding: 0; }', + '', +].join('\n'); + // Only mergeable in aggressive mode (canonicalizing the `` values is // aggressive-only), and—unlike `cssGrowingAggressive`—the merge shrinks the // file: Each rule holds only the one shared declaration, so folding them diff --git a/test/plugin.test.js b/test/plugin.test.js index e4895f8..07576ec 100644 --- a/test/plugin.test.js +++ b/test/plugin.test.js @@ -47,7 +47,7 @@ describe('Plugin: Dedup', () => { const input = '.very-long-selector-name-one { color: red; font-weight: bold; }\n.b { color: red; }\n'; const result = await postcss([cssdedup({ fix: true, savingsOnly: true })]).process(input, { from: undefined }); assert.strictEqual(result.css, input); - assert.ok(result.warnings().some(warning => /Consolidation withheld \(`savingsOnly`\): 1 merge would make the style sheet \d+ bytes bigger/.test(warning.text))); + assert.ok(result.warnings().some(warning => /1 merge withheld \(`savingsOnly`\): applying it would make the style sheet \d+ bytes bigger/.test(warning.text))); }); test('`savingsOnly: true` still applies a shrinking consolidation', async () => {