From 069ac9522f38071f592f78600b19e78db97a3ba3 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Wed, 26 Aug 2026 12:51:37 +1000 Subject: [PATCH 01/11] Surface build-clean's path matching bug with tests on all three platforms The plugin matches esbuild's metafile keys, which use forward slashes and are relative to absWorkingDir, against path.relative(process.cwd(), file). Where those two disagree every built file reads as unused, so with destructive: true the plugin deletes the build it just finished. The tests are expected to fail until that is fixed. The absWorkingDir case already fails on macOS and Linux; the separator case needs the Windows runner the new matrix adds. The diagnostic steps and the metafile probe exist to make that run readable and can come out once the behaviour is settled. --- .github/workflows/ci.yml | 10 ++- .github/workflows/test-matrix.yml | 69 ++++++++++++++++ .gitignore | 2 + packages/build-clean/package.json | 1 + .../build-clean/scripts/diagnose-paths.mjs | 66 ++++++++++++++++ .../build-clean/test/abs-working-dir.spec.ts | 29 +++++++ .../build-clean/test/esbuild-cleaning.spec.ts | 35 ++++++++ .../build-clean/test/plugin-surface.spec.ts | 37 +++++++++ .../build-clean/test/support/workspace.ts | 79 +++++++++++++++++++ 9 files changed, 327 insertions(+), 1 deletion(-) create mode 100644 .github/workflows/test-matrix.yml create mode 100644 packages/build-clean/scripts/diagnose-paths.mjs create mode 100644 packages/build-clean/test/abs-working-dir.spec.ts create mode 100644 packages/build-clean/test/esbuild-cleaning.spec.ts create mode 100644 packages/build-clean/test/plugin-surface.spec.ts create mode 100644 packages/build-clean/test/support/workspace.ts diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 73020ee..8dae24f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -25,6 +25,14 @@ jobs: uses: ./.github/workflows/checks.yml secrets: inherit + # The same test suite on linux, macos and windows. Separate from checks so the + # platform-irrelevant parts of checks (GitVersion, biome, changelog drift) are + # not tripled, and so a platform failure is legible as one. + test-matrix: + if: github.event_name == 'pull_request' || github.event_name == 'push' || github.event_name == 'workflow_dispatch' + uses: ./.github/workflows/test-matrix.yml + secrets: inherit + detect: if: github.event_name == 'pull_request' || github.event_name == 'push' || github.event_name == 'workflow_dispatch' runs-on: ubuntu-24.04 @@ -157,7 +165,7 @@ jobs: # --- Aggregate gate: one stable check name a branch ruleset can require. gate: - needs: [ checks, detect, build, prepare, release-build, publish ] + needs: [ checks, test-matrix, detect, build, prepare, release-build, publish ] if: always() runs-on: ubuntu-24.04 steps: diff --git a/.github/workflows/test-matrix.yml b/.github/workflows/test-matrix.yml new file mode 100644 index 0000000..288060a --- /dev/null +++ b/.github/workflows/test-matrix.yml @@ -0,0 +1,69 @@ +name: Test matrix + +# The workspace test suite on all three supported platforms. The runners cover +# two independent axes, so none of them is redundant: ubuntu is case-sensitive +# and unix, macos is case-insensitive and unix, windows is case-insensitive and +# not unix. +# +# checks.yml deliberately stays ubuntu-only. GitVersion, biome and the +# changelog-drift check are platform-irrelevant, and the drift check's +# `git diff --exit-code` would trip on line endings under Windows. +# +# The diagnostic steps run before the suite so their output is present whether +# the suite passes or fails. They can come out once the platform behaviour is +# settled. +on: + workflow_call: + +jobs: + test: + strategy: + fail-fast: false + matrix: + os: [ ubuntu-24.04, macos-14, windows-2022 ] + runs-on: ${{ matrix.os }} + steps: + - name: Keep line endings as committed + # Windows runners default core.autocrlf to true, which rewrites every + # checked-out text file. Path and byte handling is what this matrix + # exists to test, so the tree must be identical on all three. + shell: bash + run: git config --global core.autocrlf false + + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + - uses: ./.github/actions/setup + - run: pnpm i --frozen-lockfile + + - name: Runner diagnostics + shell: bash + env: + RUNNER_OS_NAME: ${{ runner.os }} + MATRIX_OS: ${{ matrix.os }} + run: | + echo "runner.os : $RUNNER_OS_NAME" + echo "matrix.os : $MATRIX_OS" + echo "pnpm : $(pnpm --version)" + node -e 'const p = require("node:path"); console.log("node :", process.version); console.log("platform :", process.platform); console.log("arch :", process.arch); console.log("path.sep :", JSON.stringify(p.sep)); console.log("cwd :", process.cwd());' + + - name: Filesystem case-sensitivity probe + shell: bash + run: | + probe_dir="$(mktemp -d)" + printf 'lower\n' > "$probe_dir/casetest.txt" + if [ -f "$probe_dir/CASETEST.TXT" ]; then + echo "filesystem: case-INsensitive" + else + echo "filesystem: case-sensitive" + fi + + - name: esbuild metafile path probe + # Observes esbuild alone, with the clean plugin not loaded, so its output + # is evidence about the metafile rather than about our matching code. + shell: bash + working-directory: packages/build-clean + run: node scripts/diagnose-paths.mjs + + - run: pnpm run --if-present build --only + - run: pnpm run --if-present test --only diff --git a/.gitignore b/.gitignore index 01f6b2b..5a9b23e 100644 --- a/.gitignore +++ b/.gitignore @@ -9,3 +9,5 @@ coverage/ CLAUDE.local.md *.log *.bak +packages/build-clean/test/.tmp/ +packages/build-clean/test/.diagnostics/ diff --git a/packages/build-clean/package.json b/packages/build-clean/package.json index 01a640a..a0bf341 100644 --- a/packages/build-clean/package.json +++ b/packages/build-clean/package.json @@ -185,6 +185,7 @@ ], "scripts": { "build": "tsup", + "test": "vitest run", "type-check": "tsc -p tsconfig.check.json", "watch": "tsup --watch" }, diff --git a/packages/build-clean/scripts/diagnose-paths.mjs b/packages/build-clean/scripts/diagnose-paths.mjs new file mode 100644 index 0000000..49b4b6e --- /dev/null +++ b/packages/build-clean/scripts/diagnose-paths.mjs @@ -0,0 +1,66 @@ +// Observes esbuild only - the clean plugin is deliberately not loaded here, so +// the output is evidence about esbuild's metafile, not about our code. Run from +// the package directory. Always exits 0: it reports, it does not judge. +import { mkdir, readdir, writeFile } from 'node:fs/promises'; +import { join, relative, sep } from 'node:path'; +import { build } from 'esbuild'; + +const root = join('test', '.diagnostics'); +const srcDir = join(root, 'src'); +const outDir = join(root, 'dist'); + +await mkdir(join(srcDir, 'nested'), { recursive: true }); +await writeFile(join(srcDir, 'main.ts'), "import { helper } from './nested/helper';\nconsole.log(helper());\n"); +await writeFile(join(srcDir, 'nested', 'helper.ts'), 'export const helper = () => 42;\n'); + +const result = await build({ + entryPoints: [join(srcDir, 'main.ts'), join(srcDir, 'nested', 'helper.ts')], + outdir: outDir, + bundle: true, + format: 'esm', + platform: 'node', + target: 'node22', + metafile: true, +}); + +const walk = async (dir) => { + const found = []; + for (const entry of await readdir(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + found.push(...(entry.isDirectory() ? await walk(full) : [full])); + } + return found; +}; + +const metafileKeys = Object.keys(result.metafile.outputs).sort(); +const walked = (await walk(outDir)).sort(); +const relatives = walked.map((file) => relative(process.cwd(), file)); + +console.log(`platform : ${process.platform}`); +console.log(`node : ${process.version}`); +console.log(`path.sep : ${JSON.stringify(sep)}`); +console.log(`process.cwd() : ${process.cwd()}`); +console.log(`outdir given : ${JSON.stringify(outDir)}`); +console.log(''); +console.log('esbuild metafile output keys (what the plugin matches against):'); +for (const key of metafileKeys) { + console.log(` ${JSON.stringify(key)}`); +} +console.log(''); +console.log('path.relative(cwd, walkedFile) (what the plugin computes per file):'); +for (const value of relatives) { + console.log(` ${JSON.stringify(value)}`); +} +console.log(''); + +const built = new Set(metafileKeys); +const missed = relatives.filter((value) => !built.has(value)); + +console.log(`matched : ${relatives.length - missed.length} of ${relatives.length}`); +if (missed.length > 0) { + console.log(''); + console.log('MISSED - with destructive: true the plugin deletes these freshly-built files:'); + for (const value of missed) { + console.log(` ${JSON.stringify(value)}`); + } +} diff --git a/packages/build-clean/test/abs-working-dir.spec.ts b/packages/build-clean/test/abs-working-dir.spec.ts new file mode 100644 index 0000000..1013852 --- /dev/null +++ b/packages/build-clean/test/abs-working-dir.spec.ts @@ -0,0 +1,29 @@ +import { resolve } from 'node:path'; +import { build } from 'esbuild'; +import { describe, expect, it, onTestFailed } from 'vitest'; +import cleanPlugin from '../src/esbuild'; +import { buildOptions, createCapturingLogger, createWorkspace, listOutput } from './support/workspace'; + +// esbuild keys the metafile relative to absWorkingDir; the plugin computes its +// side relative to process.cwd(). Everything here is absolute and inside the +// temp workspace, so the deletion this provokes cannot reach a real directory. +describe('absWorkingDir', () => { + it('keeps the files esbuild just built when absWorkingDir is not the process cwd', async () => { + const workspace = await createWorkspace('abs-working-dir'); + const logger = createCapturingLogger(); + onTestFailed(() => console.error(logger.lines.join('\n'))); + + await build({ + ...buildOptions(workspace), + absWorkingDir: resolve(workspace.root), + entryPoints: [resolve(workspace.srcDir, 'main.ts'), resolve(workspace.srcDir, 'nested', 'helper.ts')], + outdir: resolve(workspace.outDir), + plugins: [cleanPlugin({ destructive: true, logger })], + }); + + const expected = ['main.js', 'nested/helper.js']; + const actual = await listOutput(workspace.outDir); + + expect(actual).toEqual(expected); + }); +}); diff --git a/packages/build-clean/test/esbuild-cleaning.spec.ts b/packages/build-clean/test/esbuild-cleaning.spec.ts new file mode 100644 index 0000000..34fcfb7 --- /dev/null +++ b/packages/build-clean/test/esbuild-cleaning.spec.ts @@ -0,0 +1,35 @@ +import { writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { build } from 'esbuild'; +import { describe, expect, it, onTestFailed } from 'vitest'; +import cleanPlugin from '../src/esbuild'; +import { buildOptions, createCapturingLogger, createWorkspace, listOutput } from './support/workspace'; + +describe('esbuild output cleaning', () => { + it('keeps the files esbuild just built', async () => { + const workspace = await createWorkspace('keeps-built-files'); + const logger = createCapturingLogger(); + onTestFailed(() => console.error(logger.lines.join('\n'))); + + await build({ ...buildOptions(workspace), plugins: [cleanPlugin({ destructive: true, logger })] }); + + const expected = ['main.js', 'nested/helper.js']; + const actual = await listOutput(workspace.outDir); + + expect(actual).toEqual(expected); + }); + + it('removes a file esbuild did not build', async () => { + const workspace = await createWorkspace('removes-unused-files'); + const logger = createCapturingLogger(); + onTestFailed(() => console.error(logger.lines.join('\n'))); + await writeFile(join(workspace.outDir, 'stale.js'), '// left over from an earlier build\n'); + + await build({ ...buildOptions(workspace), plugins: [cleanPlugin({ destructive: true, logger })] }); + + const expected = false; + const actual = (await listOutput(workspace.outDir)).includes('stale.js'); + + expect(actual).toBe(expected); + }); +}); diff --git a/packages/build-clean/test/plugin-surface.spec.ts b/packages/build-clean/test/plugin-surface.spec.ts new file mode 100644 index 0000000..ba7950a --- /dev/null +++ b/packages/build-clean/test/plugin-surface.spec.ts @@ -0,0 +1,37 @@ +import { describe, expect, it } from 'vitest'; +import plugin from '../src'; + +// Behaviour-defining: the cleanup only ever runs from the esbuild hook. The +// other bundler entry points are published and importable but register nothing, +// so nothing cleans. These pin that as it stands today. +const cleanupCapableHooks = ['buildEnd', 'writeBundle', 'generateBundle', 'closeBundle']; + +// unplugin's factories return either one plugin or several, depending on the +// bundler, so both shapes are flattened before the hooks are looked for. +const cleanupHooksOn = (created: T | T[]): string[] => { + const plugins = Array.isArray(created) ? created : [created]; + return cleanupCapableHooks.filter((hook) => plugins.some((instance) => hook in instance)); +}; + +describe('plugin surface', () => { + it('registers the cleanup on the esbuild plugin', () => { + const expected = 'function'; + const actual = typeof plugin.esbuild({}).setup; + + expect(actual).toBe(expected); + }); + + it('registers no hook that could clean on the vite plugin', () => { + const expected: string[] = []; + const actual = cleanupHooksOn(plugin.vite({})); + + expect(actual).toEqual(expected); + }); + + it('registers no hook that could clean on the rollup plugin', () => { + const expected: string[] = []; + const actual = cleanupHooksOn(plugin.rollup({})); + + expect(actual).toEqual(expected); + }); +}); diff --git a/packages/build-clean/test/support/workspace.ts b/packages/build-clean/test/support/workspace.ts new file mode 100644 index 0000000..4c32f8a --- /dev/null +++ b/packages/build-clean/test/support/workspace.ts @@ -0,0 +1,79 @@ +import { mkdir, readdir, writeFile } from 'node:fs/promises'; +import { join, relative, sep } from 'node:path'; +import type { BuildOptions } from 'esbuild'; +import type { ILogger } from '../../src/types'; + +export type Workspace = { + root: string; + srcDir: string; + outDir: string; +}; + +// Two entry points in different directories, so esbuild emits a nested output +// path. A flat output would match on Windows by accident and prove nothing. +export const createWorkspace = async (name: string): Promise => { + const root = join('test', '.tmp', name); + const srcDir = join(root, 'src'); + const outDir = join(root, 'dist'); + + await mkdir(join(srcDir, 'nested'), { recursive: true }); + await mkdir(outDir, { recursive: true }); + await writeFile(join(srcDir, 'main.ts'), "import { helper } from './nested/helper';\nconsole.log(helper());\n"); + await writeFile(join(srcDir, 'nested', 'helper.ts'), 'export const helper = () => 42;\n'); + + return { root, srcDir, outDir }; +}; + +export const buildOptions = (workspace: Workspace): BuildOptions => + ({ + entryPoints: [join(workspace.srcDir, 'main.ts'), join(workspace.srcDir, 'nested', 'helper.ts')], + outdir: workspace.outDir, + bundle: true, + format: 'esm', + platform: 'node', + target: 'node22', + }) satisfies BuildOptions; + +// Normalised to forward slashes so the assertions are the same on every +// platform. A test that failed on Windows because its own expectation used the +// wrong separator would prove nothing about the plugin. +export const listOutput = async (outDir: string): Promise => { + const walk = async (dir: string): Promise => { + const found: string[] = []; + for (const entry of await readdir(dir, { withFileTypes: true })) { + const full = join(dir, entry.name); + found.push(...(entry.isDirectory() ? await walk(full) : [full])); + } + return found; + }; + + try { + const files = await walk(outDir); + return files.map((file) => relative(outDir, file).split(sep).join('/')).sort(); + } catch { + // removeEmptyDirs rmdir's the output directory once it has emptied it, so + // "everything was deleted" arrives here as a missing directory, not as an + // empty one. + return []; + } +}; + +export type CapturingLogger = ILogger & { lines: string[] }; + +export const createCapturingLogger = (): CapturingLogger => { + const lines: string[] = []; + const record = + (level: string) => + (message: string, ...args: unknown[]) => { + lines.push([`[${level}]`, message, ...args.map((arg) => String(arg))].join(' ')); + }; + + return { + lines, + debug: record('debug'), + verbose: record('verbose'), + info: record('info'), + warn: record('warn'), + error: record('error'), + }; +}; From 1ab8115ba604b6df8681d0177751098339e727b8 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Wed, 26 Aug 2026 14:23:19 +1000 Subject: [PATCH 02/11] Define what build-clean does with an output differing only by case The plugin matches names exactly, so a stale output whose name differs from a built one only by case means different things per filesystem. On Linux the two are separate files and the stale one is genuinely unused. On macOS and Windows they are one file, and the case-preserving directory entry keeps the stale spelling, so the plugin reads the file esbuild just wrote as unused and deletes it. The built file surviving is the expectation on every platform. It already fails on macOS, which puts this defect on more than just Windows. --- .../build-clean/test/case-mismatch.spec.ts | 40 +++++++++++++++++++ .../build-clean/test/support/workspace.ts | 11 +++++ 2 files changed, 51 insertions(+) create mode 100644 packages/build-clean/test/case-mismatch.spec.ts diff --git a/packages/build-clean/test/case-mismatch.spec.ts b/packages/build-clean/test/case-mismatch.spec.ts new file mode 100644 index 0000000..2bad286 --- /dev/null +++ b/packages/build-clean/test/case-mismatch.spec.ts @@ -0,0 +1,40 @@ +import { writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { build } from 'esbuild'; +import { describe, expect, it, onTestFailed } from 'vitest'; +import cleanPlugin from '../src/esbuild'; +import { buildOptions, createCapturingLogger, createWorkspace, filesystemIsCaseInsensitive, listOutput } from './support/workspace'; + +// A stale output whose name differs from a built one only by case. The plugin +// matches names exactly, so what that means depends on the filesystem: on Linux +// the two are separate files and the stale one really is unused, while on macOS +// and Windows they are one file and removing it removes the build. +describe('output differing from a built file only by case', () => { + it('keeps the file esbuild built', async () => { + const workspace = await createWorkspace('case-mismatch-keeps-built'); + const logger = createCapturingLogger(); + onTestFailed(() => console.error(logger.lines.join('\n'))); + await writeFile(join(workspace.outDir, 'Main.js'), '// stale, differs from the built main.js only by case\n'); + + await build({ ...buildOptions(workspace), plugins: [cleanPlugin({ destructive: true, logger })] }); + + const expected = true; + const actual = (await listOutput(workspace.outDir)).some((file) => file.toLowerCase() === 'main.js'); + + expect(actual).toBe(expected); + }); + + it.skipIf(filesystemIsCaseInsensitive())('removes the stale file where it is a separate file', async () => { + const workspace = await createWorkspace('case-mismatch-removes-stale'); + const logger = createCapturingLogger(); + onTestFailed(() => console.error(logger.lines.join('\n'))); + await writeFile(join(workspace.outDir, 'Main.js'), '// stale, differs from the built main.js only by case\n'); + + await build({ ...buildOptions(workspace), plugins: [cleanPlugin({ destructive: true, logger })] }); + + const expected = false; + const actual = (await listOutput(workspace.outDir)).includes('Main.js'); + + expect(actual).toBe(expected); + }); +}); diff --git a/packages/build-clean/test/support/workspace.ts b/packages/build-clean/test/support/workspace.ts index 4c32f8a..a5a883a 100644 --- a/packages/build-clean/test/support/workspace.ts +++ b/packages/build-clean/test/support/workspace.ts @@ -1,3 +1,4 @@ +import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; import { mkdir, readdir, writeFile } from 'node:fs/promises'; import { join, relative, sep } from 'node:path'; import type { BuildOptions } from 'esbuild'; @@ -58,6 +59,16 @@ export const listOutput = async (outDir: string): Promise => { } }; +// Whether two names differing only by case are the same file here. Linux says +// no, macOS and Windows say yes, and that is the whole difference the +// case-mismatch behaviour turns on. +export const filesystemIsCaseInsensitive = (): boolean => { + const probeDir = join('test', '.tmp', 'case-probe'); + mkdirSync(probeDir, { recursive: true }); + writeFileSync(join(probeDir, 'probe.txt'), 'probe\n'); + return existsSync(join(probeDir, 'PROBE.TXT')); +}; + export type CapturingLogger = ILogger & { lines: string[] }; export const createCapturingLogger = (): CapturingLogger => { From c039ebf44c982b0e8fcbf20937b678cef1d46adf Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Wed, 26 Aug 2026 19:21:06 +1000 Subject: [PATCH 03/11] Match built files by filesystem identity instead of by path string The plugin compared esbuild's metafile keys against path.relative output. Those disagree in three ways: esbuild always writes forward slashes, its keys are relative to absWorkingDir rather than to the process working directory, and a case-folding filesystem reports a stored name that need not match the one esbuild wrote. Each disagreement marked a freshly built file as unused, so with destructive: true the plugin deleted the build it had just finished. Asking the filesystem whether two paths are the same file settles all three at once, with no platform switch and no guess about case sensitivity. The output directory now resolves against esbuild's working directory as well, so a relative outdir under a different absWorkingDir no longer walks an unrelated project's output. --- .../build-clean/scripts/diagnose-paths.mjs | 45 +++++++++++++++++-- .../build-clean/src/core/cleanUnusedFiles.ts | 32 +++++++++---- packages/build-clean/src/core/fileIdentity.ts | 13 ++++++ .../build-clean/src/core/pluginFactory.ts | 7 ++- .../build-clean/src/core/validateOutDir.ts | 29 ++++++------ .../build-clean/test/relative-outdir.spec.ts | 39 ++++++++++++++++ 6 files changed, 137 insertions(+), 28 deletions(-) create mode 100644 packages/build-clean/src/core/fileIdentity.ts create mode 100644 packages/build-clean/test/relative-outdir.spec.ts diff --git a/packages/build-clean/scripts/diagnose-paths.mjs b/packages/build-clean/scripts/diagnose-paths.mjs index 49b4b6e..fd6bdf6 100644 --- a/packages/build-clean/scripts/diagnose-paths.mjs +++ b/packages/build-clean/scripts/diagnose-paths.mjs @@ -1,8 +1,8 @@ // Observes esbuild only - the clean plugin is deliberately not loaded here, so // the output is evidence about esbuild's metafile, not about our code. Run from // the package directory. Always exits 0: it reports, it does not judge. -import { mkdir, readdir, writeFile } from 'node:fs/promises'; -import { join, relative, sep } from 'node:path'; +import { mkdir, readdir, stat, writeFile } from 'node:fs/promises'; +import { join, relative, resolve, sep } from 'node:path'; import { build } from 'esbuild'; const root = join('test', '.diagnostics'); @@ -56,11 +56,48 @@ console.log(''); const built = new Set(metafileKeys); const missed = relatives.filter((value) => !built.has(value)); -console.log(`matched : ${relatives.length - missed.length} of ${relatives.length}`); +console.log(`string match : ${relatives.length - missed.length} of ${relatives.length}`); if (missed.length > 0) { console.log(''); - console.log('MISSED - with destructive: true the plugin deletes these freshly-built files:'); + console.log('MISSED by string comparison - these are freshly-built files:'); for (const value of missed) { console.log(` ${JSON.stringify(value)}`); } } + +// Identity matching is what the plugin uses instead of string comparison. dev +// and ino are printed raw because ino is the part whose value on Windows cannot +// be assumed: a zero there would collapse every file onto one identity. +console.log(''); +console.log('stat identity, from the metafile key:'); +const identityOf = async (path) => { + try { + const stats = await stat(path); + return `${stats.dev}:${stats.ino}`; + } catch (error) { + return `unreadable (${error.code})`; + } +}; + +const builtIdentities = new Set(); +for (const key of metafileKeys) { + const identity = await identityOf(resolve(process.cwd(), key)); + builtIdentities.add(identity); + console.log(` ${JSON.stringify(key)} -> ${identity}`); +} + +console.log(''); +console.log('stat identity, from the walked file:'); +let identityMatches = 0; +for (const file of walked) { + const identity = await identityOf(file); + const hit = builtIdentities.has(identity); + if (hit) { + identityMatches++; + } + console.log(` ${JSON.stringify(file)} -> ${identity} ${hit ? 'MATCH' : 'NO MATCH'}`); +} + +console.log(''); +console.log(`identity match : ${identityMatches} of ${walked.length}`); +console.log(`distinct built identities : ${builtIdentities.size} (expected ${metafileKeys.length}; fewer means ino is not distinguishing files)`); diff --git a/packages/build-clean/src/core/cleanUnusedFiles.ts b/packages/build-clean/src/core/cleanUnusedFiles.ts index 8cb2028..00fbb54 100644 --- a/packages/build-clean/src/core/cleanUnusedFiles.ts +++ b/packages/build-clean/src/core/cleanUnusedFiles.ts @@ -1,20 +1,24 @@ -import { relative } from 'node:path'; +import { relative, resolve } from 'node:path'; import { Feature } from '../enums'; import { deleteFile } from './deleteFile'; +import { fileIdentity } from './fileIdentity'; import { getAllFiles } from './getAllFiles'; import { removeEmptyDirs } from './removeEmptyDirs'; import type { ResolvedOptions } from './types'; import { validateOutDir } from './validateOutDir'; -export async function cleanUnusedFiles(outDir: string, builtFiles: Set, options: ResolvedOptions): Promise { +// baseDir is esbuild's working directory, which is what its metafile paths are +// relative to. It is not always the process working directory. +export async function cleanUnusedFiles(outDir: string, builtFiles: Set, baseDir: string, options: ResolvedOptions): Promise { const { logger } = options; - validateOutDir(outDir, logger); + const resolvedOutDir = resolve(baseDir, outDir); + validateOutDir(resolvedOutDir, baseDir, logger); try { - logger.debug(`Starting cleanup of directory: "${outDir}"`); + logger.debug(`Starting cleanup of directory: "${resolvedOutDir}"`); logger.debug(`Built files count: ${builtFiles.size}`); - const existingFiles = await getAllFiles(outDir, logger); + const existingFiles = await getAllFiles(resolvedOutDir, logger); logger.debug(`Existing files count: ${existingFiles.length}`); if (existingFiles.length === 0 && builtFiles.size > 0) { @@ -24,13 +28,23 @@ export async function cleanUnusedFiles(outDir: string, builtFiles: Set, logger.info(`Processing ${existingFiles.length} existing files vs ${builtFiles.size} built files`); + const builtIdentities = new Set(); + for (const builtFile of builtFiles) { + const identity = await fileIdentity(resolve(baseDir, builtFile)); + if (identity !== undefined) { + builtIdentities.add(identity); + } + } + logger.debug(`Resolved ${builtIdentities.size} of ${builtFiles.size} built files on disk`); + const filesToDelete: string[] = []; for (const file of existingFiles) { - const relativePath = relative(process.cwd(), file); + const relativePath = relative(baseDir, file); logger.verbose(`Checking file: "${relativePath}"`); - if (!builtFiles.has(relativePath)) { + const identity = await fileIdentity(file); + if (identity === undefined || !builtIdentities.has(identity)) { filesToDelete.push(file); logger.verbose(`Marked for deletion: "${relativePath}"`); } else { @@ -47,7 +61,7 @@ export async function cleanUnusedFiles(outDir: string, builtFiles: Set, let deletedCount = 0; for (const file of filesToDelete) { - const relativePath = relative(process.cwd(), file); + const relativePath = relative(baseDir, file); logger.info(`Deleting: "${relativePath}"`); if (options.destructive) { @@ -64,7 +78,7 @@ export async function cleanUnusedFiles(outDir: string, builtFiles: Set, } if (options.features[Feature.RemoveEmptyDirs]) { - await removeEmptyDirs(outDir, options); + await removeEmptyDirs(resolvedOutDir, options); } } catch (error) { logger.error('Error during cleanup:', error); diff --git a/packages/build-clean/src/core/fileIdentity.ts b/packages/build-clean/src/core/fileIdentity.ts new file mode 100644 index 0000000..dc73162 --- /dev/null +++ b/packages/build-clean/src/core/fileIdentity.ts @@ -0,0 +1,13 @@ +import { stat } from 'node:fs/promises'; + +// Asks the filesystem whether two paths are the same file, rather than deciding +// it from the strings. That is what makes the match correct on a case-folding +// filesystem and independent of which separator each side happens to use. +export const fileIdentity = async (path: string): Promise => { + try { + const stats = await stat(path); + return `${stats.dev}:${stats.ino}`; + } catch { + return undefined; + } +}; diff --git a/packages/build-clean/src/core/pluginFactory.ts b/packages/build-clean/src/core/pluginFactory.ts index dc224f0..34cbc14 100644 --- a/packages/build-clean/src/core/pluginFactory.ts +++ b/packages/build-clean/src/core/pluginFactory.ts @@ -29,7 +29,12 @@ export const pluginFactory: UnpluginFactory = (initialOptio const builtFiles = new Set(Object.keys(result.metafile.outputs)); logger.debug(`Found ${builtFiles.size} built files in metafile for directory: "${outdir}"`); - await cleanUnusedFiles(outdir, builtFiles, options); + // Metafile paths are relative to esbuild's working directory, which + // only defaults to the process one. + const baseDir = build.initialOptions.absWorkingDir ?? process.cwd(); + logger.debug(`Base directory: "${baseDir}"`); + + await cleanUnusedFiles(outdir, builtFiles, baseDir, options); }); }, }, diff --git a/packages/build-clean/src/core/validateOutDir.ts b/packages/build-clean/src/core/validateOutDir.ts index 26fc5d9..c71c752 100644 --- a/packages/build-clean/src/core/validateOutDir.ts +++ b/packages/build-clean/src/core/validateOutDir.ts @@ -1,37 +1,38 @@ import { relative, resolve } from 'node:path'; import type { ILogger } from '../types'; -export const validateOutDir = (outDir: string, logger: ILogger) => { - const cwd = process.cwd(); +// baseDir is esbuild's working directory. The project to protect is the one the +// build is rooted at, which is not always the process working directory. +export const validateOutDir = (outDir: string, baseDir: string, logger: ILogger) => { const resolvedOutDir = resolve(outDir); - const relativePath = relative(cwd, resolvedOutDir); + const relativePath = relative(baseDir, resolvedOutDir); const normalizedPath = outDir.replace(/\\/g, '/'); - const isAbsolutePath = resolve(outDir) !== resolve(cwd, outDir); - const isSameAsCurrentDir = resolvedOutDir === cwd; - const isParentOfCurrentDirUnix = cwd.startsWith(`${resolvedOutDir}/`); - const isParentOfCurrentDirWindows = cwd.startsWith(`${resolvedOutDir}\\`); + const isAbsolutePath = resolve(outDir) !== resolve(baseDir, outDir); + const isSameAsCurrentDir = resolvedOutDir === baseDir; + const isParentOfCurrentDirUnix = baseDir.startsWith(`${resolvedOutDir}/`); + const isParentOfCurrentDirWindows = baseDir.startsWith(`${resolvedOutDir}\\`); const isParentOfCurrentDir = isParentOfCurrentDirUnix || isParentOfCurrentDirWindows; const goesUpDirectory = relativePath.startsWith('..'); logger.verbose('Path validation:'); logger.verbose(` Input: "${outDir}"`); - logger.verbose(` Current working directory: "${cwd}"`); + logger.verbose(` Base directory: "${baseDir}"`); logger.verbose(` Resolved output directory: "${resolvedOutDir}"`); - logger.verbose(` Relative path from cwd: "${relativePath}"`); + logger.verbose(` Relative path from base directory: "${relativePath}"`); logger.verbose(` Normalized path: "${normalizedPath}"`); logger.verbose(` Is absolute path outside project: ${isAbsolutePath}`); logger.verbose(` Is same as current directory: ${isSameAsCurrentDir}`); - logger.verbose(` Is parent of current directory (Unix): ${isParentOfCurrentDirUnix}`); - logger.verbose(` Is parent of current directory (Windows): ${isParentOfCurrentDirWindows}`); - logger.verbose(` Is parent of current directory: ${isParentOfCurrentDir}`); + logger.verbose(` Is parent of base directory (Unix): ${isParentOfCurrentDirUnix}`); + logger.verbose(` Is parent of base directory (Windows): ${isParentOfCurrentDirWindows}`); + logger.verbose(` Is parent of base directory: ${isParentOfCurrentDir}`); logger.verbose(` Goes up directory levels: ${goesUpDirectory}`); - // Check if the resolved path is the same as current directory + // Check if the resolved path is the same as the base directory if (isSameAsCurrentDir) { throw new Error(`[build-cleaner] Refusing to clean current directory: "${outDir}". Use a subdirectory like "dist" or "build".`); } - // Check if the resolved path is a parent of current directory + // Check if the resolved path is a parent of the base directory if (isParentOfCurrentDir) { throw new Error(`[build-cleaner] Refusing to clean parent directory: "${outDir}". This would delete the current project.`); } diff --git a/packages/build-clean/test/relative-outdir.spec.ts b/packages/build-clean/test/relative-outdir.spec.ts new file mode 100644 index 0000000..9386b62 --- /dev/null +++ b/packages/build-clean/test/relative-outdir.spec.ts @@ -0,0 +1,39 @@ +import { writeFile } from 'node:fs/promises'; +import { join, resolve } from 'node:path'; +import { build } from 'esbuild'; +import { describe, expect, it, onTestFailed } from 'vitest'; +import cleanPlugin from '../src/esbuild'; +import { buildOptions, createCapturingLogger, createWorkspace } from './support/workspace'; + +const wouldDelete = (lines: string[]): string[] => + lines + .filter((line) => line.startsWith('[info] Deleting: "')) + .map((line) => line.slice('[info] Deleting: "'.length, -1).split('\\').join('/')) + .sort(); + +// A relative outdir with absWorkingDir set is the dangerous shape: the directory +// to walk sits under esbuild's working directory, so resolving it against the +// process one reaches an unrelated project's output. Deliberately not +// destructive, so a regression here reports the wrong target instead of +// emptying it. +describe('relative outdir under a different absWorkingDir', () => { + it('considers only files under esbuild working directory for deletion', async () => { + const workspace = await createWorkspace('relative-outdir'); + const logger = createCapturingLogger(); + onTestFailed(() => console.error(logger.lines.join('\n'))); + await writeFile(join(workspace.outDir, 'stale.js'), '// left over from an earlier build\n'); + + await build({ + ...buildOptions(workspace), + absWorkingDir: resolve(workspace.root), + entryPoints: [resolve(workspace.srcDir, 'main.ts'), resolve(workspace.srcDir, 'nested', 'helper.ts')], + outdir: 'dist', + plugins: [cleanPlugin({ destructive: false, logger })], + }); + + const expected = ['dist/stale.js']; + const actual = wouldDelete(logger.lines); + + expect(actual).toEqual(expected); + }); +}); From 2756f08ab0a0d5f3ee59ac12d181c7344a5a48cf Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Wed, 26 Aug 2026 19:54:27 +1000 Subject: [PATCH 04/11] Prove the Azure Functions adapter builds a broken import specifier on Windows The adapter read two native paths with posix semantics and used the result as an ES import specifier. On Windows that is wrong twice over: posix.relative finds no common root in backslash paths so it emits the whole absolute target, and the backslashes it keeps are then consumed as escape sequences when esbuild parses the generated source. That is the "D:aecosystemecosystem" resolution failure in the Windows build. The computation moves into a named function taking the path semantics as an argument, defaulting to posix, so nothing behaves differently yet. Two tests fail: the separator is never converted, which fails everywhere, and posix semantics are wrong for native paths, which only a Windows runner can show. --- .../src/adapter.ts | 5 ++- .../src/toImportSpecifier.ts | 6 +++ .../test/toImportSpecifier.spec.ts | 40 +++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) create mode 100644 packages/svelte-adapter-azure-functions/src/toImportSpecifier.ts create mode 100644 packages/svelte-adapter-azure-functions/test/toImportSpecifier.spec.ts diff --git a/packages/svelte-adapter-azure-functions/src/adapter.ts b/packages/svelte-adapter-azure-functions/src/adapter.ts index f8382cb..ffd87ef 100644 --- a/packages/svelte-adapter-azure-functions/src/adapter.ts +++ b/packages/svelte-adapter-azure-functions/src/adapter.ts @@ -1,9 +1,10 @@ import { writeFileSync } from 'node:fs'; -import { join, posix } from 'node:path'; +import { join } from 'node:path'; import { fileURLToPath } from 'node:url'; import type { Adapter, Builder } from '@sveltejs/kit'; import { type BuildOptions, build } from 'esbuild'; import { defaults } from './defaults'; +import { toImportSpecifier } from './toImportSpecifier'; export interface AzureFunctionsAdapterOptions { esbuildOptions?: BuildOptions; @@ -29,7 +30,7 @@ export const createAdapter = (options: AzureFunctionsAdapterOptions = {}): Adapt const distFiles = fileURLToPath(new URL('../dist', import.meta.url)); - const relativePath = posix.relative(tmp, join(builder.getServerDirectory())); + const relativePath = toImportSpecifier(tmp, join(builder.getServerDirectory())); builder.log.minor('Generating serverless function...'); builder.copy(distFiles, tmp, { diff --git a/packages/svelte-adapter-azure-functions/src/toImportSpecifier.ts b/packages/svelte-adapter-azure-functions/src/toImportSpecifier.ts new file mode 100644 index 0000000..6af3f45 --- /dev/null +++ b/packages/svelte-adapter-azure-functions/src/toImportSpecifier.ts @@ -0,0 +1,6 @@ +import { posix } from 'node:path'; + +// The result is written into generated JavaScript as an import specifier, so it +// has to be relative and forward-slashed whatever the host platform writes. +// `paths` is the semantics the two arguments are written in. +export const toImportSpecifier = (from: string, to: string, paths: typeof posix = posix): string => paths.relative(from, to); diff --git a/packages/svelte-adapter-azure-functions/test/toImportSpecifier.spec.ts b/packages/svelte-adapter-azure-functions/test/toImportSpecifier.spec.ts new file mode 100644 index 0000000..464d16a --- /dev/null +++ b/packages/svelte-adapter-azure-functions/test/toImportSpecifier.spec.ts @@ -0,0 +1,40 @@ +import { posix, resolve, win32 } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { toImportSpecifier } from '../src/toImportSpecifier'; + +describe('toImportSpecifier', () => { + it('builds a relative specifier from posix paths', () => { + const from = '/project/.svelte-kit/adapter-azure-functions'; + const to = '/project/.svelte-kit/output/server'; + + const expected = '../output/server'; + const actual = toImportSpecifier(from, to, posix); + + expect(actual).toBe(expected); + }); + + // Fails wherever the platform's own separator is not the posix one: the + // result keeps the separator it was given, and an import specifier cannot. + it('builds a forward-slashed specifier from Windows paths', () => { + const from = 'D:\\project\\.svelte-kit\\adapter-azure-functions'; + const to = 'D:\\project\\.svelte-kit\\output\\server'; + + const expected = '../output/server'; + const actual = toImportSpecifier(from, to, win32); + + expect(actual).toBe(expected); + }); + + // The other half, and the one only a Windows runner can catch: the paths the + // adapter really passes are in the running platform's form, so posix + // semantics are the wrong ones to read them with anywhere but unix. + it('builds a forward-slashed specifier from paths in the running platform form', () => { + const from = resolve('project', '.svelte-kit', 'adapter-azure-functions'); + const to = resolve('project', '.svelte-kit', 'output', 'server'); + + const expected = '../output/server'; + const actual = toImportSpecifier(from, to); + + expect(actual).toBe(expected); + }); +}); From a1bf9c17219be40ff0ec14aad6aadb4087286572 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Thu, 27 Aug 2026 11:11:38 +1000 Subject: [PATCH 05/11] Read the adapter's paths with the running platform's semantics The import specifier is now built with the platform's own relative, then converted to forward slashes for the generated source. Reading native Windows paths as posix ones produced a specifier carrying the whole absolute target and its backslashes, which esbuild then failed to resolve. The single-argument join at the call site went with it. It normalised a path that relative normalises anyway. --- packages/svelte-adapter-azure-functions/src/adapter.ts | 2 +- .../src/toImportSpecifier.ts | 7 ++++--- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/svelte-adapter-azure-functions/src/adapter.ts b/packages/svelte-adapter-azure-functions/src/adapter.ts index ffd87ef..2a82535 100644 --- a/packages/svelte-adapter-azure-functions/src/adapter.ts +++ b/packages/svelte-adapter-azure-functions/src/adapter.ts @@ -30,7 +30,7 @@ export const createAdapter = (options: AzureFunctionsAdapterOptions = {}): Adapt const distFiles = fileURLToPath(new URL('../dist', import.meta.url)); - const relativePath = toImportSpecifier(tmp, join(builder.getServerDirectory())); + const relativePath = toImportSpecifier(tmp, builder.getServerDirectory()); builder.log.minor('Generating serverless function...'); builder.copy(distFiles, tmp, { diff --git a/packages/svelte-adapter-azure-functions/src/toImportSpecifier.ts b/packages/svelte-adapter-azure-functions/src/toImportSpecifier.ts index 6af3f45..1d62b22 100644 --- a/packages/svelte-adapter-azure-functions/src/toImportSpecifier.ts +++ b/packages/svelte-adapter-azure-functions/src/toImportSpecifier.ts @@ -1,6 +1,7 @@ -import { posix } from 'node:path'; +import path, { type posix } from 'node:path'; // The result is written into generated JavaScript as an import specifier, so it // has to be relative and forward-slashed whatever the host platform writes. -// `paths` is the semantics the two arguments are written in. -export const toImportSpecifier = (from: string, to: string, paths: typeof posix = posix): string => paths.relative(from, to); +// `paths` is the semantics the two arguments are written in, and defaults to +// the running platform's, because that is the form the build tools hand over. +export const toImportSpecifier = (from: string, to: string, paths: typeof posix = path): string => paths.relative(from, to).split(paths.sep).join('/'); From d860fa7ca417c02ae0af2e858780bfaed28273b4 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Thu, 27 Aug 2026 12:27:50 +1000 Subject: [PATCH 06/11] Stop build-graphql's watch tests asserting a path separator The glob results these tests check go to esbuild's watchFiles and vite's addWatchFile, which take paths in the platform's own form. Nothing compares them against the forward-slashed paths findGraphQLFiles produces for generated source, so the separator carries no meaning here and the tests should not have been pinning it. Which files are registered is the behaviour, and that is what they assert now. The root build and test scripts also run to completion rather than stopping at the first failing task. One package failing was cancelling the rest, which hid every other package's result on a platform and cost two runs before the Windows column could be read at all. --- package.json | 4 ++-- .../build-graphql/test/esbuild-watch/esbuild-watch.spec.ts | 3 ++- packages/build-graphql/test/toPosix.ts | 6 ++++++ packages/build-graphql/test/vite-watch/vite-watch.spec.ts | 5 +++-- 4 files changed, 13 insertions(+), 5 deletions(-) create mode 100644 packages/build-graphql/test/toPosix.ts diff --git a/package.json b/package.json index bbfce43..c1d9c65 100644 --- a/package.json +++ b/package.json @@ -4,8 +4,8 @@ "private": true, "packageManager": "pnpm@11.8.0+sha512.c1f5e7c4cb241c8f174b743851d82f42b802324afc8b0f116b96adb15aa06664948dde36960a3ba1079ba5b4b29dd0140135b94b5b5f5263592249d68e555f26", "scripts": { - "build": "turbo run build", - "test": "turbo run test", + "build": "turbo run build --continue=dependencies-successful", + "test": "turbo run test --continue=dependencies-successful", "type-check": "turbo run type-check", "lint": "biome lint", "format": "biome format", diff --git a/packages/build-graphql/test/esbuild-watch/esbuild-watch.spec.ts b/packages/build-graphql/test/esbuild-watch/esbuild-watch.spec.ts index 1d834c4..c70582b 100644 --- a/packages/build-graphql/test/esbuild-watch/esbuild-watch.spec.ts +++ b/packages/build-graphql/test/esbuild-watch/esbuild-watch.spec.ts @@ -1,5 +1,6 @@ import { describe, expect, it } from 'vitest'; import { Feature, type Options } from '../../src'; +import { toPosix } from '../toPosix'; import { runEsbuildSetup } from './runEsbuildSetup'; describe('esbuild watch feature', () => { @@ -72,7 +73,7 @@ describe('esbuild watch feature', () => { with: {}, }); - const actual = result?.watchFiles; + const actual = toPosix(result?.watchFiles ?? []); const expected = ['test/mutation.graphql', 'test/query.graphql', 'test/schema.spec.graphql', 'test/sub/schema.graphql']; expect(actual).toEqual(expected); diff --git a/packages/build-graphql/test/toPosix.ts b/packages/build-graphql/test/toPosix.ts new file mode 100644 index 0000000..a01f0a7 --- /dev/null +++ b/packages/build-graphql/test/toPosix.ts @@ -0,0 +1,6 @@ +import { sep } from 'node:path'; + +// glob returns paths in the platform's own form, and the watch registrations +// these feed take them either way. Which files get registered is the behaviour; +// the separator is not, so it is normalised out of the comparison. +export const toPosix = (paths: string[]): string[] => paths.map((path) => path.split(sep).join('/')); diff --git a/packages/build-graphql/test/vite-watch/vite-watch.spec.ts b/packages/build-graphql/test/vite-watch/vite-watch.spec.ts index c23223c..d340ab7 100644 --- a/packages/build-graphql/test/vite-watch/vite-watch.spec.ts +++ b/packages/build-graphql/test/vite-watch/vite-watch.spec.ts @@ -5,6 +5,7 @@ import { virtualModuleId } from '../../src/core/consts'; import { resolveVirtualId } from '../../src/core/resolveVirtualId'; import { handleHotUpdate } from '../../src/core/vite/viteHotUpdate'; import { InvalidFeatureCombinationError } from '../../src/errors/InvalidFeatureCombinationError'; +import { toPosix } from '../toPosix'; import { expectToThrowErrorWithFields } from './expectToThrowErrorWithFields'; import { makeViteRun } from './makeViteRun'; @@ -113,7 +114,7 @@ describe('vite watch/hmr features', () => { const { ctx, runBuild } = makeViteRun(features); await runBuild(); - const actual = ctx.addWatchFile.mock.calls.map(([p]) => p); + const actual = toPosix(ctx.addWatchFile.mock.calls.map(([p]) => p)); const expected = ['test/mutation.graphql', 'test/query.graphql', 'test/schema.spec.graphql', 'test/sub/schema.graphql']; expect(actual).toEqual(expected); @@ -153,7 +154,7 @@ describe('vite watch/hmr features', () => { const { ctx, runBuild } = makeViteRun(features); await runBuild(); - const actual = ctx.addWatchFile.mock.calls.map(([p]) => p); + const actual = toPosix(ctx.addWatchFile.mock.calls.map(([p]) => p)); const expected = ['test/mutation.graphql', 'test/query.graphql', 'test/schema.spec.graphql', 'test/sub/schema.graphql']; expect(actual).toEqual(expected); From 4e9e28b83f9149167454e8693de18a4d70f79356 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Thu, 27 Aug 2026 13:20:49 +1000 Subject: [PATCH 07/11] Stop the readme promising cleaning the other bundler plugins do not do The readme showed a vite config as a working example. The vite entry point, like rollup, webpack, rspack, farm, rolldown, nuxt and astro, registers no hook that could clean, so that example never removed a file. The options block was also missing features and logger, both shipped in 1.2.0. --- packages/build-clean/CHANGELOG.md | 11 ++++++++ packages/build-clean/README.md | 25 +++++++++++-------- packages/build-clean/changes.jsonl | 3 +++ .../CHANGELOG.md | 6 +++++ .../changes.jsonl | 1 + 5 files changed, 35 insertions(+), 11 deletions(-) diff --git a/packages/build-clean/CHANGELOG.md b/packages/build-clean/CHANGELOG.md index 6f5f362..e49e80e 100644 --- a/packages/build-clean/CHANGELOG.md +++ b/packages/build-clean/CHANGELOG.md @@ -5,6 +5,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Changed + +- Corrected the documentation: cleaning runs on the esbuild path only, and the other bundler entry points register no cleanup hook + +### Fixed + +- Fixed the plugin deleting freshly built output on Windows and on case-insensitive filesystems +- Fixed the plugin cleaning the wrong directory when esbuild's working directory is not the process working directory + ## [1.3.6] - 2026-06-14 ### Changed diff --git a/packages/build-clean/README.md b/packages/build-clean/README.md index 76d79a4..1aeebb5 100644 --- a/packages/build-clean/README.md +++ b/packages/build-clean/README.md @@ -106,27 +106,30 @@ This plugin cleans after the build completes, removing only unused files while k interface Options { /** Show detailed debug information */ debug?: boolean - + /** Show verbose file-by-file processing */ verbose?: boolean - + /** Actually delete files (default: false for safety) */ destructive?: boolean + + /** Optional features. RemoveEmptyDirs is on by default */ + features?: Partial> + + /** Custom logger. When provided, debug and verbose are ignored */ + logger?: ILogger } ``` ## Other Build Tools -The plugin supports other tools via [unplugin](https://github.com/unjs/unplugin): +Cleaning runs from esbuild's build hook, so it works with esbuild directly and with +anything that builds through esbuild, such as tsup. -```ts -// vite.config.ts -import cleanPlugin from '@shellicar/build-clean/vite' - -export default defineConfig({ - plugins: [cleanPlugin({ destructive: true })] -}) -``` +The package also publishes `/vite`, `/rollup`, `/webpack`, `/rspack`, `/farm`, +`/rolldown`, `/nuxt` and `/astro` entry points through +[unplugin](https://github.com/unjs/unplugin). These register no cleanup hook, so they +currently do nothing. Use the esbuild plugin. ## Credits & Inspiration diff --git a/packages/build-clean/changes.jsonl b/packages/build-clean/changes.jsonl index e887b03..b216851 100644 --- a/packages/build-clean/changes.jsonl +++ b/packages/build-clean/changes.jsonl @@ -46,3 +46,6 @@ {"description":"Fixed GHSA-g7r4-m6w7-qqqr in esbuild","category":"security","metadata":{"ghsa":"GHSA-g7r4-m6w7-qqqr"}} {"description":"Updated esbuild peer dependency from ^0.27 to ^0.28","category":"changed"} {"type":"release","version":"1.3.6","date":"2026-06-14","tag":"build-clean@1.3.6"} +{"description":"Fixed the plugin deleting freshly built output on Windows and on case-insensitive filesystems","category":"fixed"} +{"description":"Fixed the plugin cleaning the wrong directory when esbuild's working directory is not the process working directory","category":"fixed"} +{"description":"Corrected the documentation: cleaning runs on the esbuild path only, and the other bundler entry points register no cleanup hook","category":"changed"} diff --git a/packages/svelte-adapter-azure-functions/CHANGELOG.md b/packages/svelte-adapter-azure-functions/CHANGELOG.md index f9fc5e7..6a5b09c 100644 --- a/packages/svelte-adapter-azure-functions/CHANGELOG.md +++ b/packages/svelte-adapter-azure-functions/CHANGELOG.md @@ -5,6 +5,12 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## Unreleased + +### Fixed + +- Fixed the generated function importing the server through an unresolvable path on Windows + ## [1.0.7] - 2026-06-14 ### Security diff --git a/packages/svelte-adapter-azure-functions/changes.jsonl b/packages/svelte-adapter-azure-functions/changes.jsonl index 484fb25..6df14c4 100644 --- a/packages/svelte-adapter-azure-functions/changes.jsonl +++ b/packages/svelte-adapter-azure-functions/changes.jsonl @@ -56,3 +56,4 @@ {"description":"Fixed GHSA-gv7w-rqvm-qjhr in esbuild","category":"security","metadata":{"ghsa":"GHSA-gv7w-rqvm-qjhr"}} {"description":"Fixed GHSA-g7r4-m6w7-qqqr in esbuild","category":"security","metadata":{"ghsa":"GHSA-g7r4-m6w7-qqqr"}} {"type":"release","version":"1.0.7","date":"2026-06-14","tag":"svelte-adapter-azure-functions@1.0.7"} +{"description":"Fixed the generated function importing the server through an unresolvable path on Windows","category":"fixed"} From d4ddaf31920488062cd53268201717434285e744 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Wed, 2 Sep 2026 10:03:57 +1000 Subject: [PATCH 08/11] Refuse to clean rather than guess when the output directory cannot be trusted Three cases where the plugin could not know what it was looking at and deleted anyway: none of the build's outputs present under the directory it was given, the directory unreadable rather than absent, and a directory outside the build altogether. Each now removes nothing and says why. A new strict option turns that refusal into a build failure, off by default so an upgrade cannot start breaking builds. The containment guard could not see a directory on another drive or a UNC share, because path.relative returns an absolute path when no relative route exists and the check only looked for a leading "..". The absolute-path guard beside it could never fire at all, since resolving an absolute path against any base returns it unchanged. One condition replaces both. Refusals name the output directory as it was configured again, which also restores what the source-directory check reads. A symlink pointing at a build output is removed, by identifying files with lstat rather than stat. A hard link to one is kept and now says so in the readme: both names are equally the file esbuild wrote, and telling them apart means returning to the path comparison that deleted builds on Windows. The test workspaces move out of the repository into the OS temp directory, where scratch output belongs. They had been written into the package's own test directory, behind a gitignore entry that kept them out of sight. --- .github/workflows/test-matrix.yml | 33 ------ .gitignore | 2 - packages/build-clean/CHANGELOG.md | 8 ++ packages/build-clean/README.md | 18 +++ packages/build-clean/changes.jsonl | 5 + .../build-clean/scripts/diagnose-paths.mjs | 103 ------------------ .../build-clean/src/core/cleanUnusedFiles.ts | 28 ++++- .../build-clean/src/core/defaultOptions.ts | 1 + packages/build-clean/src/core/fileIdentity.ts | 9 +- packages/build-clean/src/core/getAllFiles.ts | 11 +- .../build-clean/src/core/isOutsideBase.ts | 10 ++ packages/build-clean/src/core/types.ts | 2 +- .../build-clean/src/core/validateOutDir.ts | 33 +++--- packages/build-clean/src/types.ts | 7 ++ .../build-clean/test/abs-working-dir.spec.ts | 27 +++-- .../build-clean/test/isOutsideBase.spec.ts | 50 +++++++++ packages/build-clean/test/links.spec.ts | 45 ++++++++ packages/build-clean/test/refusals.spec.ts | 57 ++++++++++ .../build-clean/test/relative-outdir.spec.ts | 10 +- .../build-clean/test/support/workspace.ts | 30 ++++- .../build-clean/test/validateOutDir.spec.ts | 57 ++++++++++ 21 files changed, 357 insertions(+), 189 deletions(-) delete mode 100644 packages/build-clean/scripts/diagnose-paths.mjs create mode 100644 packages/build-clean/src/core/isOutsideBase.ts create mode 100644 packages/build-clean/test/isOutsideBase.spec.ts create mode 100644 packages/build-clean/test/links.spec.ts create mode 100644 packages/build-clean/test/refusals.spec.ts create mode 100644 packages/build-clean/test/validateOutDir.spec.ts diff --git a/.github/workflows/test-matrix.yml b/.github/workflows/test-matrix.yml index 288060a..4aace0a 100644 --- a/.github/workflows/test-matrix.yml +++ b/.github/workflows/test-matrix.yml @@ -8,10 +8,6 @@ name: Test matrix # checks.yml deliberately stays ubuntu-only. GitVersion, biome and the # changelog-drift check are platform-irrelevant, and the drift check's # `git diff --exit-code` would trip on line endings under Windows. -# -# The diagnostic steps run before the suite so their output is present whether -# the suite passes or fails. They can come out once the platform behaviour is -# settled. on: workflow_call: @@ -36,34 +32,5 @@ jobs: - uses: ./.github/actions/setup - run: pnpm i --frozen-lockfile - - name: Runner diagnostics - shell: bash - env: - RUNNER_OS_NAME: ${{ runner.os }} - MATRIX_OS: ${{ matrix.os }} - run: | - echo "runner.os : $RUNNER_OS_NAME" - echo "matrix.os : $MATRIX_OS" - echo "pnpm : $(pnpm --version)" - node -e 'const p = require("node:path"); console.log("node :", process.version); console.log("platform :", process.platform); console.log("arch :", process.arch); console.log("path.sep :", JSON.stringify(p.sep)); console.log("cwd :", process.cwd());' - - - name: Filesystem case-sensitivity probe - shell: bash - run: | - probe_dir="$(mktemp -d)" - printf 'lower\n' > "$probe_dir/casetest.txt" - if [ -f "$probe_dir/CASETEST.TXT" ]; then - echo "filesystem: case-INsensitive" - else - echo "filesystem: case-sensitive" - fi - - - name: esbuild metafile path probe - # Observes esbuild alone, with the clean plugin not loaded, so its output - # is evidence about the metafile rather than about our matching code. - shell: bash - working-directory: packages/build-clean - run: node scripts/diagnose-paths.mjs - - run: pnpm run --if-present build --only - run: pnpm run --if-present test --only diff --git a/.gitignore b/.gitignore index 5a9b23e..01f6b2b 100644 --- a/.gitignore +++ b/.gitignore @@ -9,5 +9,3 @@ coverage/ CLAUDE.local.md *.log *.bak -packages/build-clean/test/.tmp/ -packages/build-clean/test/.diagnostics/ diff --git a/packages/build-clean/CHANGELOG.md b/packages/build-clean/CHANGELOG.md index e49e80e..b0de183 100644 --- a/packages/build-clean/CHANGELOG.md +++ b/packages/build-clean/CHANGELOG.md @@ -7,6 +7,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## Unreleased +### Added + +- Added a strict option that turns a refusal to clean into a build failure + ### Changed - Corrected the documentation: cleaning runs on the esbuild path only, and the other bundler entry points register no cleanup hook @@ -15,6 +19,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Fixed the plugin deleting freshly built output on Windows and on case-insensitive filesystems - Fixed the plugin cleaning the wrong directory when esbuild's working directory is not the process working directory +- Nothing is deleted when none of the build's outputs can be found, or the output directory cannot be read, rather than deleting everything in it +- Refusing to clean a directory outside the build, including one on another drive or a network share on Windows +- A symlink pointing at a build output is now removed; a hard link to one is kept +- Refusal messages name the output directory as it was configured, not its resolved path ## [1.3.6] - 2026-06-14 diff --git a/packages/build-clean/README.md b/packages/build-clean/README.md index 1aeebb5..d469b51 100644 --- a/packages/build-clean/README.md +++ b/packages/build-clean/README.md @@ -113,6 +113,9 @@ interface Options { /** Actually delete files (default: false for safety) */ destructive?: boolean + /** Turn a refusal to clean into a build failure (default: false) */ + strict?: boolean + /** Optional features. RemoveEmptyDirs is on by default */ features?: Partial> @@ -121,6 +124,21 @@ interface Options { } ``` +## What gets removed + +Files in the output directory are matched against the build's own outputs by asking the +filesystem whether they are the same file, rather than by comparing paths. Anything the +build did not produce is removed. + +A symlink pointing at a build output is removed, because the build did not create it. A +hard link to a build output is kept: a hard link is not a reference to a file, it is the +file, so there is nothing to distinguish it from the name the build wrote. + +When none of the build's outputs can be found in the output directory, or the directory +cannot be read, nothing is removed and the reason is logged. That is deliberately not a +build failure, so an upgrade cannot start breaking builds. Set `strict: true` if you +want it to fail instead. + ## Other Build Tools Cleaning runs from esbuild's build hook, so it works with esbuild directly and with diff --git a/packages/build-clean/changes.jsonl b/packages/build-clean/changes.jsonl index b216851..d7814e6 100644 --- a/packages/build-clean/changes.jsonl +++ b/packages/build-clean/changes.jsonl @@ -49,3 +49,8 @@ {"description":"Fixed the plugin deleting freshly built output on Windows and on case-insensitive filesystems","category":"fixed"} {"description":"Fixed the plugin cleaning the wrong directory when esbuild's working directory is not the process working directory","category":"fixed"} {"description":"Corrected the documentation: cleaning runs on the esbuild path only, and the other bundler entry points register no cleanup hook","category":"changed"} +{"description":"Nothing is deleted when none of the build's outputs can be found, or the output directory cannot be read, rather than deleting everything in it","category":"fixed"} +{"description":"Refusing to clean a directory outside the build, including one on another drive or a network share on Windows","category":"fixed"} +{"description":"A symlink pointing at a build output is now removed; a hard link to one is kept","category":"fixed"} +{"description":"Refusal messages name the output directory as it was configured, not its resolved path","category":"fixed"} +{"description":"Added a strict option that turns a refusal to clean into a build failure","category":"added"} diff --git a/packages/build-clean/scripts/diagnose-paths.mjs b/packages/build-clean/scripts/diagnose-paths.mjs deleted file mode 100644 index fd6bdf6..0000000 --- a/packages/build-clean/scripts/diagnose-paths.mjs +++ /dev/null @@ -1,103 +0,0 @@ -// Observes esbuild only - the clean plugin is deliberately not loaded here, so -// the output is evidence about esbuild's metafile, not about our code. Run from -// the package directory. Always exits 0: it reports, it does not judge. -import { mkdir, readdir, stat, writeFile } from 'node:fs/promises'; -import { join, relative, resolve, sep } from 'node:path'; -import { build } from 'esbuild'; - -const root = join('test', '.diagnostics'); -const srcDir = join(root, 'src'); -const outDir = join(root, 'dist'); - -await mkdir(join(srcDir, 'nested'), { recursive: true }); -await writeFile(join(srcDir, 'main.ts'), "import { helper } from './nested/helper';\nconsole.log(helper());\n"); -await writeFile(join(srcDir, 'nested', 'helper.ts'), 'export const helper = () => 42;\n'); - -const result = await build({ - entryPoints: [join(srcDir, 'main.ts'), join(srcDir, 'nested', 'helper.ts')], - outdir: outDir, - bundle: true, - format: 'esm', - platform: 'node', - target: 'node22', - metafile: true, -}); - -const walk = async (dir) => { - const found = []; - for (const entry of await readdir(dir, { withFileTypes: true })) { - const full = join(dir, entry.name); - found.push(...(entry.isDirectory() ? await walk(full) : [full])); - } - return found; -}; - -const metafileKeys = Object.keys(result.metafile.outputs).sort(); -const walked = (await walk(outDir)).sort(); -const relatives = walked.map((file) => relative(process.cwd(), file)); - -console.log(`platform : ${process.platform}`); -console.log(`node : ${process.version}`); -console.log(`path.sep : ${JSON.stringify(sep)}`); -console.log(`process.cwd() : ${process.cwd()}`); -console.log(`outdir given : ${JSON.stringify(outDir)}`); -console.log(''); -console.log('esbuild metafile output keys (what the plugin matches against):'); -for (const key of metafileKeys) { - console.log(` ${JSON.stringify(key)}`); -} -console.log(''); -console.log('path.relative(cwd, walkedFile) (what the plugin computes per file):'); -for (const value of relatives) { - console.log(` ${JSON.stringify(value)}`); -} -console.log(''); - -const built = new Set(metafileKeys); -const missed = relatives.filter((value) => !built.has(value)); - -console.log(`string match : ${relatives.length - missed.length} of ${relatives.length}`); -if (missed.length > 0) { - console.log(''); - console.log('MISSED by string comparison - these are freshly-built files:'); - for (const value of missed) { - console.log(` ${JSON.stringify(value)}`); - } -} - -// Identity matching is what the plugin uses instead of string comparison. dev -// and ino are printed raw because ino is the part whose value on Windows cannot -// be assumed: a zero there would collapse every file onto one identity. -console.log(''); -console.log('stat identity, from the metafile key:'); -const identityOf = async (path) => { - try { - const stats = await stat(path); - return `${stats.dev}:${stats.ino}`; - } catch (error) { - return `unreadable (${error.code})`; - } -}; - -const builtIdentities = new Set(); -for (const key of metafileKeys) { - const identity = await identityOf(resolve(process.cwd(), key)); - builtIdentities.add(identity); - console.log(` ${JSON.stringify(key)} -> ${identity}`); -} - -console.log(''); -console.log('stat identity, from the walked file:'); -let identityMatches = 0; -for (const file of walked) { - const identity = await identityOf(file); - const hit = builtIdentities.has(identity); - if (hit) { - identityMatches++; - } - console.log(` ${JSON.stringify(file)} -> ${identity} ${hit ? 'MATCH' : 'NO MATCH'}`); -} - -console.log(''); -console.log(`identity match : ${identityMatches} of ${walked.length}`); -console.log(`distinct built identities : ${builtIdentities.size} (expected ${metafileKeys.length}; fewer means ino is not distinguishing files)`); diff --git a/packages/build-clean/src/core/cleanUnusedFiles.ts b/packages/build-clean/src/core/cleanUnusedFiles.ts index 00fbb54..f8a3a63 100644 --- a/packages/build-clean/src/core/cleanUnusedFiles.ts +++ b/packages/build-clean/src/core/cleanUnusedFiles.ts @@ -7,18 +7,33 @@ import { removeEmptyDirs } from './removeEmptyDirs'; import type { ResolvedOptions } from './types'; import { validateOutDir } from './validateOutDir'; +// Nothing is deleted when the plugin cannot trust what it is looking at. The +// refusal is loud but does not fail the build unless the caller asked for that, +// because a plugin that starts breaking builds on upgrade is its own incident. +const refuse = (reason: string, options: ResolvedOptions, cause?: unknown): void => { + const message = `[build-cleaner] Refusing to clean. ${reason}`; + options.logger.error(message, cause); + if (options.strict) { + throw new Error(message, { cause }); + } +}; + // baseDir is esbuild's working directory, which is what its metafile paths are // relative to. It is not always the process working directory. export async function cleanUnusedFiles(outDir: string, builtFiles: Set, baseDir: string, options: ResolvedOptions): Promise { const { logger } = options; - const resolvedOutDir = resolve(baseDir, outDir); - validateOutDir(resolvedOutDir, baseDir, logger); + const resolvedOutDir = validateOutDir(outDir, baseDir, logger); try { logger.debug(`Starting cleanup of directory: "${resolvedOutDir}"`); logger.debug(`Built files count: ${builtFiles.size}`); - const existingFiles = await getAllFiles(resolvedOutDir, logger); + let existingFiles: string[]; + try { + existingFiles = await getAllFiles(resolvedOutDir, logger); + } catch (error) { + return refuse(`Could not read the output directory: "${resolvedOutDir}"`, options, error); + } logger.debug(`Existing files count: ${existingFiles.length}`); if (existingFiles.length === 0 && builtFiles.size > 0) { @@ -37,6 +52,13 @@ export async function cleanUnusedFiles(outDir: string, builtFiles: Set, } logger.debug(`Resolved ${builtIdentities.size} of ${builtFiles.size} built files on disk`); + // Every output the build reported is missing from where it should be, so + // this directory is not the one that was built into. Deleting what does not + // match would take all of it. + if (builtIdentities.size === 0) { + return refuse(`No built file was found under "${resolvedOutDir}", of the ${builtFiles.size} the build reported`, options); + } + const filesToDelete: string[] = []; for (const file of existingFiles) { diff --git a/packages/build-clean/src/core/defaultOptions.ts b/packages/build-clean/src/core/defaultOptions.ts index 8b77d8c..975ece5 100644 --- a/packages/build-clean/src/core/defaultOptions.ts +++ b/packages/build-clean/src/core/defaultOptions.ts @@ -5,6 +5,7 @@ export const defaultOptions = { debug: false, verbose: false, destructive: false, + strict: false, features: { [Feature.RemoveEmptyDirs]: true, }, diff --git a/packages/build-clean/src/core/fileIdentity.ts b/packages/build-clean/src/core/fileIdentity.ts index dc73162..d6ff97f 100644 --- a/packages/build-clean/src/core/fileIdentity.ts +++ b/packages/build-clean/src/core/fileIdentity.ts @@ -1,11 +1,16 @@ -import { stat } from 'node:fs/promises'; +import { lstat } from 'node:fs/promises'; // Asks the filesystem whether two paths are the same file, rather than deciding // it from the strings. That is what makes the match correct on a case-folding // filesystem and independent of which separator each side happens to use. +// +// lstat, not stat: following a symlink would give it the identity of whatever it +// points at, so a link sitting in the output directory would be mistaken for the +// file it targets and kept. A hard link is a different matter and cannot be told +// apart this way, because it is not a reference to a file, it is the file. export const fileIdentity = async (path: string): Promise => { try { - const stats = await stat(path); + const stats = await lstat(path); return `${stats.dev}:${stats.ino}`; } catch { return undefined; diff --git a/packages/build-clean/src/core/getAllFiles.ts b/packages/build-clean/src/core/getAllFiles.ts index 71b07c2..253d684 100644 --- a/packages/build-clean/src/core/getAllFiles.ts +++ b/packages/build-clean/src/core/getAllFiles.ts @@ -2,6 +2,8 @@ import { readdir } from 'node:fs/promises'; import { join } from 'node:path'; import type { ILogger } from '../types'; +const isNotFound = (error: unknown): boolean => typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT'; + export async function getAllFiles(dir: string, logger: ILogger): Promise { const files: string[] = []; @@ -22,8 +24,13 @@ export async function getAllFiles(dir: string, logger: ILogger): Promise { + const relativePath = paths.relative(baseDir, candidate); + return relativePath === '..' || relativePath.startsWith(`..${paths.sep}`) || paths.isAbsolute(relativePath); +}; diff --git a/packages/build-clean/src/core/types.ts b/packages/build-clean/src/core/types.ts index 19d47dc..3b98141 100644 --- a/packages/build-clean/src/core/types.ts +++ b/packages/build-clean/src/core/types.ts @@ -3,7 +3,7 @@ import type { Options } from '../types'; type FullFeatures = Record; -type RequiredOptions = 'debug' | 'verbose' | 'destructive' | 'features' | 'logger'; +type RequiredOptions = 'debug' | 'verbose' | 'destructive' | 'strict' | 'features' | 'logger'; type MakeRequired = Omit & Required>; diff --git a/packages/build-clean/src/core/validateOutDir.ts b/packages/build-clean/src/core/validateOutDir.ts index c71c752..a29fbe9 100644 --- a/packages/build-clean/src/core/validateOutDir.ts +++ b/packages/build-clean/src/core/validateOutDir.ts @@ -1,18 +1,21 @@ import { relative, resolve } from 'node:path'; import type { ILogger } from '../types'; - -// baseDir is esbuild's working directory. The project to protect is the one the -// build is rooted at, which is not always the process working directory. -export const validateOutDir = (outDir: string, baseDir: string, logger: ILogger) => { - const resolvedOutDir = resolve(outDir); +import { isOutsideBase } from './isOutsideBase'; + +// outDir is the value the caller configured, kept in that form so the refusals +// name what they would go and change. baseDir is esbuild's working directory: +// the project to protect is the one the build is rooted at, which is not always +// the process working directory. Returns the resolved directory so one place +// decides what was validated and what gets cleaned. +export const validateOutDir = (outDir: string, baseDir: string, logger: ILogger): string => { + const resolvedOutDir = resolve(baseDir, outDir); const relativePath = relative(baseDir, resolvedOutDir); const normalizedPath = outDir.replace(/\\/g, '/'); - const isAbsolutePath = resolve(outDir) !== resolve(baseDir, outDir); const isSameAsCurrentDir = resolvedOutDir === baseDir; const isParentOfCurrentDirUnix = baseDir.startsWith(`${resolvedOutDir}/`); const isParentOfCurrentDirWindows = baseDir.startsWith(`${resolvedOutDir}\\`); const isParentOfCurrentDir = isParentOfCurrentDirUnix || isParentOfCurrentDirWindows; - const goesUpDirectory = relativePath.startsWith('..'); + const isOutside = isOutsideBase(baseDir, resolvedOutDir); logger.verbose('Path validation:'); logger.verbose(` Input: "${outDir}"`); @@ -20,12 +23,11 @@ export const validateOutDir = (outDir: string, baseDir: string, logger: ILogger) logger.verbose(` Resolved output directory: "${resolvedOutDir}"`); logger.verbose(` Relative path from base directory: "${relativePath}"`); logger.verbose(` Normalized path: "${normalizedPath}"`); - logger.verbose(` Is absolute path outside project: ${isAbsolutePath}`); - logger.verbose(` Is same as current directory: ${isSameAsCurrentDir}`); + logger.verbose(` Is same as base directory: ${isSameAsCurrentDir}`); logger.verbose(` Is parent of base directory (Unix): ${isParentOfCurrentDirUnix}`); logger.verbose(` Is parent of base directory (Windows): ${isParentOfCurrentDirWindows}`); logger.verbose(` Is parent of base directory: ${isParentOfCurrentDir}`); - logger.verbose(` Goes up directory levels: ${goesUpDirectory}`); + logger.verbose(` Is outside the base directory: ${isOutside}`); // Check if the resolved path is the same as the base directory if (isSameAsCurrentDir) { @@ -37,16 +39,11 @@ export const validateOutDir = (outDir: string, baseDir: string, logger: ILogger) throw new Error(`[build-cleaner] Refusing to clean parent directory: "${outDir}". This would delete the current project.`); } - // Check if the relative path goes up (.., ../.., etc.) - if (goesUpDirectory) { + // Outside the project, whether by climbing out or by having no route at all + if (isOutside) { throw new Error(`[build-cleaner] Refusing to clean directory outside project: "${outDir}". Use a subdirectory like "dist" or "build".`); } - // Check if it's an absolute path outside the project - if (isAbsolutePath) { - throw new Error(`[build-cleaner] Refusing to clean absolute path outside project: "${outDir}". Use a relative subdirectory.`); - } - // Prevent cleaning common source directories (even as subdirectories) const dangerousPaths = ['src', 'source', 'lib', 'app', 'components', 'pages', 'routes']; const isDangerousPath = dangerousPaths.some((dangerous) => normalizedPath === dangerous || normalizedPath.endsWith(`/${dangerous}`)); @@ -56,4 +53,6 @@ export const validateOutDir = (outDir: string, baseDir: string, logger: ILogger) } logger.debug(`Validated output directory: "${outDir}" -> "${resolvedOutDir}"`); + + return resolvedOutDir; }; diff --git a/packages/build-clean/src/types.ts b/packages/build-clean/src/types.ts index fc264b1..54e17c4 100644 --- a/packages/build-clean/src/types.ts +++ b/packages/build-clean/src/types.ts @@ -29,6 +29,13 @@ export interface Options { */ destructive?: boolean; + /** + * Turn a refusal to clean into a build failure. When false the plugin logs the + * refusal and leaves the output directory alone. + * @default false + */ + strict?: boolean; + /** * Feature flags for optional functionality */ diff --git a/packages/build-clean/test/abs-working-dir.spec.ts b/packages/build-clean/test/abs-working-dir.spec.ts index 1013852..3e79cf9 100644 --- a/packages/build-clean/test/abs-working-dir.spec.ts +++ b/packages/build-clean/test/abs-working-dir.spec.ts @@ -1,25 +1,28 @@ -import { resolve } from 'node:path'; import { build } from 'esbuild'; import { describe, expect, it, onTestFailed } from 'vitest'; import cleanPlugin from '../src/esbuild'; import { buildOptions, createCapturingLogger, createWorkspace, listOutput } from './support/workspace'; -// esbuild keys the metafile relative to absWorkingDir; the plugin computes its -// side relative to process.cwd(). Everything here is absolute and inside the -// temp workspace, so the deletion this provokes cannot reach a real directory. +// esbuild keys the metafile relative to absWorkingDir; the plugin used to +// compute its side relative to process.cwd(). Every workspace here is rooted +// outside the repository, so the two are never the same and this path is +// exercised by every test in the suite. describe('absWorkingDir', () => { - it('keeps the files esbuild just built when absWorkingDir is not the process cwd', async () => { + it('is not the process working directory, or the rest of this proves nothing', async () => { + const workspace = await createWorkspace('abs-working-dir-differs'); + + const expected = false; + const actual = workspace.root === process.cwd(); + + expect(actual).toBe(expected); + }); + + it('keeps the files esbuild just built', async () => { const workspace = await createWorkspace('abs-working-dir'); const logger = createCapturingLogger(); onTestFailed(() => console.error(logger.lines.join('\n'))); - await build({ - ...buildOptions(workspace), - absWorkingDir: resolve(workspace.root), - entryPoints: [resolve(workspace.srcDir, 'main.ts'), resolve(workspace.srcDir, 'nested', 'helper.ts')], - outdir: resolve(workspace.outDir), - plugins: [cleanPlugin({ destructive: true, logger })], - }); + await build({ ...buildOptions(workspace), plugins: [cleanPlugin({ destructive: true, logger })] }); const expected = ['main.js', 'nested/helper.js']; const actual = await listOutput(workspace.outDir); diff --git a/packages/build-clean/test/isOutsideBase.spec.ts b/packages/build-clean/test/isOutsideBase.spec.ts new file mode 100644 index 0000000..6444372 --- /dev/null +++ b/packages/build-clean/test/isOutsideBase.spec.ts @@ -0,0 +1,50 @@ +import { posix, win32 } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { isOutsideBase } from '../src/core/isOutsideBase'; + +describe('isOutsideBase', () => { + it('treats a directory below the base as inside', () => { + const expected = false; + const actual = isOutsideBase('/proj', '/proj/dist', posix); + + expect(actual).toBe(expected); + }); + + it('treats a directory whose name merely starts with dots as inside', () => { + const expected = false; + const actual = isOutsideBase('/proj', '/proj/..hidden', posix); + + expect(actual).toBe(expected); + }); + + it('treats a sibling of the base as outside', () => { + const expected = true; + const actual = isOutsideBase('/proj', '/elsewhere', posix); + + expect(actual).toBe(expected); + }); + + // The case no existing guard caught. On Windows there is no relative route + // between drives, so relative returns the target unchanged and a check for a + // leading ".." reads false. + it('treats another drive as outside', () => { + const expected = true; + const actual = isOutsideBase('C:\\proj', 'D:\\stuff', win32); + + expect(actual).toBe(expected); + }); + + it('treats a UNC share as outside', () => { + const expected = true; + const actual = isOutsideBase('C:\\proj', '\\\\server\\share\\stuff', win32); + + expect(actual).toBe(expected); + }); + + it('treats a directory below the base on the same drive as inside', () => { + const expected = false; + const actual = isOutsideBase('C:\\proj', 'C:\\proj\\dist', win32); + + expect(actual).toBe(expected); + }); +}); diff --git a/packages/build-clean/test/links.spec.ts b/packages/build-clean/test/links.spec.ts new file mode 100644 index 0000000..b7e0aa0 --- /dev/null +++ b/packages/build-clean/test/links.spec.ts @@ -0,0 +1,45 @@ +import { link, symlink } from 'node:fs/promises'; +import { join } from 'node:path'; +import { build } from 'esbuild'; +import { describe, expect, it, onTestFailed } from 'vitest'; +import cleanPlugin from '../src/esbuild'; +import { buildOptions, createCapturingLogger, createWorkspace, listOutput } from './support/workspace'; + +// Matching by filesystem identity has to answer for links, and the two kinds +// have different answers. +describe('links in the output directory', () => { + it('removes a symlink pointing at a built file', async () => { + const workspace = await createWorkspace('symlink'); + const logger = createCapturingLogger(); + onTestFailed(() => console.error(logger.lines.join('\n'))); + + await build(buildOptions(workspace)); + await symlink('main.js', join(workspace.outDir, 'stale-link.js')); + await build({ ...buildOptions(workspace), plugins: [cleanPlugin({ destructive: true, logger })] }); + + const expected = false; + const actual = (await listOutput(workspace.outDir)).includes('stale-link.js'); + + expect(actual).toBe(expected); + }); + + // A hard link is not a reference to a file, it is the file. Both names are + // equally the thing esbuild wrote, and there is no fact about which one is + // canonical, so identity matching keeps it. Decided behaviour, not an + // oversight: the alternative is going back to comparing path strings, which is + // what deleted people's builds on Windows. + it('keeps a hard link to a built file', async () => { + const workspace = await createWorkspace('hard-link'); + const logger = createCapturingLogger(); + onTestFailed(() => console.error(logger.lines.join('\n'))); + + await build(buildOptions(workspace)); + await link(join(workspace.outDir, 'main.js'), join(workspace.outDir, 'stale-link.js')); + await build({ ...buildOptions(workspace), plugins: [cleanPlugin({ destructive: true, logger })] }); + + const expected = true; + const actual = (await listOutput(workspace.outDir)).includes('stale-link.js'); + + expect(actual).toBe(expected); + }); +}); diff --git a/packages/build-clean/test/refusals.spec.ts b/packages/build-clean/test/refusals.spec.ts new file mode 100644 index 0000000..0685551 --- /dev/null +++ b/packages/build-clean/test/refusals.spec.ts @@ -0,0 +1,57 @@ +import { writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { cleanUnusedFiles } from '../src/core/cleanUnusedFiles'; +import { resolveOptions } from '../src/core/resolveOptions'; +import { createCapturingLogger, createWorkspace, listOutput } from './support/workspace'; + +// When the plugin cannot trust what it is looking at, it deletes nothing. This +// is the difference between the Windows incident being a loud error and being a +// silently deleted build. +describe('refusing to clean', () => { + it('deletes nothing when no built file is found in the output directory', async () => { + const workspace = await createWorkspace('refuse-nothing-resolved'); + await writeFile(join(workspace.outDir, 'stale.js'), '// not a build output\n'); + const options = resolveOptions({ destructive: true, logger: createCapturingLogger() }); + + await cleanUnusedFiles('dist', new Set(['dist/never-built.js']), workspace.root, options); + + const expected = ['stale.js']; + const actual = await listOutput(workspace.outDir); + + expect(actual).toEqual(expected); + }); + + it('fails the build when no built file is found and strict is on', async () => { + const workspace = await createWorkspace('refuse-nothing-resolved-strict'); + await writeFile(join(workspace.outDir, 'stale.js'), '// not a build output\n'); + const options = resolveOptions({ destructive: true, strict: true, logger: createCapturingLogger() }); + + await expect(cleanUnusedFiles('dist', new Set(['dist/never-built.js']), workspace.root, options)).rejects.toThrow('Refusing to clean'); + }); + + // A directory that is not there yet is the ordinary first build. A directory + // that cannot be read is not the same thing, and must not be mistaken for an + // empty one. A plain file standing where a directory should be produces that + // second case on every platform. + it('refuses without failing the build when the output directory cannot be read', async () => { + const workspace = await createWorkspace('refuse-unreadable'); + await writeFile(join(workspace.root, 'notadirectory'), 'this is a file\n'); + const logger = createCapturingLogger(); + + await cleanUnusedFiles('notadirectory', new Set(['dist/main.js']), workspace.root, resolveOptions({ destructive: true, logger })); + + const expected = true; + const actual = logger.lines.some((line) => line.includes('Refusing to clean. Could not read the output directory')); + + expect(actual).toBe(expected); + }); + + it('fails the build when the output directory cannot be read and strict is on', async () => { + const workspace = await createWorkspace('refuse-unreadable-strict'); + await writeFile(join(workspace.root, 'notadirectory'), 'this is a file\n'); + const options = resolveOptions({ destructive: true, strict: true, logger: createCapturingLogger() }); + + await expect(cleanUnusedFiles('notadirectory', new Set(['dist/main.js']), workspace.root, options)).rejects.toThrow('Refusing to clean'); + }); +}); diff --git a/packages/build-clean/test/relative-outdir.spec.ts b/packages/build-clean/test/relative-outdir.spec.ts index 9386b62..17ae43a 100644 --- a/packages/build-clean/test/relative-outdir.spec.ts +++ b/packages/build-clean/test/relative-outdir.spec.ts @@ -1,5 +1,5 @@ import { writeFile } from 'node:fs/promises'; -import { join, resolve } from 'node:path'; +import { join } from 'node:path'; import { build } from 'esbuild'; import { describe, expect, it, onTestFailed } from 'vitest'; import cleanPlugin from '../src/esbuild'; @@ -23,13 +23,7 @@ describe('relative outdir under a different absWorkingDir', () => { onTestFailed(() => console.error(logger.lines.join('\n'))); await writeFile(join(workspace.outDir, 'stale.js'), '// left over from an earlier build\n'); - await build({ - ...buildOptions(workspace), - absWorkingDir: resolve(workspace.root), - entryPoints: [resolve(workspace.srcDir, 'main.ts'), resolve(workspace.srcDir, 'nested', 'helper.ts')], - outdir: 'dist', - plugins: [cleanPlugin({ destructive: false, logger })], - }); + await build({ ...buildOptions(workspace), plugins: [cleanPlugin({ destructive: false, logger })] }); const expected = ['dist/stale.js']; const actual = wouldDelete(logger.lines); diff --git a/packages/build-clean/test/support/workspace.ts b/packages/build-clean/test/support/workspace.ts index a5a883a..1a055cf 100644 --- a/packages/build-clean/test/support/workspace.ts +++ b/packages/build-clean/test/support/workspace.ts @@ -1,6 +1,7 @@ import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; -import { mkdir, readdir, writeFile } from 'node:fs/promises'; -import { join, relative, sep } from 'node:path'; +import { mkdir, readdir, rm, writeFile } from 'node:fs/promises'; +import { tmpdir } from 'node:os'; +import { basename, join, relative, sep } from 'node:path'; import type { BuildOptions } from 'esbuild'; import type { ILogger } from '../../src/types'; @@ -10,13 +11,27 @@ export type Workspace = { outDir: string; }; +// Test workspaces live in the OS temp directory, never in the repository. These +// builds write real files and the plugin deletes real files, and none of that +// belongs anywhere near source. +const workspacesRoot = join(tmpdir(), 'shellicar-build-clean-tests'); + // Two entry points in different directories, so esbuild emits a nested output // path. A flat output would match on Windows by accident and prove nothing. +// +// The workspace is removed before it is rebuilt. Writing over it is not enough: +// writeFile overwrites but link and symlink do not, so a test creating either +// passes once and then fails with EEXIST on every later run. export const createWorkspace = async (name: string): Promise => { - const root = join('test', '.tmp', name); + if (name !== basename(name) || name === '.' || name === '..') { + throw new Error(`Workspace name must be a single path segment, got "${name}"`); + } + + const root = join(workspacesRoot, name); const srcDir = join(root, 'src'); const outDir = join(root, 'dist'); + await rm(root, { recursive: true, force: true }); await mkdir(join(srcDir, 'nested'), { recursive: true }); await mkdir(outDir, { recursive: true }); await writeFile(join(srcDir, 'main.ts'), "import { helper } from './nested/helper';\nconsole.log(helper());\n"); @@ -25,10 +40,13 @@ export const createWorkspace = async (name: string): Promise => { return { root, srcDir, outDir }; }; +// absWorkingDir is the workspace, which is what a real project's build sees: the +// output directory sits under the directory the build is rooted at. export const buildOptions = (workspace: Workspace): BuildOptions => ({ - entryPoints: [join(workspace.srcDir, 'main.ts'), join(workspace.srcDir, 'nested', 'helper.ts')], - outdir: workspace.outDir, + absWorkingDir: workspace.root, + entryPoints: ['src/main.ts', 'src/nested/helper.ts'], + outdir: 'dist', bundle: true, format: 'esm', platform: 'node', @@ -63,7 +81,7 @@ export const listOutput = async (outDir: string): Promise => { // no, macOS and Windows say yes, and that is the whole difference the // case-mismatch behaviour turns on. export const filesystemIsCaseInsensitive = (): boolean => { - const probeDir = join('test', '.tmp', 'case-probe'); + const probeDir = join(workspacesRoot, '.case-probe'); mkdirSync(probeDir, { recursive: true }); writeFileSync(join(probeDir, 'probe.txt'), 'probe\n'); return existsSync(join(probeDir, 'PROBE.TXT')); diff --git a/packages/build-clean/test/validateOutDir.spec.ts b/packages/build-clean/test/validateOutDir.spec.ts new file mode 100644 index 0000000..a2885c8 --- /dev/null +++ b/packages/build-clean/test/validateOutDir.spec.ts @@ -0,0 +1,57 @@ +import { resolve } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { validateOutDir } from '../src/core/validateOutDir'; +import { createCapturingLogger } from './support/workspace'; + +// validateOutDir is the only thing standing between the plugin and a directory +// it was never asked to clean, and it does no IO, so the refusals test directly. +// The base is built with resolve so it is native on whichever platform runs. +const base = resolve('proj'); +const validate = (outDir: string) => () => validateOutDir(outDir, base, createCapturingLogger()); + +describe('validateOutDir', () => { + it('accepts a directory below the base directory', () => { + expect(validate('dist')).not.toThrow(); + }); + + it('accepts a nested directory below the base directory', () => { + expect(validate('build/output')).not.toThrow(); + }); + + it('accepts an absolute directory below the base directory', () => { + expect(validate(resolve(base, 'dist'))).not.toThrow(); + }); + + it('returns the directory resolved against the base', () => { + const expected = resolve(base, 'dist'); + const actual = validateOutDir('dist', base, createCapturingLogger()); + + expect(actual).toBe(expected); + }); + + it('refuses the base directory itself', () => { + expect(validate('.')).toThrow('Refusing to clean current directory'); + }); + + it('refuses a parent of the base directory', () => { + expect(validate('..')).toThrow('Refusing to clean parent directory'); + }); + + it('refuses a sibling of the base directory', () => { + expect(validate('../elsewhere')).toThrow('Refusing to clean directory outside project'); + }); + + it('refuses a source directory', () => { + expect(validate('src')).toThrow('Refusing to clean source directory'); + }); + + it('refuses a nested source directory', () => { + expect(validate('packages/thing/lib')).toThrow('Refusing to clean source directory'); + }); + + // The refusal has to name what the caller wrote in their config, not the path + // it was resolved to, or it points at nothing they can go and edit. + it('names the configured value in the refusal, not the resolved path', () => { + expect(validate('src')).toThrow('"src"'); + }); +}); From 396e7e98bfbbebd761dc6cf22d40526b91513093 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Wed, 2 Sep 2026 10:37:11 +1000 Subject: [PATCH 09/11] Stop the plugin adding its own error to a build that already failed A failed build writes no metafile, and the plugin read that as the metafile option being off, telling the user to enable something it turns on itself. Every compile error came with a second error saying so. It now returns when the build reports errors, and keeps the diagnostic for the case it actually describes: a build that succeeded with the option removed underneath it. Two faults in the refusal path, both reached only by the branch added in the previous commit and neither covered by a test. An absent cause was still passed to the logger, which spreads its arguments, so the refusal ended in the word undefined. And a refusal under strict was caught by the surrounding handler and logged a second time wrapped in its own message. A build reporting no outputs still refuses rather than emptying the directory, which is the case this guard exists for, but it no longer says "of the 0 the build reported". The change entries are rewritten as changelog lines rather than prose, and gain one that was missing: a hard link to a build output used to be removed and now is not. --- packages/build-clean/CHANGELOG.md | 16 +++++++----- packages/build-clean/changes.jsonl | 16 +++++++----- .../build-clean/src/core/cleanUnusedFiles.ts | 22 +++++++++++++--- .../build-clean/src/core/pluginFactory.ts | 10 +++++++ .../build-clean/test/failed-build.spec.ts | 24 +++++++++++++++++ packages/build-clean/test/refusals.spec.ts | 26 +++++++++++++++++++ .../CHANGELOG.md | 2 +- .../changes.jsonl | 2 +- 8 files changed, 99 insertions(+), 19 deletions(-) create mode 100644 packages/build-clean/test/failed-build.spec.ts diff --git a/packages/build-clean/CHANGELOG.md b/packages/build-clean/CHANGELOG.md index b0de183..b8ef9b0 100644 --- a/packages/build-clean/CHANGELOG.md +++ b/packages/build-clean/CHANGELOG.md @@ -13,16 +13,18 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Changed -- Corrected the documentation: cleaning runs on the esbuild path only, and the other bundler entry points register no cleanup hook +- Hard links to build outputs are no longer removed +- Corrected the readme: cleaning runs on the esbuild path only ### Fixed -- Fixed the plugin deleting freshly built output on Windows and on case-insensitive filesystems -- Fixed the plugin cleaning the wrong directory when esbuild's working directory is not the process working directory -- Nothing is deleted when none of the build's outputs can be found, or the output directory cannot be read, rather than deleting everything in it -- Refusing to clean a directory outside the build, including one on another drive or a network share on Windows -- A symlink pointing at a build output is now removed; a hard link to one is kept -- Refusal messages name the output directory as it was configured, not its resolved path +- Fixed built output being deleted on Windows and on case-insensitive filesystems +- Fixed the output directory being resolved against the process working directory rather than esbuild's +- Fixed every file being deleted when no build output is found in the output directory +- Fixed an unreadable output directory being treated as an empty one +- Fixed a directory outside the build being cleaned, including one on another drive or network share +- Fixed a symlink to a build output not being removed +- Fixed refusal messages naming the resolved path rather than the configured value ## [1.3.6] - 2026-06-14 diff --git a/packages/build-clean/changes.jsonl b/packages/build-clean/changes.jsonl index d7814e6..ac87120 100644 --- a/packages/build-clean/changes.jsonl +++ b/packages/build-clean/changes.jsonl @@ -46,11 +46,13 @@ {"description":"Fixed GHSA-g7r4-m6w7-qqqr in esbuild","category":"security","metadata":{"ghsa":"GHSA-g7r4-m6w7-qqqr"}} {"description":"Updated esbuild peer dependency from ^0.27 to ^0.28","category":"changed"} {"type":"release","version":"1.3.6","date":"2026-06-14","tag":"build-clean@1.3.6"} -{"description":"Fixed the plugin deleting freshly built output on Windows and on case-insensitive filesystems","category":"fixed"} -{"description":"Fixed the plugin cleaning the wrong directory when esbuild's working directory is not the process working directory","category":"fixed"} -{"description":"Corrected the documentation: cleaning runs on the esbuild path only, and the other bundler entry points register no cleanup hook","category":"changed"} -{"description":"Nothing is deleted when none of the build's outputs can be found, or the output directory cannot be read, rather than deleting everything in it","category":"fixed"} -{"description":"Refusing to clean a directory outside the build, including one on another drive or a network share on Windows","category":"fixed"} -{"description":"A symlink pointing at a build output is now removed; a hard link to one is kept","category":"fixed"} -{"description":"Refusal messages name the output directory as it was configured, not its resolved path","category":"fixed"} +{"description":"Fixed built output being deleted on Windows and on case-insensitive filesystems","category":"fixed"} +{"description":"Fixed the output directory being resolved against the process working directory rather than esbuild's","category":"fixed"} +{"description":"Fixed every file being deleted when no build output is found in the output directory","category":"fixed"} +{"description":"Fixed an unreadable output directory being treated as an empty one","category":"fixed"} +{"description":"Fixed a directory outside the build being cleaned, including one on another drive or network share","category":"fixed"} +{"description":"Fixed a symlink to a build output not being removed","category":"fixed"} +{"description":"Fixed refusal messages naming the resolved path rather than the configured value","category":"fixed"} {"description":"Added a strict option that turns a refusal to clean into a build failure","category":"added"} +{"description":"Hard links to build outputs are no longer removed","category":"changed"} +{"description":"Corrected the readme: cleaning runs on the esbuild path only","category":"changed"} diff --git a/packages/build-clean/src/core/cleanUnusedFiles.ts b/packages/build-clean/src/core/cleanUnusedFiles.ts index f8a3a63..192e994 100644 --- a/packages/build-clean/src/core/cleanUnusedFiles.ts +++ b/packages/build-clean/src/core/cleanUnusedFiles.ts @@ -10,11 +10,23 @@ import { validateOutDir } from './validateOutDir'; // Nothing is deleted when the plugin cannot trust what it is looking at. The // refusal is loud but does not fail the build unless the caller asked for that, // because a plugin that starts breaking builds on upgrade is its own incident. +// Thrown only by refuse, so the catch below can let it through without logging +// a refusal that has already been reported. +class RefusalError extends Error {} + const refuse = (reason: string, options: ResolvedOptions, cause?: unknown): void => { const message = `[build-cleaner] Refusing to clean. ${reason}`; - options.logger.error(message, cause); + + // Passing an absent cause still counts as an argument, and the logger spreads + // its arguments, so the word undefined would reach the user. + if (cause === undefined) { + options.logger.error(message); + } else { + options.logger.error(message, cause); + } + if (options.strict) { - throw new Error(message, { cause }); + throw new RefusalError(message, { cause }); } }; @@ -56,7 +68,8 @@ export async function cleanUnusedFiles(outDir: string, builtFiles: Set, // this directory is not the one that was built into. Deleting what does not // match would take all of it. if (builtIdentities.size === 0) { - return refuse(`No built file was found under "${resolvedOutDir}", of the ${builtFiles.size} the build reported`, options); + const reason = builtFiles.size === 0 ? 'The build reported no output files' : `None of the ${builtFiles.size} files the build reported were found under "${resolvedOutDir}"`; + return refuse(reason, options); } const filesToDelete: string[] = []; @@ -103,6 +116,9 @@ export async function cleanUnusedFiles(outDir: string, builtFiles: Set, await removeEmptyDirs(resolvedOutDir, options); } } catch (error) { + if (error instanceof RefusalError) { + throw error; + } logger.error('Error during cleanup:', error); throw error; } diff --git a/packages/build-clean/src/core/pluginFactory.ts b/packages/build-clean/src/core/pluginFactory.ts index 34cbc14..9cf8590 100644 --- a/packages/build-clean/src/core/pluginFactory.ts +++ b/packages/build-clean/src/core/pluginFactory.ts @@ -17,11 +17,21 @@ export const pluginFactory: UnpluginFactory = (initialOptio build.onEnd(async (result) => { logger.debug('Build completed, starting cleanup process'); + // A build that failed wrote nothing, so there is nothing to clean and + // nothing useful to say. Its own errors are what the user needs to + // read, and an error from here would sit on top of them. + if (result.errors.length > 0) { + logger.debug(`Build failed with ${result.errors.length} error(s), skipping cleanup`); + return; + } + const outdir = build.initialOptions.outdir; if (!outdir) { throw new Error('[build-cleaner] No output directory specified in build options'); } + // The build succeeded, so this means something removed the metafile + // option that setup turned on. if (!result.metafile) { throw new Error('[build-cleaner] No metafile available - ensure metafile is enabled'); } diff --git a/packages/build-clean/test/failed-build.spec.ts b/packages/build-clean/test/failed-build.spec.ts new file mode 100644 index 0000000..9828661 --- /dev/null +++ b/packages/build-clean/test/failed-build.spec.ts @@ -0,0 +1,24 @@ +import { writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { build } from 'esbuild'; +import { describe, expect, it } from 'vitest'; +import cleanPlugin from '../src/esbuild'; +import { buildOptions, createCapturingLogger, createWorkspace } from './support/workspace'; + +// A build that fails produces no metafile, because there are no outputs to +// describe. That is the build's own failure and the plugin has nothing to say +// about it: adding an error of its own buries the one the user needs to read. +describe('a build that fails to compile', () => { + it('reports only the errors the build itself produced', async () => { + const workspace = await createWorkspace('failed-build'); + const logger = createCapturingLogger(); + await writeFile(join(workspace.srcDir, 'main.ts'), 'this is not valid typescript !!!\n'); + + const failure = await build({ ...buildOptions(workspace), plugins: [cleanPlugin({ destructive: true, logger })] }).catch((error: unknown) => error); + + const expected: string[] = []; + const actual = (failure as { errors: { text: string }[] }).errors.map((error) => error.text).filter((text) => text.includes('build-cleaner')); + + expect(actual).toEqual(expected); + }); +}); diff --git a/packages/build-clean/test/refusals.spec.ts b/packages/build-clean/test/refusals.spec.ts index 0685551..2f799ed 100644 --- a/packages/build-clean/test/refusals.spec.ts +++ b/packages/build-clean/test/refusals.spec.ts @@ -30,6 +30,32 @@ describe('refusing to clean', () => { await expect(cleanUnusedFiles('dist', new Set(['dist/never-built.js']), workspace.root, options)).rejects.toThrow('Refusing to clean'); }); + it('does not print a trailing undefined when the refusal has no cause', async () => { + const workspace = await createWorkspace('refuse-no-cause-message'); + await writeFile(join(workspace.outDir, 'stale.js'), '// not a build output\n'); + const logger = createCapturingLogger(); + + await cleanUnusedFiles('dist', new Set(['dist/never-built.js']), workspace.root, resolveOptions({ destructive: true, logger })); + + const expected = false; + const actual = logger.lines.some((line) => line.endsWith('undefined')); + + expect(actual).toBe(expected); + }); + + it('logs the refusal once when strict turns it into a failure', async () => { + const workspace = await createWorkspace('refuse-strict-single-log'); + await writeFile(join(workspace.outDir, 'stale.js'), '// not a build output\n'); + const logger = createCapturingLogger(); + + await cleanUnusedFiles('dist', new Set(['dist/never-built.js']), workspace.root, resolveOptions({ destructive: true, strict: true, logger })).catch(() => {}); + + const expected = 1; + const actual = logger.lines.filter((line) => line.startsWith('[error]')).length; + + expect(actual).toBe(expected); + }); + // A directory that is not there yet is the ordinary first build. A directory // that cannot be read is not the same thing, and must not be mistaken for an // empty one. A plain file standing where a directory should be produces that diff --git a/packages/svelte-adapter-azure-functions/CHANGELOG.md b/packages/svelte-adapter-azure-functions/CHANGELOG.md index 6a5b09c..3df88df 100644 --- a/packages/svelte-adapter-azure-functions/CHANGELOG.md +++ b/packages/svelte-adapter-azure-functions/CHANGELOG.md @@ -9,7 +9,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed -- Fixed the generated function importing the server through an unresolvable path on Windows +- Fixed an unresolvable server import in the generated function on Windows ## [1.0.7] - 2026-06-14 diff --git a/packages/svelte-adapter-azure-functions/changes.jsonl b/packages/svelte-adapter-azure-functions/changes.jsonl index 6df14c4..e45b87e 100644 --- a/packages/svelte-adapter-azure-functions/changes.jsonl +++ b/packages/svelte-adapter-azure-functions/changes.jsonl @@ -56,4 +56,4 @@ {"description":"Fixed GHSA-gv7w-rqvm-qjhr in esbuild","category":"security","metadata":{"ghsa":"GHSA-gv7w-rqvm-qjhr"}} {"description":"Fixed GHSA-g7r4-m6w7-qqqr in esbuild","category":"security","metadata":{"ghsa":"GHSA-g7r4-m6w7-qqqr"}} {"type":"release","version":"1.0.7","date":"2026-06-14","tag":"svelte-adapter-azure-functions@1.0.7"} -{"description":"Fixed the generated function importing the server through an unresolvable path on Windows","category":"fixed"} +{"description":"Fixed an unresolvable server import in the generated function on Windows","category":"fixed"} From 897dbdf50386bcd8cb0af609ee795c9653f810aa Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Wed, 2 Sep 2026 11:29:05 +1000 Subject: [PATCH 10/11] Clean the output directory when the build produced nothing Refusing there was new in this branch and wrong. The guard reads zero matches as the wrong directory, and that only carries information when there was something to match: a build that produced nothing matches nothing wherever it is pointed. Everything present came from an earlier build, which is exactly what this plugin removes, and leaving it means a stale output can be deployed as current. The rest is the coverage that would have caught it, and three other branches that had none. The default logger had no test at all, which is how a refusal came to print the word undefined to users. Neither the missing output directory of a first build, nor the metafile and outdir the hook refuses to run without, nor a file that cannot be deleted, nor the working-directory fallback taken by every build that does not set absWorkingDir. removeEmptyDirs guards reading a directory and reports the failure, then reads it again unguarded, so the failure surfaces as a throw anyway. Pinned as it stands rather than changed. Branch coverage of the core is 96%. What is left is the handler that logs and rethrows an unexpected error, which needs an injected fault to reach and alters nothing on the way through. --- .../build-clean/src/core/cleanUnusedFiles.ts | 10 ++- .../build-clean/test/delete-failure.spec.ts | 24 +++++++ packages/build-clean/test/first-build.spec.ts | 35 ++++++++++ packages/build-clean/test/logger.spec.ts | 70 +++++++++++++++++++ .../build-clean/test/plugin-factory.spec.ts | 53 ++++++++++++++ .../test/process-cwd-fallback.spec.ts | 47 +++++++++++++ packages/build-clean/test/refusals.spec.ts | 16 +++++ .../test/remove-empty-dirs.spec.ts | 69 ++++++++++++++++++ 8 files changed, 321 insertions(+), 3 deletions(-) create mode 100644 packages/build-clean/test/delete-failure.spec.ts create mode 100644 packages/build-clean/test/first-build.spec.ts create mode 100644 packages/build-clean/test/logger.spec.ts create mode 100644 packages/build-clean/test/plugin-factory.spec.ts create mode 100644 packages/build-clean/test/process-cwd-fallback.spec.ts create mode 100644 packages/build-clean/test/remove-empty-dirs.spec.ts diff --git a/packages/build-clean/src/core/cleanUnusedFiles.ts b/packages/build-clean/src/core/cleanUnusedFiles.ts index 192e994..b91400f 100644 --- a/packages/build-clean/src/core/cleanUnusedFiles.ts +++ b/packages/build-clean/src/core/cleanUnusedFiles.ts @@ -67,9 +67,13 @@ export async function cleanUnusedFiles(outDir: string, builtFiles: Set, // Every output the build reported is missing from where it should be, so // this directory is not the one that was built into. Deleting what does not // match would take all of it. - if (builtIdentities.size === 0) { - const reason = builtFiles.size === 0 ? 'The build reported no output files' : `None of the ${builtFiles.size} files the build reported were found under "${resolvedOutDir}"`; - return refuse(reason, options); + // + // Only when there was something to find. A build that produced nothing + // matches nothing whatever directory it is pointed at, so zero matches says + // nothing about the directory, and everything present is from an earlier + // build and due to be removed. + if (builtFiles.size > 0 && builtIdentities.size === 0) { + return refuse(`None of the ${builtFiles.size} files the build reported were found under "${resolvedOutDir}"`, options); } const filesToDelete: string[] = []; diff --git a/packages/build-clean/test/delete-failure.spec.ts b/packages/build-clean/test/delete-failure.spec.ts new file mode 100644 index 0000000..1c5aa06 --- /dev/null +++ b/packages/build-clean/test/delete-failure.spec.ts @@ -0,0 +1,24 @@ +import { rm, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { deleteFile } from '../src/core/deleteFile'; +import { createCapturingLogger, createWorkspace } from './support/workspace'; + +// A file that cannot be deleted warns and lets the build carry on. Aborting on +// one unremovable leftover would fail a build that otherwise succeeded. +describe('deleting a file that cannot be removed', () => { + it('warns rather than throwing', async () => { + const workspace = await createWorkspace('delete-failure'); + const missing = join(workspace.outDir, 'already-gone.js'); + await writeFile(missing, '// about to disappear\n'); + await rm(missing); + const logger = createCapturingLogger(); + + await deleteFile(missing, logger); + + const expected = true; + const actual = logger.lines.some((line) => line.includes('Failed to delete')); + + expect(actual).toBe(expected); + }); +}); diff --git a/packages/build-clean/test/first-build.spec.ts b/packages/build-clean/test/first-build.spec.ts new file mode 100644 index 0000000..81d28b7 --- /dev/null +++ b/packages/build-clean/test/first-build.spec.ts @@ -0,0 +1,35 @@ +import { describe, expect, it } from 'vitest'; +import { cleanUnusedFiles } from '../src/core/cleanUnusedFiles'; +import { resolveOptions } from '../src/core/resolveOptions'; +import { createCapturingLogger, createWorkspace } from './support/workspace'; + +// An output directory that does not exist yet is the ordinary first build. It +// means no files, not a directory that cannot be read, and the two must not be +// treated alike. +describe('the first build', () => { + it('treats a missing output directory as empty rather than refusing', async () => { + const workspace = await createWorkspace('first-build-missing'); + const logger = createCapturingLogger(); + + await cleanUnusedFiles('not-created-yet', new Set(['dist/main.js']), workspace.root, resolveOptions({ destructive: true, logger })); + + const expected = false; + const actual = logger.lines.some((line) => line.includes('Refusing to clean')); + + expect(actual).toBe(expected); + }); + + // The build wrote files and the output directory holds none of them, which is + // what tsup's own clean does just before the plugin looks. + it('says to disable tsup clean when the output directory is empty but the build produced files', async () => { + const workspace = await createWorkspace('first-build-empty'); + const logger = createCapturingLogger(); + + await cleanUnusedFiles('dist', new Set(['dist/main.js']), workspace.root, resolveOptions({ destructive: true, logger })); + + const expected = true; + const actual = logger.lines.some((line) => line.includes('Disable tsup "clean: true"')); + + expect(actual).toBe(expected); + }); +}); diff --git a/packages/build-clean/test/logger.spec.ts b/packages/build-clean/test/logger.spec.ts new file mode 100644 index 0000000..aaee12e --- /dev/null +++ b/packages/build-clean/test/logger.spec.ts @@ -0,0 +1,70 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import { createLogger } from '../src/core/createLogger'; + +// The logger every consumer gets when they pass none. The suite otherwise +// injects a fake everywhere, so nothing exercised this, which is how a refusal +// came to print the word undefined to users without a test noticing. +describe('the default logger', () => { + afterEach(() => { + vi.restoreAllMocks(); + }); + + const captureError = () => { + const calls: unknown[][] = []; + vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { + calls.push(args); + }); + return calls; + }; + + it('passes the message through to the console', () => { + const calls = captureError(); + + createLogger({ prefix: 'build-cleaner' }).error('something went wrong'); + + const expected = true; + const actual = calls.some((call) => call.includes('something went wrong')); + + expect(actual).toBe(expected); + }); + + // The logger spreads its extra arguments, so a caller passing an absent one + // still hands console a value to render. + it('passes nothing beyond the message when called with only a message', () => { + const calls = captureError(); + + createLogger({ prefix: 'build-cleaner' }).error('something went wrong'); + + const expected = 2; + const actual = calls[0].length; + + expect(actual).toBe(expected); + }); + + it('expands an object argument rather than printing it as [object Object]', () => { + const calls = captureError(); + + createLogger({ prefix: 'build-cleaner' }).error({ nested: { value: 1 } } as unknown as string); + + const expected = true; + const actual = calls[0].some((argument) => typeof argument === 'string' && argument.includes('nested')); + + expect(actual).toBe(expected); + }); + + // What resolveOptions hands it for every consumer who asks for neither, since + // both default to false there. + it('says nothing at debug level when debug is off', () => { + const calls: unknown[][] = []; + vi.spyOn(console, 'debug').mockImplementation((...args: unknown[]) => { + calls.push(args); + }); + + createLogger({ prefix: 'build-cleaner', debug: false }).debug('noise'); + + const expected = 0; + const actual = calls.length; + + expect(actual).toBe(expected); + }); +}); diff --git a/packages/build-clean/test/plugin-factory.spec.ts b/packages/build-clean/test/plugin-factory.spec.ts new file mode 100644 index 0000000..d83b673 --- /dev/null +++ b/packages/build-clean/test/plugin-factory.spec.ts @@ -0,0 +1,53 @@ +import type { Plugin } from 'esbuild'; +import { build } from 'esbuild'; +import { describe, expect, it } from 'vitest'; +import cleanPlugin from '../src/esbuild'; +import { buildOptions, createCapturingLogger, createWorkspace } from './support/workspace'; + +const errorTexts = (failure: unknown): string[] => (failure as { errors?: { text: string }[] }).errors?.map((error) => error.text) ?? []; + +// The two things the plugin refuses to proceed without. Both are configuration +// faults rather than anything about the output directory's contents, so they +// throw rather than going through the refusal path. +describe('what the esbuild hook requires', () => { + it('reports a build that has no output directory', async () => { + const workspace = await createWorkspace('no-outdir'); + const logger = createCapturingLogger(); + + const failure = await build({ + absWorkingDir: workspace.root, + entryPoints: ['src/main.ts'], + outfile: 'out.js', + bundle: true, + format: 'esm', + platform: 'node', + target: 'node22', + plugins: [cleanPlugin({ logger })], + }).catch((error: unknown) => error); + + const expected = true; + const actual = errorTexts(failure).some((text) => text.includes('No output directory specified')); + + expect(actual).toBe(expected); + }); + + // setup turns the metafile on, so its absence after a successful build means + // something else turned it back off. + it('reports a metafile switched off after the plugin enabled it', async () => { + const workspace = await createWorkspace('metafile-removed'); + const logger = createCapturingLogger(); + const disableMetafile: Plugin = { + name: 'disable-metafile', + setup(build) { + build.initialOptions.metafile = false; + }, + }; + + const failure = await build({ ...buildOptions(workspace), plugins: [cleanPlugin({ logger }), disableMetafile] }).catch((error: unknown) => error); + + const expected = true; + const actual = errorTexts(failure).some((text) => text.includes('No metafile available')); + + expect(actual).toBe(expected); + }); +}); diff --git a/packages/build-clean/test/process-cwd-fallback.spec.ts b/packages/build-clean/test/process-cwd-fallback.spec.ts new file mode 100644 index 0000000..f54f0db --- /dev/null +++ b/packages/build-clean/test/process-cwd-fallback.spec.ts @@ -0,0 +1,47 @@ +import { writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import type { PluginBuild } from 'esbuild'; +import { describe, expect, it } from 'vitest'; +import cleanPlugin from '../src/esbuild'; +import { createCapturingLogger, createWorkspace, listOutput } from './support/workspace'; + +// esbuild leaves initialOptions.absWorkingDir undefined when a build does not +// set it, which is the common case, and keys its metafile relative to the +// process working directory instead. Every other test sets absWorkingDir, so +// nothing else reaches the fallback. +// +// Driving the hook directly rather than through a real build: esbuild's service +// keeps its own working directory, so moving the process one moves what the +// plugin sees without moving what esbuild resolves against, and the build would +// fail for an unrelated reason. +describe('a build that does not set absWorkingDir', () => { + it('cleans relative to the process working directory', async () => { + const workspace = await createWorkspace('process-cwd-fallback'); + const logger = createCapturingLogger(); + await writeFile(join(workspace.outDir, 'main.js'), 'console.log(1);\n'); + await writeFile(join(workspace.outDir, 'stale.js'), '// left over from an earlier build\n'); + + let onEnd: ((result: unknown) => Promise) | undefined; + const fakeBuild = { + initialOptions: { outdir: 'dist' }, + onEnd: (callback: (result: unknown) => Promise) => { + onEnd = callback; + }, + } as unknown as PluginBuild; + + cleanPlugin({ destructive: true, logger }).setup(fakeBuild); + + const originalCwd = process.cwd(); + try { + process.chdir(workspace.root); + await onEnd?.({ errors: [], warnings: [], metafile: { outputs: { 'dist/main.js': {} } } }); + } finally { + process.chdir(originalCwd); + } + + const expected = ['main.js']; + const actual = await listOutput(workspace.outDir); + + expect(actual).toEqual(expected); + }); +}); diff --git a/packages/build-clean/test/refusals.spec.ts b/packages/build-clean/test/refusals.spec.ts index 2f799ed..8767c44 100644 --- a/packages/build-clean/test/refusals.spec.ts +++ b/packages/build-clean/test/refusals.spec.ts @@ -22,6 +22,22 @@ describe('refusing to clean', () => { expect(actual).toEqual(expected); }); + // The boundary of the refusal. Zero matches only means the wrong directory + // when there was something to match: a build that produced nothing matches + // nothing by definition, and everything present is from an earlier build. + it('removes stale files when the build produced no outputs at all', async () => { + const workspace = await createWorkspace('no-outputs'); + await writeFile(join(workspace.outDir, 'stale.js'), '// from an earlier build\n'); + const options = resolveOptions({ destructive: true, logger: createCapturingLogger() }); + + await cleanUnusedFiles('dist', new Set(), workspace.root, options); + + const expected: string[] = []; + const actual = await listOutput(workspace.outDir); + + expect(actual).toEqual(expected); + }); + it('fails the build when no built file is found and strict is on', async () => { const workspace = await createWorkspace('refuse-nothing-resolved-strict'); await writeFile(join(workspace.outDir, 'stale.js'), '// not a build output\n'); diff --git a/packages/build-clean/test/remove-empty-dirs.spec.ts b/packages/build-clean/test/remove-empty-dirs.spec.ts new file mode 100644 index 0000000..a391efb --- /dev/null +++ b/packages/build-clean/test/remove-empty-dirs.spec.ts @@ -0,0 +1,69 @@ +import { mkdir, readdir, writeFile } from 'node:fs/promises'; +import { join } from 'node:path'; +import { describe, expect, it } from 'vitest'; +import { removeEmptyDirs } from '../src/core/removeEmptyDirs'; +import { resolveOptions } from '../src/core/resolveOptions'; +import { createCapturingLogger, createWorkspace } from './support/workspace'; + +describe('removing empty directories', () => { + it('removes a directory left with nothing in it', async () => { + const workspace = await createWorkspace('remove-empty-nested'); + await mkdir(join(workspace.outDir, 'empty'), { recursive: true }); + await writeFile(join(workspace.outDir, 'kept.js'), '// keeps the parent alive\n'); + const options = resolveOptions({ destructive: true, logger: createCapturingLogger() }); + + await removeEmptyDirs(workspace.outDir, options); + + const expected = false; + const actual = await mkdir(join(workspace.outDir, 'empty'), { recursive: false }).then( + () => false, + () => true, + ); + + expect(actual).toBe(expected); + }); + + // destructive is off by default, so this is what most consumers get. + it('leaves the directory in place when not destructive', async () => { + const workspace = await createWorkspace('remove-empty-dry-run'); + const emptyDir = join(workspace.outDir, 'empty'); + await mkdir(emptyDir, { recursive: true }); + const options = resolveOptions({ destructive: false, logger: createCapturingLogger() }); + + await removeEmptyDirs(emptyDir, options); + + const expected = true; + const actual = await readdir(emptyDir).then( + () => true, + () => false, + ); + + expect(actual).toBe(expected); + }); + + // Reading the directory is guarded and reports the failure, but the check for + // what is left in it is not, so the failure surfaces as a throw regardless. + // Pinned as it stands; the guard does not do what its shape suggests. + it('reports a directory it cannot read', async () => { + const workspace = await createWorkspace('remove-empty-unreadable'); + const notADirectory = join(workspace.root, 'notadirectory'); + await writeFile(notADirectory, 'this is a file\n'); + const logger = createCapturingLogger(); + + await removeEmptyDirs(notADirectory, resolveOptions({ destructive: true, logger })).catch(() => {}); + + const expected = true; + const actual = logger.lines.some((line) => line.includes('Error reading directory')); + + expect(actual).toBe(expected); + }); + + it('throws when given something that is not a directory', async () => { + const workspace = await createWorkspace('remove-empty-throws'); + const notADirectory = join(workspace.root, 'notadirectory'); + await writeFile(notADirectory, 'this is a file\n'); + const options = resolveOptions({ destructive: true, logger: createCapturingLogger() }); + + await expect(removeEmptyDirs(notADirectory, options)).rejects.toThrow(); + }); +}); From cf59f18a19de8569c00d617513cb158726f6e7a6 Mon Sep 17 00:00:00 2001 From: Stephen Hellicar Date: Wed, 2 Sep 2026 11:48:06 +1000 Subject: [PATCH 11/11] Throw a named error type for each way the plugin refuses Every failure was a bare Error carrying a sentence, so the only way to tell one from another, in a catch block or in a test, was to match the prose. The message became the contract by accident, and a test asserting it breaks on a reword and holds when the behaviour is wrong. Each now has its own exported type, over a shared base so a caller can catch the lot or one of them, following the shape build-graphql already uses. The base passes its message to Error rather than leaving it empty, since these surface in a build log where the sentence is what gets read. Also removes four test files that existed to move a coverage number rather than to prove anything. They asserted the message text of the line above them, or how many arguments reached console, which can only fail when the implementation is reworded and cannot fail when the behaviour is wrong. The logger tests were the worst of it: a console wrapper is not ours to prove, and testing it meant replacing console and asserting on that instead. --- packages/build-clean/CHANGELOG.md | 1 + packages/build-clean/changes.jsonl | 1 + .../build-clean/src/core/cleanUnusedFiles.ts | 15 ++-- .../build-clean/src/core/pluginFactory.ts | 6 +- .../build-clean/src/core/validateOutDir.ts | 12 ++-- .../build-clean/src/errors/BuildCleanError.ts | 9 +++ .../src/errors/CleanRefusedError.ts | 15 ++++ .../src/errors/MissingMetafileError.ts | 11 +++ .../src/errors/MissingOutputDirectoryError.ts | 11 +++ .../OutputDirectoryContainsBaseError.ts | 13 ++++ .../src/errors/OutputDirectoryIsBaseError.ts | 13 ++++ .../errors/OutputDirectoryIsSourceError.ts | 14 ++++ .../errors/OutputDirectoryOutsideBaseError.ts | 14 ++++ packages/build-clean/src/errors/index.ts | 10 +++ packages/build-clean/src/index.ts | 5 +- .../build-clean/test/delete-failure.spec.ts | 24 ------- packages/build-clean/test/first-build.spec.ts | 35 ---------- packages/build-clean/test/logger.spec.ts | 70 ------------------- .../build-clean/test/plugin-factory.spec.ts | 53 -------------- packages/build-clean/test/refusals.spec.ts | 5 +- .../test/remove-empty-dirs.spec.ts | 26 ------- .../build-clean/test/validateOutDir.spec.ts | 25 ++++--- 22 files changed, 153 insertions(+), 235 deletions(-) create mode 100644 packages/build-clean/src/errors/BuildCleanError.ts create mode 100644 packages/build-clean/src/errors/CleanRefusedError.ts create mode 100644 packages/build-clean/src/errors/MissingMetafileError.ts create mode 100644 packages/build-clean/src/errors/MissingOutputDirectoryError.ts create mode 100644 packages/build-clean/src/errors/OutputDirectoryContainsBaseError.ts create mode 100644 packages/build-clean/src/errors/OutputDirectoryIsBaseError.ts create mode 100644 packages/build-clean/src/errors/OutputDirectoryIsSourceError.ts create mode 100644 packages/build-clean/src/errors/OutputDirectoryOutsideBaseError.ts create mode 100644 packages/build-clean/src/errors/index.ts delete mode 100644 packages/build-clean/test/delete-failure.spec.ts delete mode 100644 packages/build-clean/test/first-build.spec.ts delete mode 100644 packages/build-clean/test/logger.spec.ts delete mode 100644 packages/build-clean/test/plugin-factory.spec.ts diff --git a/packages/build-clean/CHANGELOG.md b/packages/build-clean/CHANGELOG.md index b8ef9b0..d7e7dd4 100644 --- a/packages/build-clean/CHANGELOG.md +++ b/packages/build-clean/CHANGELOG.md @@ -10,6 +10,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added - Added a strict option that turns a refusal to clean into a build failure +- Added an exported error type for each failure the plugin raises, so they can be caught by type rather than by message ### Changed diff --git a/packages/build-clean/changes.jsonl b/packages/build-clean/changes.jsonl index ac87120..2b91bd7 100644 --- a/packages/build-clean/changes.jsonl +++ b/packages/build-clean/changes.jsonl @@ -56,3 +56,4 @@ {"description":"Added a strict option that turns a refusal to clean into a build failure","category":"added"} {"description":"Hard links to build outputs are no longer removed","category":"changed"} {"description":"Corrected the readme: cleaning runs on the esbuild path only","category":"changed"} +{"description":"Added an exported error type for each failure the plugin raises, so they can be caught by type rather than by message","category":"added"} diff --git a/packages/build-clean/src/core/cleanUnusedFiles.ts b/packages/build-clean/src/core/cleanUnusedFiles.ts index b91400f..e822ecf 100644 --- a/packages/build-clean/src/core/cleanUnusedFiles.ts +++ b/packages/build-clean/src/core/cleanUnusedFiles.ts @@ -1,5 +1,6 @@ import { relative, resolve } from 'node:path'; import { Feature } from '../enums'; +import { CleanRefusedError } from '../errors/CleanRefusedError'; import { deleteFile } from './deleteFile'; import { fileIdentity } from './fileIdentity'; import { getAllFiles } from './getAllFiles'; @@ -10,23 +11,19 @@ import { validateOutDir } from './validateOutDir'; // Nothing is deleted when the plugin cannot trust what it is looking at. The // refusal is loud but does not fail the build unless the caller asked for that, // because a plugin that starts breaking builds on upgrade is its own incident. -// Thrown only by refuse, so the catch below can let it through without logging -// a refusal that has already been reported. -class RefusalError extends Error {} - const refuse = (reason: string, options: ResolvedOptions, cause?: unknown): void => { - const message = `[build-cleaner] Refusing to clean. ${reason}`; + const refusal = new CleanRefusedError(reason, { cause }); // Passing an absent cause still counts as an argument, and the logger spreads // its arguments, so the word undefined would reach the user. if (cause === undefined) { - options.logger.error(message); + options.logger.error(refusal.message); } else { - options.logger.error(message, cause); + options.logger.error(refusal.message, cause); } if (options.strict) { - throw new RefusalError(message, { cause }); + throw refusal; } }; @@ -120,7 +117,7 @@ export async function cleanUnusedFiles(outDir: string, builtFiles: Set, await removeEmptyDirs(resolvedOutDir, options); } } catch (error) { - if (error instanceof RefusalError) { + if (error instanceof CleanRefusedError) { throw error; } logger.error('Error during cleanup:', error); diff --git a/packages/build-clean/src/core/pluginFactory.ts b/packages/build-clean/src/core/pluginFactory.ts index 9cf8590..50e09a6 100644 --- a/packages/build-clean/src/core/pluginFactory.ts +++ b/packages/build-clean/src/core/pluginFactory.ts @@ -1,4 +1,6 @@ import type { UnpluginFactory, UnpluginOptions } from 'unplugin'; +import { MissingMetafileError } from '../errors/MissingMetafileError'; +import { MissingOutputDirectoryError } from '../errors/MissingOutputDirectoryError'; import type { Options } from '../types'; import { cleanUnusedFiles } from './cleanUnusedFiles'; import { resolveOptions } from './resolveOptions'; @@ -27,13 +29,13 @@ export const pluginFactory: UnpluginFactory = (initialOptio const outdir = build.initialOptions.outdir; if (!outdir) { - throw new Error('[build-cleaner] No output directory specified in build options'); + throw new MissingOutputDirectoryError(); } // The build succeeded, so this means something removed the metafile // option that setup turned on. if (!result.metafile) { - throw new Error('[build-cleaner] No metafile available - ensure metafile is enabled'); + throw new MissingMetafileError(); } const builtFiles = new Set(Object.keys(result.metafile.outputs)); diff --git a/packages/build-clean/src/core/validateOutDir.ts b/packages/build-clean/src/core/validateOutDir.ts index a29fbe9..78b857b 100644 --- a/packages/build-clean/src/core/validateOutDir.ts +++ b/packages/build-clean/src/core/validateOutDir.ts @@ -1,4 +1,8 @@ import { relative, resolve } from 'node:path'; +import { OutputDirectoryContainsBaseError } from '../errors/OutputDirectoryContainsBaseError'; +import { OutputDirectoryIsBaseError } from '../errors/OutputDirectoryIsBaseError'; +import { OutputDirectoryIsSourceError } from '../errors/OutputDirectoryIsSourceError'; +import { OutputDirectoryOutsideBaseError } from '../errors/OutputDirectoryOutsideBaseError'; import type { ILogger } from '../types'; import { isOutsideBase } from './isOutsideBase'; @@ -31,17 +35,17 @@ export const validateOutDir = (outDir: string, baseDir: string, logger: ILogger) // Check if the resolved path is the same as the base directory if (isSameAsCurrentDir) { - throw new Error(`[build-cleaner] Refusing to clean current directory: "${outDir}". Use a subdirectory like "dist" or "build".`); + throw new OutputDirectoryIsBaseError(outDir); } // Check if the resolved path is a parent of the base directory if (isParentOfCurrentDir) { - throw new Error(`[build-cleaner] Refusing to clean parent directory: "${outDir}". This would delete the current project.`); + throw new OutputDirectoryContainsBaseError(outDir); } // Outside the project, whether by climbing out or by having no route at all if (isOutside) { - throw new Error(`[build-cleaner] Refusing to clean directory outside project: "${outDir}". Use a subdirectory like "dist" or "build".`); + throw new OutputDirectoryOutsideBaseError(outDir); } // Prevent cleaning common source directories (even as subdirectories) @@ -49,7 +53,7 @@ export const validateOutDir = (outDir: string, baseDir: string, logger: ILogger) const isDangerousPath = dangerousPaths.some((dangerous) => normalizedPath === dangerous || normalizedPath.endsWith(`/${dangerous}`)); if (isDangerousPath) { - throw new Error(`[build-cleaner] Refusing to clean source directory: "${outDir}". Use a build output directory like "dist" or "build".`); + throw new OutputDirectoryIsSourceError(outDir); } logger.debug(`Validated output directory: "${outDir}" -> "${resolvedOutDir}"`); diff --git a/packages/build-clean/src/errors/BuildCleanError.ts b/packages/build-clean/src/errors/BuildCleanError.ts new file mode 100644 index 0000000..3888fc0 --- /dev/null +++ b/packages/build-clean/src/errors/BuildCleanError.ts @@ -0,0 +1,9 @@ +export abstract class BuildCleanError extends Error { + public readonly kind: string; + + protected constructor(kind: string, message: string, options?: ErrorOptions) { + super(message, options); + this.kind = kind; + this.name = new.target.name; + } +} diff --git a/packages/build-clean/src/errors/CleanRefusedError.ts b/packages/build-clean/src/errors/CleanRefusedError.ts new file mode 100644 index 0000000..19811dd --- /dev/null +++ b/packages/build-clean/src/errors/CleanRefusedError.ts @@ -0,0 +1,15 @@ +import { BuildCleanError } from './BuildCleanError'; + +/** + * Thrown when the plugin cannot tell what it is looking at and declines to + * delete anything. Only thrown when `strict` is on; otherwise the refusal is + * logged and the build carries on. + */ +export class CleanRefusedError extends BuildCleanError { + public readonly reason: string; + + public constructor(reason: string, options?: ErrorOptions) { + super('CleanRefused', `[build-cleaner] Refusing to clean. ${reason}`, options); + this.reason = reason; + } +} diff --git a/packages/build-clean/src/errors/MissingMetafileError.ts b/packages/build-clean/src/errors/MissingMetafileError.ts new file mode 100644 index 0000000..4a04d22 --- /dev/null +++ b/packages/build-clean/src/errors/MissingMetafileError.ts @@ -0,0 +1,11 @@ +import { BuildCleanError } from './BuildCleanError'; + +/** + * Thrown when a build succeeded without producing a metafile. The plugin turns + * the metafile on itself, so its absence means something switched it back off. + */ +export class MissingMetafileError extends BuildCleanError { + public constructor() { + super('MissingMetafile', '[build-cleaner] No metafile available - ensure metafile is enabled'); + } +} diff --git a/packages/build-clean/src/errors/MissingOutputDirectoryError.ts b/packages/build-clean/src/errors/MissingOutputDirectoryError.ts new file mode 100644 index 0000000..f63ad34 --- /dev/null +++ b/packages/build-clean/src/errors/MissingOutputDirectoryError.ts @@ -0,0 +1,11 @@ +import { BuildCleanError } from './BuildCleanError'; + +/** + * Thrown when the build specifies no output directory, so there is nothing for + * the plugin to clean. + */ +export class MissingOutputDirectoryError extends BuildCleanError { + public constructor() { + super('MissingOutputDirectory', '[build-cleaner] No output directory specified in build options'); + } +} diff --git a/packages/build-clean/src/errors/OutputDirectoryContainsBaseError.ts b/packages/build-clean/src/errors/OutputDirectoryContainsBaseError.ts new file mode 100644 index 0000000..31c4d81 --- /dev/null +++ b/packages/build-clean/src/errors/OutputDirectoryContainsBaseError.ts @@ -0,0 +1,13 @@ +import { BuildCleanError } from './BuildCleanError'; + +/** + * Thrown when the output directory contains the directory the build is rooted at. + */ +export class OutputDirectoryContainsBaseError extends BuildCleanError { + public readonly outDir: string; + + public constructor(outDir: string) { + super('OutputDirectoryContainsBase', `[build-cleaner] Refusing to clean parent directory: "${outDir}". This would delete the current project.`); + this.outDir = outDir; + } +} diff --git a/packages/build-clean/src/errors/OutputDirectoryIsBaseError.ts b/packages/build-clean/src/errors/OutputDirectoryIsBaseError.ts new file mode 100644 index 0000000..af619de --- /dev/null +++ b/packages/build-clean/src/errors/OutputDirectoryIsBaseError.ts @@ -0,0 +1,13 @@ +import { BuildCleanError } from './BuildCleanError'; + +/** + * Thrown when the output directory is the directory the build is rooted at. + */ +export class OutputDirectoryIsBaseError extends BuildCleanError { + public readonly outDir: string; + + public constructor(outDir: string) { + super('OutputDirectoryIsBase', `[build-cleaner] Refusing to clean current directory: "${outDir}". Use a subdirectory like "dist" or "build".`); + this.outDir = outDir; + } +} diff --git a/packages/build-clean/src/errors/OutputDirectoryIsSourceError.ts b/packages/build-clean/src/errors/OutputDirectoryIsSourceError.ts new file mode 100644 index 0000000..2170caa --- /dev/null +++ b/packages/build-clean/src/errors/OutputDirectoryIsSourceError.ts @@ -0,0 +1,14 @@ +import { BuildCleanError } from './BuildCleanError'; + +/** + * Thrown when the output directory carries the name of a directory that + * normally holds source rather than build output. + */ +export class OutputDirectoryIsSourceError extends BuildCleanError { + public readonly outDir: string; + + public constructor(outDir: string) { + super('OutputDirectoryIsSource', `[build-cleaner] Refusing to clean source directory: "${outDir}". Use a build output directory like "dist" or "build".`); + this.outDir = outDir; + } +} diff --git a/packages/build-clean/src/errors/OutputDirectoryOutsideBaseError.ts b/packages/build-clean/src/errors/OutputDirectoryOutsideBaseError.ts new file mode 100644 index 0000000..984b782 --- /dev/null +++ b/packages/build-clean/src/errors/OutputDirectoryOutsideBaseError.ts @@ -0,0 +1,14 @@ +import { BuildCleanError } from './BuildCleanError'; + +/** + * Thrown when the output directory lies outside the directory the build is + * rooted at, whether by climbing out of it or by being on another root. + */ +export class OutputDirectoryOutsideBaseError extends BuildCleanError { + public readonly outDir: string; + + public constructor(outDir: string) { + super('OutputDirectoryOutsideBase', `[build-cleaner] Refusing to clean directory outside project: "${outDir}". Use a subdirectory like "dist" or "build".`); + this.outDir = outDir; + } +} diff --git a/packages/build-clean/src/errors/index.ts b/packages/build-clean/src/errors/index.ts new file mode 100644 index 0000000..487dbc0 --- /dev/null +++ b/packages/build-clean/src/errors/index.ts @@ -0,0 +1,10 @@ +import { BuildCleanError } from './BuildCleanError'; +import { CleanRefusedError } from './CleanRefusedError'; +import { MissingMetafileError } from './MissingMetafileError'; +import { MissingOutputDirectoryError } from './MissingOutputDirectoryError'; +import { OutputDirectoryContainsBaseError } from './OutputDirectoryContainsBaseError'; +import { OutputDirectoryIsBaseError } from './OutputDirectoryIsBaseError'; +import { OutputDirectoryIsSourceError } from './OutputDirectoryIsSourceError'; +import { OutputDirectoryOutsideBaseError } from './OutputDirectoryOutsideBaseError'; + +export { BuildCleanError, CleanRefusedError, MissingMetafileError, MissingOutputDirectoryError, OutputDirectoryContainsBaseError, OutputDirectoryIsBaseError, OutputDirectoryIsSourceError, OutputDirectoryOutsideBaseError }; diff --git a/packages/build-clean/src/index.ts b/packages/build-clean/src/index.ts index fc4e969..847b025 100644 --- a/packages/build-clean/src/index.ts +++ b/packages/build-clean/src/index.ts @@ -1 +1,4 @@ -export { plugin as default } from './core/plugin'; +import { plugin } from './core/plugin'; +import { BuildCleanError, CleanRefusedError, MissingMetafileError, MissingOutputDirectoryError, OutputDirectoryContainsBaseError, OutputDirectoryIsBaseError, OutputDirectoryIsSourceError, OutputDirectoryOutsideBaseError } from './errors'; + +export { BuildCleanError, CleanRefusedError, MissingMetafileError, MissingOutputDirectoryError, OutputDirectoryContainsBaseError, OutputDirectoryIsBaseError, OutputDirectoryIsSourceError, OutputDirectoryOutsideBaseError, plugin as default }; diff --git a/packages/build-clean/test/delete-failure.spec.ts b/packages/build-clean/test/delete-failure.spec.ts deleted file mode 100644 index 1c5aa06..0000000 --- a/packages/build-clean/test/delete-failure.spec.ts +++ /dev/null @@ -1,24 +0,0 @@ -import { rm, writeFile } from 'node:fs/promises'; -import { join } from 'node:path'; -import { describe, expect, it } from 'vitest'; -import { deleteFile } from '../src/core/deleteFile'; -import { createCapturingLogger, createWorkspace } from './support/workspace'; - -// A file that cannot be deleted warns and lets the build carry on. Aborting on -// one unremovable leftover would fail a build that otherwise succeeded. -describe('deleting a file that cannot be removed', () => { - it('warns rather than throwing', async () => { - const workspace = await createWorkspace('delete-failure'); - const missing = join(workspace.outDir, 'already-gone.js'); - await writeFile(missing, '// about to disappear\n'); - await rm(missing); - const logger = createCapturingLogger(); - - await deleteFile(missing, logger); - - const expected = true; - const actual = logger.lines.some((line) => line.includes('Failed to delete')); - - expect(actual).toBe(expected); - }); -}); diff --git a/packages/build-clean/test/first-build.spec.ts b/packages/build-clean/test/first-build.spec.ts deleted file mode 100644 index 81d28b7..0000000 --- a/packages/build-clean/test/first-build.spec.ts +++ /dev/null @@ -1,35 +0,0 @@ -import { describe, expect, it } from 'vitest'; -import { cleanUnusedFiles } from '../src/core/cleanUnusedFiles'; -import { resolveOptions } from '../src/core/resolveOptions'; -import { createCapturingLogger, createWorkspace } from './support/workspace'; - -// An output directory that does not exist yet is the ordinary first build. It -// means no files, not a directory that cannot be read, and the two must not be -// treated alike. -describe('the first build', () => { - it('treats a missing output directory as empty rather than refusing', async () => { - const workspace = await createWorkspace('first-build-missing'); - const logger = createCapturingLogger(); - - await cleanUnusedFiles('not-created-yet', new Set(['dist/main.js']), workspace.root, resolveOptions({ destructive: true, logger })); - - const expected = false; - const actual = logger.lines.some((line) => line.includes('Refusing to clean')); - - expect(actual).toBe(expected); - }); - - // The build wrote files and the output directory holds none of them, which is - // what tsup's own clean does just before the plugin looks. - it('says to disable tsup clean when the output directory is empty but the build produced files', async () => { - const workspace = await createWorkspace('first-build-empty'); - const logger = createCapturingLogger(); - - await cleanUnusedFiles('dist', new Set(['dist/main.js']), workspace.root, resolveOptions({ destructive: true, logger })); - - const expected = true; - const actual = logger.lines.some((line) => line.includes('Disable tsup "clean: true"')); - - expect(actual).toBe(expected); - }); -}); diff --git a/packages/build-clean/test/logger.spec.ts b/packages/build-clean/test/logger.spec.ts deleted file mode 100644 index aaee12e..0000000 --- a/packages/build-clean/test/logger.spec.ts +++ /dev/null @@ -1,70 +0,0 @@ -import { afterEach, describe, expect, it, vi } from 'vitest'; -import { createLogger } from '../src/core/createLogger'; - -// The logger every consumer gets when they pass none. The suite otherwise -// injects a fake everywhere, so nothing exercised this, which is how a refusal -// came to print the word undefined to users without a test noticing. -describe('the default logger', () => { - afterEach(() => { - vi.restoreAllMocks(); - }); - - const captureError = () => { - const calls: unknown[][] = []; - vi.spyOn(console, 'error').mockImplementation((...args: unknown[]) => { - calls.push(args); - }); - return calls; - }; - - it('passes the message through to the console', () => { - const calls = captureError(); - - createLogger({ prefix: 'build-cleaner' }).error('something went wrong'); - - const expected = true; - const actual = calls.some((call) => call.includes('something went wrong')); - - expect(actual).toBe(expected); - }); - - // The logger spreads its extra arguments, so a caller passing an absent one - // still hands console a value to render. - it('passes nothing beyond the message when called with only a message', () => { - const calls = captureError(); - - createLogger({ prefix: 'build-cleaner' }).error('something went wrong'); - - const expected = 2; - const actual = calls[0].length; - - expect(actual).toBe(expected); - }); - - it('expands an object argument rather than printing it as [object Object]', () => { - const calls = captureError(); - - createLogger({ prefix: 'build-cleaner' }).error({ nested: { value: 1 } } as unknown as string); - - const expected = true; - const actual = calls[0].some((argument) => typeof argument === 'string' && argument.includes('nested')); - - expect(actual).toBe(expected); - }); - - // What resolveOptions hands it for every consumer who asks for neither, since - // both default to false there. - it('says nothing at debug level when debug is off', () => { - const calls: unknown[][] = []; - vi.spyOn(console, 'debug').mockImplementation((...args: unknown[]) => { - calls.push(args); - }); - - createLogger({ prefix: 'build-cleaner', debug: false }).debug('noise'); - - const expected = 0; - const actual = calls.length; - - expect(actual).toBe(expected); - }); -}); diff --git a/packages/build-clean/test/plugin-factory.spec.ts b/packages/build-clean/test/plugin-factory.spec.ts deleted file mode 100644 index d83b673..0000000 --- a/packages/build-clean/test/plugin-factory.spec.ts +++ /dev/null @@ -1,53 +0,0 @@ -import type { Plugin } from 'esbuild'; -import { build } from 'esbuild'; -import { describe, expect, it } from 'vitest'; -import cleanPlugin from '../src/esbuild'; -import { buildOptions, createCapturingLogger, createWorkspace } from './support/workspace'; - -const errorTexts = (failure: unknown): string[] => (failure as { errors?: { text: string }[] }).errors?.map((error) => error.text) ?? []; - -// The two things the plugin refuses to proceed without. Both are configuration -// faults rather than anything about the output directory's contents, so they -// throw rather than going through the refusal path. -describe('what the esbuild hook requires', () => { - it('reports a build that has no output directory', async () => { - const workspace = await createWorkspace('no-outdir'); - const logger = createCapturingLogger(); - - const failure = await build({ - absWorkingDir: workspace.root, - entryPoints: ['src/main.ts'], - outfile: 'out.js', - bundle: true, - format: 'esm', - platform: 'node', - target: 'node22', - plugins: [cleanPlugin({ logger })], - }).catch((error: unknown) => error); - - const expected = true; - const actual = errorTexts(failure).some((text) => text.includes('No output directory specified')); - - expect(actual).toBe(expected); - }); - - // setup turns the metafile on, so its absence after a successful build means - // something else turned it back off. - it('reports a metafile switched off after the plugin enabled it', async () => { - const workspace = await createWorkspace('metafile-removed'); - const logger = createCapturingLogger(); - const disableMetafile: Plugin = { - name: 'disable-metafile', - setup(build) { - build.initialOptions.metafile = false; - }, - }; - - const failure = await build({ ...buildOptions(workspace), plugins: [cleanPlugin({ logger }), disableMetafile] }).catch((error: unknown) => error); - - const expected = true; - const actual = errorTexts(failure).some((text) => text.includes('No metafile available')); - - expect(actual).toBe(expected); - }); -}); diff --git a/packages/build-clean/test/refusals.spec.ts b/packages/build-clean/test/refusals.spec.ts index 8767c44..95795fb 100644 --- a/packages/build-clean/test/refusals.spec.ts +++ b/packages/build-clean/test/refusals.spec.ts @@ -3,6 +3,7 @@ import { join } from 'node:path'; import { describe, expect, it } from 'vitest'; import { cleanUnusedFiles } from '../src/core/cleanUnusedFiles'; import { resolveOptions } from '../src/core/resolveOptions'; +import { CleanRefusedError } from '../src/errors/CleanRefusedError'; import { createCapturingLogger, createWorkspace, listOutput } from './support/workspace'; // When the plugin cannot trust what it is looking at, it deletes nothing. This @@ -43,7 +44,7 @@ describe('refusing to clean', () => { await writeFile(join(workspace.outDir, 'stale.js'), '// not a build output\n'); const options = resolveOptions({ destructive: true, strict: true, logger: createCapturingLogger() }); - await expect(cleanUnusedFiles('dist', new Set(['dist/never-built.js']), workspace.root, options)).rejects.toThrow('Refusing to clean'); + await expect(cleanUnusedFiles('dist', new Set(['dist/never-built.js']), workspace.root, options)).rejects.toThrow(CleanRefusedError); }); it('does not print a trailing undefined when the refusal has no cause', async () => { @@ -94,6 +95,6 @@ describe('refusing to clean', () => { await writeFile(join(workspace.root, 'notadirectory'), 'this is a file\n'); const options = resolveOptions({ destructive: true, strict: true, logger: createCapturingLogger() }); - await expect(cleanUnusedFiles('notadirectory', new Set(['dist/main.js']), workspace.root, options)).rejects.toThrow('Refusing to clean'); + await expect(cleanUnusedFiles('notadirectory', new Set(['dist/main.js']), workspace.root, options)).rejects.toThrow(CleanRefusedError); }); }); diff --git a/packages/build-clean/test/remove-empty-dirs.spec.ts b/packages/build-clean/test/remove-empty-dirs.spec.ts index a391efb..e6c373c 100644 --- a/packages/build-clean/test/remove-empty-dirs.spec.ts +++ b/packages/build-clean/test/remove-empty-dirs.spec.ts @@ -40,30 +40,4 @@ describe('removing empty directories', () => { expect(actual).toBe(expected); }); - - // Reading the directory is guarded and reports the failure, but the check for - // what is left in it is not, so the failure surfaces as a throw regardless. - // Pinned as it stands; the guard does not do what its shape suggests. - it('reports a directory it cannot read', async () => { - const workspace = await createWorkspace('remove-empty-unreadable'); - const notADirectory = join(workspace.root, 'notadirectory'); - await writeFile(notADirectory, 'this is a file\n'); - const logger = createCapturingLogger(); - - await removeEmptyDirs(notADirectory, resolveOptions({ destructive: true, logger })).catch(() => {}); - - const expected = true; - const actual = logger.lines.some((line) => line.includes('Error reading directory')); - - expect(actual).toBe(expected); - }); - - it('throws when given something that is not a directory', async () => { - const workspace = await createWorkspace('remove-empty-throws'); - const notADirectory = join(workspace.root, 'notadirectory'); - await writeFile(notADirectory, 'this is a file\n'); - const options = resolveOptions({ destructive: true, logger: createCapturingLogger() }); - - await expect(removeEmptyDirs(notADirectory, options)).rejects.toThrow(); - }); }); diff --git a/packages/build-clean/test/validateOutDir.spec.ts b/packages/build-clean/test/validateOutDir.spec.ts index a2885c8..faf6813 100644 --- a/packages/build-clean/test/validateOutDir.spec.ts +++ b/packages/build-clean/test/validateOutDir.spec.ts @@ -1,6 +1,10 @@ import { resolve } from 'node:path'; import { describe, expect, it } from 'vitest'; import { validateOutDir } from '../src/core/validateOutDir'; +import { OutputDirectoryContainsBaseError } from '../src/errors/OutputDirectoryContainsBaseError'; +import { OutputDirectoryIsBaseError } from '../src/errors/OutputDirectoryIsBaseError'; +import { OutputDirectoryIsSourceError } from '../src/errors/OutputDirectoryIsSourceError'; +import { OutputDirectoryOutsideBaseError } from '../src/errors/OutputDirectoryOutsideBaseError'; import { createCapturingLogger } from './support/workspace'; // validateOutDir is the only thing standing between the plugin and a directory @@ -30,28 +34,31 @@ describe('validateOutDir', () => { }); it('refuses the base directory itself', () => { - expect(validate('.')).toThrow('Refusing to clean current directory'); + expect(validate('.')).toThrow(OutputDirectoryIsBaseError); }); it('refuses a parent of the base directory', () => { - expect(validate('..')).toThrow('Refusing to clean parent directory'); + expect(validate('..')).toThrow(OutputDirectoryContainsBaseError); }); it('refuses a sibling of the base directory', () => { - expect(validate('../elsewhere')).toThrow('Refusing to clean directory outside project'); + expect(validate('../elsewhere')).toThrow(OutputDirectoryOutsideBaseError); }); it('refuses a source directory', () => { - expect(validate('src')).toThrow('Refusing to clean source directory'); + expect(validate('src')).toThrow(OutputDirectoryIsSourceError); }); it('refuses a nested source directory', () => { - expect(validate('packages/thing/lib')).toThrow('Refusing to clean source directory'); + expect(validate('packages/thing/lib')).toThrow(OutputDirectoryIsSourceError); }); - // The refusal has to name what the caller wrote in their config, not the path - // it was resolved to, or it points at nothing they can go and edit. - it('names the configured value in the refusal, not the resolved path', () => { - expect(validate('src')).toThrow('"src"'); + // The refusal carries what the caller wrote in their config, not the path it + // was resolved to, so it points at something they can go and edit. + it('carries the configured value rather than the resolved path', () => { + const expected = 'src'; + const actual = new OutputDirectoryIsSourceError('src').outDir; + + expect(actual).toBe(expected); }); });