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..4aace0a --- /dev/null +++ b/.github/workflows/test-matrix.yml @@ -0,0 +1,36 @@ +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. +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 + + - run: pnpm run --if-present build --only + - run: pnpm run --if-present test --only 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-clean/CHANGELOG.md b/packages/build-clean/CHANGELOG.md index 6f5f362..d7e7dd4 100644 --- a/packages/build-clean/CHANGELOG.md +++ b/packages/build-clean/CHANGELOG.md @@ -5,6 +5,28 @@ 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 + +### 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 + +- Hard links to build outputs are no longer removed +- Corrected the readme: cleaning runs on the esbuild path only + +### Fixed + +- 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 ### Changed diff --git a/packages/build-clean/README.md b/packages/build-clean/README.md index 76d79a4..d469b51 100644 --- a/packages/build-clean/README.md +++ b/packages/build-clean/README.md @@ -106,27 +106,48 @@ 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 + + /** Turn a refusal to clean into a build failure (default: false) */ + strict?: boolean + + /** Optional features. RemoveEmptyDirs is on by default */ + features?: Partial> + + /** Custom logger. When provided, debug and verbose are ignored */ + logger?: ILogger } ``` -## Other Build Tools +## What gets removed -The plugin supports other tools via [unplugin](https://github.com/unjs/unplugin): +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. -```ts -// vite.config.ts -import cleanPlugin from '@shellicar/build-clean/vite' +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. -export default defineConfig({ - plugins: [cleanPlugin({ destructive: true })] -}) -``` +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 +anything that builds through esbuild, such as tsup. + +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..2b91bd7 100644 --- a/packages/build-clean/changes.jsonl +++ b/packages/build-clean/changes.jsonl @@ -46,3 +46,14 @@ {"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 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"} +{"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/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/src/core/cleanUnusedFiles.ts b/packages/build-clean/src/core/cleanUnusedFiles.ts index 8cb2028..e822ecf 100644 --- a/packages/build-clean/src/core/cleanUnusedFiles.ts +++ b/packages/build-clean/src/core/cleanUnusedFiles.ts @@ -1,20 +1,48 @@ -import { relative } from 'node:path'; +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'; import { removeEmptyDirs } from './removeEmptyDirs'; import type { ResolvedOptions } from './types'; import { validateOutDir } from './validateOutDir'; -export async function cleanUnusedFiles(outDir: string, builtFiles: Set, options: ResolvedOptions): Promise { +// 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 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(refusal.message); + } else { + options.logger.error(refusal.message, cause); + } + + if (options.strict) { + throw refusal; + } +}; + +// 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 = validateOutDir(outDir, 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); + 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) { @@ -24,13 +52,35 @@ 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`); + + // 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. + // + // 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[] = []; 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 +97,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,9 +114,12 @@ export async function cleanUnusedFiles(outDir: string, builtFiles: Set, } if (options.features[Feature.RemoveEmptyDirs]) { - await removeEmptyDirs(outDir, options); + await removeEmptyDirs(resolvedOutDir, options); } } catch (error) { + if (error instanceof CleanRefusedError) { + throw error; + } logger.error('Error during cleanup:', error); throw error; } 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 new file mode 100644 index 0000000..d6ff97f --- /dev/null +++ b/packages/build-clean/src/core/fileIdentity.ts @@ -0,0 +1,18 @@ +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 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/pluginFactory.ts b/packages/build-clean/src/core/pluginFactory.ts index dc224f0..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'; @@ -17,19 +19,34 @@ 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'); + 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)); 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/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 26fc5d9..78b857b 100644 --- a/packages/build-clean/src/core/validateOutDir.ts +++ b/packages/build-clean/src/core/validateOutDir.ts @@ -1,49 +1,51 @@ 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'; - -export const validateOutDir = (outDir: string, logger: ILogger) => { - const cwd = process.cwd(); - const resolvedOutDir = resolve(outDir); - const relativePath = relative(cwd, resolvedOutDir); +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(cwd, outDir); - const isSameAsCurrentDir = resolvedOutDir === cwd; - const isParentOfCurrentDirUnix = cwd.startsWith(`${resolvedOutDir}/`); - const isParentOfCurrentDirWindows = cwd.startsWith(`${resolvedOutDir}\\`); + 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}"`); - 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(` Goes up directory levels: ${goesUpDirectory}`); - - // Check if the resolved path is the same as current directory + 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(` Is outside the base directory: ${isOutside}`); + + // 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 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.`); + throw new OutputDirectoryContainsBaseError(outDir); } - // Check if the relative path goes up (.., ../.., etc.) - if (goesUpDirectory) { - 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.`); + // Outside the project, whether by climbing out or by having no route at all + if (isOutside) { + throw new OutputDirectoryOutsideBaseError(outDir); } // Prevent cleaning common source directories (even as subdirectories) @@ -51,8 +53,10 @@ export const validateOutDir = (outDir: 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}"`); + + return 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/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 new file mode 100644 index 0000000..3e79cf9 --- /dev/null +++ b/packages/build-clean/test/abs-working-dir.spec.ts @@ -0,0 +1,32 @@ +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 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('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), 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/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/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/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/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/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/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 new file mode 100644 index 0000000..95795fb --- /dev/null +++ b/packages/build-clean/test/refusals.spec.ts @@ -0,0 +1,100 @@ +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 { 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 +// 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); + }); + + // 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'); + const options = resolveOptions({ destructive: true, strict: true, logger: createCapturingLogger() }); + + 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 () => { + 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 + // 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(CleanRefusedError); + }); +}); 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..17ae43a --- /dev/null +++ b/packages/build-clean/test/relative-outdir.spec.ts @@ -0,0 +1,33 @@ +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 } 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), plugins: [cleanPlugin({ destructive: false, logger })] }); + + const expected = ['dist/stale.js']; + const actual = wouldDelete(logger.lines); + + expect(actual).toEqual(expected); + }); +}); 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..e6c373c --- /dev/null +++ b/packages/build-clean/test/remove-empty-dirs.spec.ts @@ -0,0 +1,43 @@ +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); + }); +}); diff --git a/packages/build-clean/test/support/workspace.ts b/packages/build-clean/test/support/workspace.ts new file mode 100644 index 0000000..1a055cf --- /dev/null +++ b/packages/build-clean/test/support/workspace.ts @@ -0,0 +1,108 @@ +import { existsSync, mkdirSync, writeFileSync } from 'node:fs'; +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'; + +export type Workspace = { + root: string; + srcDir: string; + 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 => { + 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"); + await writeFile(join(srcDir, 'nested', 'helper.ts'), 'export const helper = () => 42;\n'); + + 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 => + ({ + absWorkingDir: workspace.root, + entryPoints: ['src/main.ts', 'src/nested/helper.ts'], + outdir: 'dist', + 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 []; + } +}; + +// 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(workspacesRoot, '.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 => { + 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'), + }; +}; diff --git a/packages/build-clean/test/validateOutDir.spec.ts b/packages/build-clean/test/validateOutDir.spec.ts new file mode 100644 index 0000000..faf6813 --- /dev/null +++ b/packages/build-clean/test/validateOutDir.spec.ts @@ -0,0 +1,64 @@ +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 +// 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(OutputDirectoryIsBaseError); + }); + + it('refuses a parent of the base directory', () => { + expect(validate('..')).toThrow(OutputDirectoryContainsBaseError); + }); + + it('refuses a sibling of the base directory', () => { + expect(validate('../elsewhere')).toThrow(OutputDirectoryOutsideBaseError); + }); + + it('refuses a source directory', () => { + expect(validate('src')).toThrow(OutputDirectoryIsSourceError); + }); + + it('refuses a nested source directory', () => { + expect(validate('packages/thing/lib')).toThrow(OutputDirectoryIsSourceError); + }); + + // 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); + }); +}); 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); diff --git a/packages/svelte-adapter-azure-functions/CHANGELOG.md b/packages/svelte-adapter-azure-functions/CHANGELOG.md index f9fc5e7..3df88df 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 an unresolvable server import in the generated function 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..e45b87e 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 an unresolvable server import in the generated function on Windows","category":"fixed"} diff --git a/packages/svelte-adapter-azure-functions/src/adapter.ts b/packages/svelte-adapter-azure-functions/src/adapter.ts index f8382cb..2a82535 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, 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..1d62b22 --- /dev/null +++ b/packages/svelte-adapter-azure-functions/src/toImportSpecifier.ts @@ -0,0 +1,7 @@ +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, 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('/'); 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); + }); +});