From 152e5de5ecf7a4398e99d5dc794d68aa280113ba Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Fri, 21 Nov 2025 14:59:24 +0000 Subject: [PATCH] refactor(linter/plugins): ensure debug assertions always shaken out of release build (#15946) The `assert*` functions are no-ops in release builds, and minifier removes them as dead code. However, there's an annoyance - we have to ensure that they're only used in files which end up in the `plugins.js` chunk. If we use those assertions elsewhere (e.g. in `cli.ts`), TSDown creates a shared chunk containing them, and then minifier can't see that they can be removed, and leaves all the assertions in the output. Fix this problem by adding a plugin to TSDown for the release build which replaces `import`s from `asserts.ts` with inlined empty functions. ```ts // Original code import { assertIs, assertIsNonNull } from '../utils/asserts.ts'; ``` ```ts // After transform function assertIs() {} function assertIsNonNull() {} ``` This allows us to use the assertion functions anywhere, and minifier can always remove them as dead code. --- apps/oxlint/package.json | 1 + apps/oxlint/tsdown.config.ts | 79 +++++++++++++++++++++++++++++++++++- pnpm-lock.yaml | 6 +++ pnpm-workspace.yaml | 1 + 4 files changed, 86 insertions(+), 1 deletion(-) diff --git a/apps/oxlint/package.json b/apps/oxlint/package.json index 2c6a191332398..1674a11f20615 100644 --- a/apps/oxlint/package.json +++ b/apps/oxlint/package.json @@ -27,6 +27,7 @@ "esquery": "^1.6.0", "execa": "^9.6.0", "jiti": "^2.6.0", + "rolldown": "catalog:", "tsdown": "catalog:", "type-fest": "^5.2.0", "typescript": "catalog:", diff --git a/apps/oxlint/tsdown.config.ts b/apps/oxlint/tsdown.config.ts index 9ceaf9e292b42..343d0d7fb821a 100644 --- a/apps/oxlint/tsdown.config.ts +++ b/apps/oxlint/tsdown.config.ts @@ -1,4 +1,13 @@ -import { defineConfig, type UserConfig } from 'tsdown'; +import { join } from 'node:path'; +import { defineConfig } from 'tsdown'; + +import type { UserConfig } from 'tsdown'; +import type { Plugin } from 'rolldown'; + +const ASSERTS_PATH = join(import.meta.dirname, 'src-js/utils/asserts.ts'); + +const replaceAssertsPlugin = createReplaceAssertsPlugin(); +const plugins = [replaceAssertsPlugin]; const commonConfig: UserConfig = { format: 'esm', @@ -22,6 +31,11 @@ const commonConfig: UserConfig = { codegen: { removeWhitespace: false }, }, define: { DEBUG: 'false' }, + plugins, + inputOptions: { + // For `replaceAssertsPlugin` + experimental: { nativeMagicString: true }, + }, }; // Only generate `.d.ts` file for main export, not for CLI @@ -44,7 +58,70 @@ const debugConfigs = configs.map((config) => ({ ...config, outDir: 'debug', define: { DEBUG: 'true' }, + plugins: plugins.filter((plugin) => plugin !== replaceAssertsPlugin), })); configs.push(...debugConfigs); export default configs; + +/** + * Create a plugin to remove imports of `assert*` functions from `src-js/utils/asserts.ts`, + * and replace those imports with empty function declarations. + * + * ```ts + * // Original code + * import { assertIs, assertIsNonNull } from '../utils/asserts.ts'; + * + * // After transform + * function assertIs() {} + * function assertIsNonNull() {} + * ``` + * + * Minifier can already remove all calls to these functions as dead code, but only if the functions are defined + * in the same file as the call sites. + * + * Problem is that `asserts.ts` is imported by files which end up in all output chunks. + * So without this transform, TSDown creates a shared chunk for `asserts.ts`. Minifier works chunk-by-chunk, + * so can't see that these functions are no-ops, and doesn't remove the function calls. + * + * Inlining these functions in each file solves the problem, and minifier removes all trace of them. + * + * @returns Plugin + */ +function createReplaceAssertsPlugin(): Plugin { + return { + name: 'replace-asserts', + transform: { + // Only process TS files in `src-js` directory + filter: { id: /\/src-js\/.+\.ts$/ }, + + async handler(code, id, meta) { + const magicString = meta.magicString!; + const program = this.parse(code, { lang: 'ts' }); + + stmts: for (const stmt of program.body) { + if (stmt.type !== 'ImportDeclaration') continue; + + // Check if import is from `utils/asserts.ts`. + // `endsWith` check is just a shortcut to avoid resolving the specifier to a full path for most imports. + const source = stmt.source.value; + if (!source.endsWith('/asserts.ts') && !source.endsWith('/asserts.js')) continue; + // oxlint-disable-next-line no-await-in-loop + const importedId = await this.resolve(source, id); + if (importedId === null || importedId.id !== ASSERTS_PATH) continue; + + // Replace `import` statement with empty function declarations + let functionsCode = ''; + for (const specifier of stmt.specifiers) { + // Skip this `import` statement if it's a default or namespace import - can't handle those + if (specifier.type !== 'ImportSpecifier') continue stmts; + functionsCode += `function ${specifier.local.name}() {}\n`; + } + magicString.overwrite(stmt.start, stmt.end, functionsCode); + } + + return { code: magicString }; + }, + }, + }; +} diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 4b1c7af8dcde9..fd2202a10e98d 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -18,6 +18,9 @@ catalogs: publint: specifier: 0.3.15 version: 0.3.15 + rolldown: + specifier: 1.0.0-beta.51 + version: 1.0.0-beta.51 tsdown: specifier: 0.16.6 version: 0.16.6 @@ -110,6 +113,9 @@ importers: jiti: specifier: ^2.6.0 version: 2.6.1 + rolldown: + specifier: 'catalog:' + version: 1.0.0-beta.51 tsdown: specifier: 'catalog:' version: 0.16.6(@arethetypeswrong/core@0.18.2)(publint@0.3.15)(typescript@5.9.3) diff --git a/pnpm-workspace.yaml b/pnpm-workspace.yaml index 923ecc4f69e87..3f7de3a076521 100644 --- a/pnpm-workspace.yaml +++ b/pnpm-workspace.yaml @@ -13,6 +13,7 @@ catalog: '@napi-rs/wasm-runtime': 1.0.7 '@types/node': ^24.0.0 publint: 0.3.15 + rolldown: 1.0.0-beta.51 tsdown: 0.16.6 typescript: 5.9.3 vitest: 4.0.10