From 95f994cf27aa4187664478b5832b7b1e081abae0 Mon Sep 17 00:00:00 2001 From: Jens Oliver Meiert Date: Tue, 8 Sep 2026 15:38:36 +0200 Subject: [PATCH] feat: ensure atomic file replacement in `--fix` mode Implemented an atomic replacement mechanism for files modified with `--fix`, preventing potential data loss during interrupted operations. Added tests to verify file mode preservation and symlink handling, and updated documentation and changelog to reflect the changes. (This commit message was AI-generated.) Signed-off-by: Jens Oliver Meiert --- CHANGELOG.md | 8 +++++++- README.md | 4 ++-- package-lock.json | 4 ++-- package.json | 2 +- src/cli/file-pass.js | 30 ++++++++++++++++++++++++++++-- src/cli/options.js | 2 +- test/cli.test.js | 39 +++++++++++++++++++++++++++++++++++++++ 7 files changed, 80 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 893c7e3..eefded1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,12 @@ 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.2] - 2026-09-08 + +### Fixed + +* Made `--fix` stage a replacement beside each target and atomically rename it into place, so an interrupted write cannot leave a CSS file empty; original file modes are retained, and a symlink argument continues to update its destination rather than replacing the link + ## [1.13.1] - 2026-09-08 ### Fixed @@ -223,4 +229,4 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), ### Added -* Released initial version \ No newline at end of file +* Released initial version diff --git a/README.md b/README.md index de1e35e..a8f4b7d 100644 --- a/README.md +++ b/README.md @@ -68,13 +68,13 @@ The two aren’t always aligned, though: Folding a declaration into a shared sel npx css-dedup [options] [path…] ``` -Pass one or more paths—each is analyzed (and, with `--fix`, rewritten) independently. Without a path, CSS Dedup analyzes the current directory. A directory is searched recursively for .css files (skipping node_modules and dotfolders); the result is unrolled into that same per-file list, so mixing files and directories works, too. Pass `-` instead of a file to read CSS from STDIN (can’t be combined with other file arguments); in `--fix` mode this prints the consolidated CSS to STDOUT, rather than writing a file, so it composes in a pipeline (status/summary output moves to STDERR in that case, keeping STDOUT pure CSS). +Pass one or more paths—each is analyzed (and, with `--fix`, atomically replaced) independently. Without a path, CSS Dedup analyzes the current directory. A directory is searched recursively for .css files (skipping node_modules and dotfolders); the result is unrolled into that same per-file list, so mixing files and directories works, too. Pass `-` instead of a file to read CSS from STDIN (can’t be combined with other file arguments); in `--fix` mode this prints the consolidated CSS to STDOUT, rather than writing a file, so it composes in a pipeline (status/summary output moves to STDERR in that case, keeping STDOUT pure CSS). The input is CSS. A preprocessor source named as an argument (.scss, .sass, .less, .styl) is skipped. Run CSS Dedup on the compiled style sheet instead—duplication in a preprocessor source is often deliberate (one mixin used in ten places), it only becomes real duplication after compilation, and the byte figures the report is built around describe what actually ships. The reason for skipping rather than trying: Constructs like `@include`, `@extend`, `#{…}`, and `@if` decide what a rule finally contains, which is exactly what the merge-safety checks would need to see to know whether moving a declaration across rules is safe. | Option | Description | | --- | --- | -| `--fix`, `-f` | Consolidate declarations that are safe to merge automatically, rewriting each file in place (or printing to STDOUT for `-`) | +| `--fix`, `-f` | Consolidate declarations that are safe to merge automatically, atomically replacing each file (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 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) | diff --git a/package-lock.json b/package-lock.json index d3d65a6..e1b5307 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "css-dedup", - "version": "1.13.1", + "version": "1.13.2", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "css-dedup", - "version": "1.13.1", + "version": "1.13.2", "license": "MIT", "dependencies": { "postcss": "^8.5.26" diff --git a/package.json b/package.json index 50a7513..a339422 100644 --- a/package.json +++ b/package.json @@ -60,5 +60,5 @@ }, "type": "module", "types": "src/index.d.ts", - "version": "1.13.1" + "version": "1.13.2" } diff --git a/src/cli/file-pass.js b/src/cli/file-pass.js index 630f019..eb077cb 100644 --- a/src/cli/file-pass.js +++ b/src/cli/file-pass.js @@ -2,7 +2,8 @@ // a structured-cloneable payload for `css-dedup.js` to render. Split out so the // same pass runs on the main thread or on a worker (see `pool.js`). -import { writeFile } from 'node:fs/promises'; +import { chmod, mkdtemp, realpath, rename, rm, stat, writeFile } from 'node:fs/promises'; +import { basename, dirname, join } from 'node:path'; import { analyze, dedup } from '../index.js'; import { declarationKey } from '../lib/normalization.js'; @@ -76,6 +77,31 @@ function computeReportPasses(css, targetOptions) { }; } +// Replace a file only after the complete new content is safely staged beside +// it. `realpath()` intentionally follows a final symlink: `writeFile()` used +// to write through one, and replacing the link itself would be surprising. +// `rename()` is atomic when both paths share a directory/filesystem, so an +// interruption can leave either version, never a target truncated to zero. +async function replaceFileAtomically(label, output) { + const target = await realpath(label); + const source = await stat(target); + const stageDir = await mkdtemp(join(dirname(target), `.${basename(target)}.css-dedup-`)); + const staged = join(stageDir, 'replacement.css'); + + try { + await writeFile(staged, output); + // `writeFile()` creates a fresh file using the process umask. Give the + // replacement the target’s mode before it becomes visible in its place. + await chmod(staged, source.mode); + await rename(staged, target); + } finally { + // Covers write/chmod/rename failures. A forced process termination cannot + // run cleanup, but it can at most leave this private, clearly named stage + // directory behind; the original target remains intact until `rename()`. + await rm(stageDir, { recursive: true, force: true }); + } +} + // `--fix`: consolidate, and write where there’s a file to write. The write // happens here, not at render time, so a parallel run does its I/O on the // worker. STDIN has no file, so its output rides back on the payload. @@ -100,7 +126,7 @@ async function computeFixPass(css, targetOptions, { isStdin, label }) { } const wrote = !isStdin && applied.length > 0; - if (wrote) await writeFile(label, output); + if (wrote) await replaceFileAtomically(label, output); return { mode: 'fix', diff --git a/src/cli/options.js b/src/cli/options.js index 4cbb607..6ff9e9b 100644 --- a/src/cli/options.js +++ b/src/cli/options.js @@ -30,7 +30,7 @@ Arguments: path One or more CSS files or directories to analyze, defaulting to the current directory (directories are searched recursively for .css files, skipping node_modules and dotfolders); pass \`-\` to read from STDIN instead. Preprocessor sources (.scss, .sass, .less, .styl) are skipped—run CSS Dedup on the compiled style sheet. Options: - -f, --fix Consolidate declarations that are safe to merge automatically, rewriting each file in place (or printing to STDOUT for \`-\`) + -f, --fix Consolidate declarations that are safe to merge automatically, atomically replacing each file (or printing to STDOUT for \`-\`) -a, --aggressive Also apply merges that are probably—but not provably—safe (test afterwards); only applies together with \`--fix\` -s, --savings-only Leave a file untouched when its consolidation would make it bigger, not smaller (checked per file); only applies together with \`--fix\` -i, --ignore-selector Regular expression for selectors to exclude from analysis (repeatable) diff --git a/test/cli.test.js b/test/cli.test.js index 91e0541..71a10f1 100644 --- a/test/cli.test.js +++ b/test/cli.test.js @@ -62,6 +62,45 @@ describe('CLI', () => { } }); + const skipFileModeTest = process.platform === 'win32' ? 'POSIX file modes are not available on Windows' : false; + + test('`--fix` atomically replaces a file while preserving its mode', { skip: skipFileModeTest }, () => { + const dirTemp = makeTempDir('temp_atomic_fix'); + const file = path.join(dirTemp, 'executable.css'); + fs.writeFileSync(file, '.a { color: red; }\n.b { color: red; }\n'); + fs.chmodSync(file, 0o754); + + try { + const { status } = run(['--fix', file]); + assert.strictEqual(status, 0); + assert.match(fs.readFileSync(file, 'utf8'), RE_MERGED_AB); + assert.strictEqual(fs.statSync(file).mode & 0o777, 0o754); + assert.deepStrictEqual(fs.readdirSync(dirTemp), ['executable.css']); + } finally { + fs.rmSync(dirTemp, { recursive: true, force: true }); + } + }); + + const skipSymlinkFixTest = process.platform === 'win32' ? 'creating symlinks requires extra Windows privileges' : false; + + test('`--fix` follows a symlink target instead of replacing the link', { skip: skipSymlinkFixTest }, () => { + const dirTemp = makeTempDir('temp_atomic_fix_symlink'); + const target = path.join(dirTemp, 'target.css'); + const link = path.join(dirTemp, 'linked.css'); + fs.writeFileSync(target, '.a { color: red; }\n.b { color: red; }\n'); + fs.symlinkSync('target.css', link); + + try { + const { status } = run(['--fix', link]); + assert.strictEqual(status, 0); + assert.ok(fs.lstatSync(link).isSymbolicLink()); + assert.match(fs.readFileSync(target, 'utf8'), RE_MERGED_AB); + assert.strictEqual(fs.readFileSync(link, 'utf8'), fs.readFileSync(target, 'utf8')); + } finally { + fs.rmSync(dirTemp, { recursive: true, force: true }); + } + }); + test('Runs `--fix` without a target once the prompt is answered', () => { const dirTemp = makeTempDir('temp_prompt_accept'); const file = path.join(dirTemp, 'prompted.css');