diff --git a/.changeset/purple-teeth-look.md b/.changeset/purple-teeth-look.md new file mode 100644 index 000000000..740e97cfd --- /dev/null +++ b/.changeset/purple-teeth-look.md @@ -0,0 +1,9 @@ +--- +'skuba': minor +--- + +lint: Remove `unbundle` from tsdown configs + +When skuba first migrated packages to tsdown, it set `unbundle: true` with a `// TODO: determine if your project can be bundled` comment as a prompt for consumers to revisit the setting. This patch now automates that follow-up. + +The `unbundle` field and its TODO comment will be removed from your `tsdown.config` file if your package does not appear to ship translations (`.vocab` files) or CSS (`.css` files). diff --git a/src/cli/format/__snapshots__/format.int.test.ts.snap b/src/cli/format/__snapshots__/format.int.test.ts.snap index ddb75da86..e19bb6bad 100644 --- a/src/cli/format/__snapshots__/format.int.test.ts.snap +++ b/src/cli/format/__snapshots__/format.int.test.ts.snap @@ -70,6 +70,8 @@ Patch skipped: Migrate tsdown config to 0.21 - no tsdown.config files found upgr Patch skipped: Replace pnpm install --prod with pnpm prune --prod in Dockerfiles - no dockerfiles found upgrade-skuba +Patch skipped: Unbundle tsdown.config files to avoid shipping translations and CSS in packages that use tsdown for non-translation purposes - no tsdown.config files found upgrade-skuba + skuba update complete. upgrade-skuba Processed skuba lints in s. @@ -191,6 +193,8 @@ Patch skipped: Migrate tsdown config to 0.21 - no tsdown.config files found upgr Patch skipped: Replace pnpm install --prod with pnpm prune --prod in Dockerfiles - no dockerfiles found upgrade-skuba +Patch skipped: Unbundle tsdown.config files to avoid shipping translations and CSS in packages that use tsdown for non-translation purposes - no tsdown.config files found upgrade-skuba + skuba update complete. upgrade-skuba Processed skuba lints in s. @@ -305,6 +309,8 @@ Patch skipped: Migrate tsdown config to 0.21 - no tsdown.config files found upgr Patch skipped: Replace pnpm install --prod with pnpm prune --prod in Dockerfiles - no dockerfiles found upgrade-skuba +Patch skipped: Unbundle tsdown.config files to avoid shipping translations and CSS in packages that use tsdown for non-translation purposes - no tsdown.config files found upgrade-skuba + skuba update complete. upgrade-skuba Processed skuba lints in s. @@ -391,6 +397,8 @@ Patch skipped: Migrate tsdown config to 0.21 - no tsdown.config files found upgr Patch skipped: Replace pnpm install --prod with pnpm prune --prod in Dockerfiles - no dockerfiles found upgrade-skuba +Patch skipped: Unbundle tsdown.config files to avoid shipping translations and CSS in packages that use tsdown for non-translation purposes - no tsdown.config files found upgrade-skuba + skuba update complete. upgrade-skuba Processed skuba lints in s. diff --git a/src/cli/lint/internalLints/upgrade/patches/15.2.0/migrateTsdown.ts b/src/cli/lint/internalLints/upgrade/patches/15.2.0/migrateTsdown.ts index d5eb863a5..833a6185c 100644 --- a/src/cli/lint/internalLints/upgrade/patches/15.2.0/migrateTsdown.ts +++ b/src/cli/lint/internalLints/upgrade/patches/15.2.0/migrateTsdown.ts @@ -25,7 +25,7 @@ interface FoundEntry { pairNode: SgNode; } -const removeNodeWithComma = (node: SgNode): Edit[] => { +export const removeNodeWithComma = (node: SgNode): Edit[] => { const edits: Edit[] = [node.replace('')]; const maybeCommaAfter = node.next(); if (maybeCommaAfter?.text().trim() === ',') { diff --git a/src/cli/lint/internalLints/upgrade/patches/15.3.1/index.ts b/src/cli/lint/internalLints/upgrade/patches/15.3.1/index.ts index 6ee33128c..a0d53c1f9 100644 --- a/src/cli/lint/internalLints/upgrade/patches/15.3.1/index.ts +++ b/src/cli/lint/internalLints/upgrade/patches/15.3.1/index.ts @@ -1,6 +1,7 @@ import type { Patches } from '../../index.js'; import { tryPatchDockerfilePruneProd } from './patchDockerfilePruneProd.js'; +import { tryPatchTsdownUnbundle } from './patchTsdownUnbundle.js'; export const patches: Patches = [ { @@ -8,4 +9,9 @@ export const patches: Patches = [ description: 'Replace pnpm install --prod with pnpm prune --prod in Dockerfiles', }, + { + apply: tryPatchTsdownUnbundle, + description: + 'Unbundle tsdown.config files to avoid shipping translations and CSS in packages that use tsdown for non-translation purposes', + }, ]; diff --git a/src/cli/lint/internalLints/upgrade/patches/15.3.1/patchTsdownUnbundle.test.ts b/src/cli/lint/internalLints/upgrade/patches/15.3.1/patchTsdownUnbundle.test.ts new file mode 100644 index 000000000..f56c9a4d9 --- /dev/null +++ b/src/cli/lint/internalLints/upgrade/patches/15.3.1/patchTsdownUnbundle.test.ts @@ -0,0 +1,480 @@ +import memfs, { vol } from 'memfs'; +import { beforeEach, describe, expect, it, vi } from 'vitest'; + +import { log } from '../../../../../../utils/logging.js'; +import { configForPackageManager } from '../../../../../../utils/packageManager.js'; +import type { PatchConfig, PatchReturnType } from '../../index.js'; + +import { + patchTsdownUnbundle, + tryPatchTsdownUnbundle, +} from './patchTsdownUnbundle.js'; + +vi.mock('../../../../../../utils/exec.js', () => ({ + createExec: () => vi.fn(), +})); + +vi.mock('fs-extra', () => ({ + default: memfs.fs, + ...memfs.fs, +})); + +vi.mock('fast-glob', () => ({ + default: async (pat: any, opts: any) => { + const actualFastGlob = + await vi.importActual('fast-glob'); + return actualFastGlob.glob(pat, { ...opts, fs: memfs }); + }, +})); + +vi.mock('../../../../../../utils/logging.js', () => ({ + log: { + warn: vi.fn(), + subtle: vi.fn(), + }, +})); + +const volToJson = () => vol.toJSON(process.cwd(), undefined, true); + +beforeEach(() => { + vol.reset(); + vi.clearAllMocks(); +}); + +const baseArgs: PatchConfig = { + manifest: { + packageJson: { + name: 'test', + version: '1.0.0', + readme: 'README.md', + _id: 'test', + }, + path: 'package.json', + }, + packageManager: configForPackageManager('pnpm'), + mode: 'format', +}; + +describe('patchTsdownUnbundle', () => { + it('should skip if no tsdown configs are found', async () => { + vol.fromJSON({ + 'index.ts': '', + }); + + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'lint', + }), + ).resolves.toEqual({ + result: 'skip', + reason: 'no tsdown.config files found', + } satisfies PatchReturnType); + }); + + it('should skip if no unbundle field is found', async () => { + vol.fromJSON({ + 'tsdown.config.ts': `import { defineConfig } from 'tsdown'; +export default defineConfig({ + failOnWarn: true, + entry: ['src/index.ts'], + format: ['cjs', 'esm'], + outDir: 'lib', + dts: true, + publint: true, + attw: true, +}); +`, + }); + + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'lint', + }), + ).resolves.toEqual({ + result: 'skip', + reason: 'no tsdown.config fields to migrate', + } satisfies PatchReturnType); + }); + + it('should skip if unbundle is true but TODO comment is absent', async () => { + vol.fromJSON({ + 'tsdown.config.ts': `import { defineConfig } from 'tsdown'; +export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, + publint: true, +}); +`, + }); + + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'lint', + }), + ).resolves.toEqual({ + result: 'skip', + reason: 'no tsdown.config fields to migrate', + } satisfies PatchReturnType); + }); + + it('should skip if .vocab files are present (potentially ships translations)', async () => { + vol.fromJSON({ + 'tsdown.config.ts': `import { defineConfig } from 'tsdown'; +export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, +}); +`, + 'src/.vocab/translations.json': '{}', + }); + + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'lint', + }), + ).resolves.toEqual({ + result: 'skip', + reason: 'potentially ships translations', + } satisfies PatchReturnType); + }); + + it('should skip if .css files are present (potentially ships css)', async () => { + vol.fromJSON({ + 'tsdown.config.ts': `import { defineConfig } from 'tsdown'; +export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, +}); +`, + 'src/styles/button.css': '.button {}', + }); + + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'lint', + }), + ).resolves.toEqual({ + result: 'skip', + reason: 'potentially ships css', + } satisfies PatchReturnType); + }); + + it('should remove unbundle from tsdown.config.mts', async () => { + vol.fromJSON({ + 'tsdown.config.mts': `import { defineConfig } from 'tsdown'; +export default defineConfig({ + deps: { + onlyBundle: false, + }, + failOnWarn: true, + entry: ['src/index.ts'], + format: ['cjs', 'esm'], + outDir: 'lib', + dts: true, + checks: { + legacyCjs: false, + }, + publint: true, + attw: true, + unbundle: true, // TODO: determine if your project can be bundled + // @smithy devDeps are intentionally inlined + exports: { devExports: '@seek/indie-rita-ora/source' }, +}); +`, + }); + + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'format', + }), + ).resolves.toEqual({ + result: 'apply', + } satisfies PatchReturnType); + + expect(volToJson()).toMatchInlineSnapshot(` + { + "tsdown.config.mts": "import { defineConfig } from 'tsdown'; + export default defineConfig({ + deps: { + onlyBundle: false, + }, + failOnWarn: true, + entry: ['src/index.ts'], + format: ['cjs', 'esm'], + outDir: 'lib', + dts: true, + checks: { + legacyCjs: false, + }, + publint: true, + attw: true, + + // @smithy devDeps are intentionally inlined + exports: { devExports: '@seek/indie-rita-ora/source' }, + }); + ", + } + `); + }); + + it('should process multiple tsdown config files and patch all that qualify', async () => { + vol.fromJSON({ + 'packages/pkg-a/tsdown.config.ts': `import { defineConfig } from 'tsdown'; +export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, +}); +`, + 'packages/pkg-b/tsdown.config.mts': `import { defineConfig } from 'tsdown'; +export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, +}); +`, + 'packages/pkg-c/tsdown.config.ts': `import { defineConfig } from 'tsdown'; +export default defineConfig({ + entry: ['src/index.ts'], + publint: true, +}); +`, + }); + + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'format', + }), + ).resolves.toEqual({ + result: 'apply', + } satisfies PatchReturnType); + + const result = volToJson(); + + expect(result['packages/pkg-a/tsdown.config.ts']).not.toContain('unbundle'); + expect(result['packages/pkg-b/tsdown.config.mts']).not.toContain( + 'unbundle', + ); + expect(result['packages/pkg-c/tsdown.config.ts']).toMatchInlineSnapshot(` + "import { defineConfig } from 'tsdown'; + export default defineConfig({ + entry: ['src/index.ts'], + publint: true, + }); + " + `); + }); + + it('should report lint result without modifying file', async () => { + const originalContent = `import { defineConfig } from 'tsdown'; +export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, +}); +`; + + vol.fromJSON({ + 'tsdown.config.ts': originalContent, + }); + + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'lint', + }), + ).resolves.toEqual({ + result: 'apply', + } satisfies PatchReturnType); + + expect(volToJson()).toEqual({ + 'tsdown.config.ts': originalContent, + }); + }); + it('should patch a package that has unbundle even when another package has .vocab files', async () => { + vol.fromJSON({ + 'packages/pkg-a/tsdown.config.ts': `import { defineConfig } from 'tsdown'; + export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, + }); + `, + 'packages/pkg-b/tsdown.config.ts': `import { defineConfig } from 'tsdown'; + export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, + }); + `, + 'packages/pkg-b/src/.vocab/translations.json': '{}', + }); + + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'format', + }), + ).resolves.toEqual({ + result: 'apply', + } satisfies PatchReturnType); + + const result = volToJson(); + + // pkg-a has no .vocab files so should be patched + expect(result['packages/pkg-a/tsdown.config.ts']).not.toContain('unbundle'); + + // pkg-b ships translations so should be left untouched + expect(result['packages/pkg-b/tsdown.config.ts']).toContain('unbundle'); + }); + + it('should patch a package that has unbundle even when another package has .css files', async () => { + vol.fromJSON({ + 'packages/pkg-a/tsdown.config.ts': `import { defineConfig } from 'tsdown'; + export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, + }); + `, + 'packages/pkg-b/tsdown.config.ts': `import { defineConfig } from 'tsdown'; + export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, + }); + `, + 'packages/pkg-b/src/styles/button.css': '.button {}', + }); + + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'format', + }), + ).resolves.toEqual({ + result: 'apply', + } satisfies PatchReturnType); + + const result = volToJson(); + + // pkg-a has no .css files so should be patched + expect(result['packages/pkg-a/tsdown.config.ts']).not.toContain('unbundle'); + + // pkg-b ships css so should be left untouched + expect(result['packages/pkg-b/tsdown.config.ts']).toContain('unbundle'); + }); + + it('should skip with translations reason when all packages with unbundle ship translations', async () => { + vol.fromJSON({ + 'packages/pkg-a/tsdown.config.ts': `import { defineConfig } from 'tsdown'; + export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, + }); + `, + 'packages/pkg-b/tsdown.config.ts': `import { defineConfig } from 'tsdown'; + export default defineConfig({ + entry: ['src/index.ts'], + publint: true, + }); + `, + 'packages/pkg-a/src/.vocab/translations.json': '{}', + }); + + // pkg-a would be migrated but ships translations — skipped + // pkg-b has no unbundle field — nothing to migrate + // net result: nothing was patched + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'lint', + }), + ).resolves.toEqual({ + result: 'skip', + reason: 'no tsdown.config fields to migrate', + } satisfies PatchReturnType); + }); + + it('should prefer translations skip reason over css skip reason', async () => { + vol.fromJSON({ + 'tsdown.config.ts': `import { defineConfig } from 'tsdown'; + export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, + }); + `, + 'src/.vocab/translations.json': '{}', + 'src/styles/button.css': '.button {}', + }); + + await expect( + patchTsdownUnbundle({ + ...baseArgs, + mode: 'lint', + }), + ).resolves.toEqual({ + result: 'skip', + reason: 'potentially ships translations', + } satisfies PatchReturnType); + }); +}); + +describe('tryPatchTsdownUnbundle', () => { + it('should return the result of patchTsdownUnbundle on success', async () => { + vol.fromJSON({ + 'tsdown.config.ts': `import { defineConfig } from 'tsdown'; +export default defineConfig({ + entry: ['src/index.ts'], + unbundle: true, // TODO: determine if your project can be bundled + publint: true, +}); +`, + }); + + await expect( + tryPatchTsdownUnbundle({ + ...baseArgs, + mode: 'lint', + }), + ).resolves.toEqual({ + result: 'apply', + } satisfies PatchReturnType); + }); + + it('should return skip and log a warning if patchTsdownUnbundle throws', async () => { + vol.fromJSON({ + 'tsdown.config.ts': 'any content', + }); + + vi.spyOn(memfs.fs.promises, 'readFile').mockRejectedValueOnce( + new Error('disk read failure'), + ); + + await expect( + tryPatchTsdownUnbundle({ + ...baseArgs, + mode: 'format', + }), + ).resolves.toEqual({ + result: 'skip', + reason: 'due to an error', + } satisfies PatchReturnType); + + expect(log.warn).toHaveBeenCalledWith( + 'Failed to apply tsdown unbundle patch.', + ); + expect(log.subtle).toHaveBeenCalledOnce(); + }); +}); diff --git a/src/cli/lint/internalLints/upgrade/patches/15.3.1/patchTsdownUnbundle.ts b/src/cli/lint/internalLints/upgrade/patches/15.3.1/patchTsdownUnbundle.ts new file mode 100644 index 000000000..5009d68fd --- /dev/null +++ b/src/cli/lint/internalLints/upgrade/patches/15.3.1/patchTsdownUnbundle.ts @@ -0,0 +1,164 @@ +import path from 'path'; +import { inspect } from 'util'; + +import { type Edit, type SgNode, parseAsync } from '@ast-grep/napi'; +import fg from 'fast-glob'; +import fs from 'fs-extra'; + +import { log } from '../../../../../../utils/logging.js'; +import type { PatchFunction, PatchReturnType } from '../../index.js'; +import { removeNodeWithComma } from '../15.2.0/migrateTsdown.js'; + +const removeUnbundleFieldEdits = (node: SgNode): Edit[] => { + const unbundlePair = node.find({ + rule: { + kind: 'pair', + has: { + field: 'key', + regex: '^unbundle$', + }, + }, + }); + + if (!unbundlePair) { + return []; + } + + const nextNode = unbundlePair.next(); + const maybeComment = nextNode?.kind() === ',' ? nextNode.next() : nextNode; + + if ( + maybeComment?.kind() !== 'comment' || + maybeComment.text() !== '// TODO: determine if your project can be bundled' + ) { + return []; + } + + const edits = removeNodeWithComma(unbundlePair); + if (edits.length === 0) { + return []; + } + + const lastEdit = edits[edits.length - 1]; + return [ + ...edits.slice(0, -1), + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + { ...lastEdit!, endPos: maybeComment.range().end.index }, + ]; +}; + +const packageShipsTranslations = async (packageDir: string) => { + const translations = await fg(['**/.vocab/**'], { + cwd: packageDir, + ignore: ['**/.git', '**/node_modules'], + }); + return translations.length > 0; +}; + +const packageShipsCss = async (packageDir: string) => { + const cssFiles = await fg(['**/*.css'], { + cwd: packageDir, + ignore: ['**/.git', '**/node_modules'], + }); + return cssFiles.length > 0; +}; + +type SkipReason = 'potentially ships translations' | 'potentially ships css'; + +type Config = + | { kind: 'skipped'; file: string; reason: SkipReason } + | { kind: 'parsed'; file: string; updated: string | undefined }; + +export const patchTsdownUnbundle: PatchFunction = async ({ + mode, +}): Promise => { + const tsdownFiles = await fg(['**/tsdown.config.{ts,js,mjs,mts,cts}'], { + ignore: ['**/.git', '**/node_modules'], + }); + + if (tsdownFiles.length === 0) { + return { + result: 'skip', + reason: 'no tsdown.config files found', + }; + } + + const tsdownConfigs = await Promise.all( + tsdownFiles.map(async (file): Promise => { + const packageDir = path.dirname(file); + const [content, shipsTranslations, shipsCss] = await Promise.all([ + fs.promises.readFile(file, 'utf8'), + packageShipsTranslations(packageDir), + packageShipsCss(packageDir), + ]); + + if (shipsTranslations) { + return { + kind: 'skipped', + file, + reason: 'potentially ships translations', + }; + } + + if (shipsCss) { + return { kind: 'skipped', file, reason: 'potentially ships css' }; + } + + const ast = (await parseAsync('TypeScript', content)).root(); + const edits = removeUnbundleFieldEdits(ast); + const updated = edits.length ? ast.commitEdits(edits) : undefined; + return { kind: 'parsed', file, updated }; + }), + ); + + const parsedConfigs = tsdownConfigs.filter( + (entry): entry is Extract => + entry.kind === 'parsed', + ); + + if (parsedConfigs.length === 0) { + const firstSkippedEntry = tsdownConfigs.find((e) => e.kind === 'skipped'); + return { + result: 'skip', + reason: firstSkippedEntry?.reason ?? 'no tsdown.config fields to migrate', + }; + } + + const configsToUpdate = parsedConfigs.filter( + (entry): entry is { kind: 'parsed'; file: string; updated: string } => + entry.updated !== undefined, + ); + + if (configsToUpdate.length === 0) { + return { + result: 'skip', + reason: 'no tsdown.config fields to migrate', + }; + } + + if (mode === 'lint') { + return { + result: 'apply', + }; + } + + await Promise.all( + configsToUpdate.map(async ({ file, updated }) => { + await fs.promises.writeFile(file, updated, 'utf8'); + }), + ); + + return { + result: 'apply', + }; +}; + +export const tryPatchTsdownUnbundle: PatchFunction = async (config) => { + try { + return await patchTsdownUnbundle(config); + } catch (err) { + log.warn('Failed to apply tsdown unbundle patch.'); + log.subtle(inspect(err)); + return { result: 'skip', reason: 'due to an error' }; + } +};