From 545cb22aa8267f77da43a9eedeb4296452d60ba9 Mon Sep 17 00:00:00 2001 From: Nicolas DUBIEN Date: Fri, 27 Mar 2026 14:50:13 +0100 Subject: [PATCH] =?UTF-8?q?=E2=9C=A8(packaged)=20Replace=20keepNodeModules?= =?UTF-8?q?=20with=20flexible=20keep=20patterns=20(#6713)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Description Replaces the `keepNodeModules` boolean option with a more flexible `keep` array option that accepts glob patterns. This allows users to preserve any files or directories matching specified patterns, not just `node_modules`. ### Changes - **API**: Changed `removeNonPublishedFiles()` option from `keepNodeModules: boolean` to `keep: string[]` (glob patterns) - **CLI**: Updated `--keep-node-modules` flag to `--keep ` (can be specified multiple times) - **Implementation**: Updated pattern matching logic to use glob-based filtering instead of a hardcoded path check - **Tests**: Refactored test cases to cover glob pattern matching with multiple scenarios (node_modules, tsconfig files, multiple patterns) - **Documentation**: Updated README and type definitions to reflect the new API ### Motivation The new approach is more flexible and allows users to preserve any files or directories matching glob patterns (e.g., `tsconfig*`, `src`, etc.) rather than being limited to just `node_modules`. ## Checklist - [x] I have a full understanding of every line in this PR - [ ] I flagged the impact of my change (minor / patch / major) either by running `pnpm run bump` or by following the instructions from the changeset bot - [x] I kept this PR focused on a single concern and did not bundle unrelated changes - [x] I followed the [gitmoji](https://gitmoji.dev/) specification for the name of the PR - [x] I added relevant tests and they would have failed without my PR https://claude.ai/code/session_01Sf6CnWKuP1jHUcFiLJ97u3 --------- Co-authored-by: Claude --- .changeset/sixty-bobcats-jump.md | 5 ++ .github/workflows/build-status.yml | 8 +--- packages/packaged/README.md | 4 +- packages/packaged/bin/packaged.js | 23 ++++++++-- packages/packaged/src/packaged.ts | 14 ++++-- packages/packaged/test-types/main.mts | 2 +- packages/packaged/test/packaged.spec.ts | 61 +++++++++++++++---------- 7 files changed, 75 insertions(+), 42 deletions(-) create mode 100644 .changeset/sixty-bobcats-jump.md diff --git a/.changeset/sixty-bobcats-jump.md b/.changeset/sixty-bobcats-jump.md new file mode 100644 index 00000000..5ddb1637 --- /dev/null +++ b/.changeset/sixty-bobcats-jump.md @@ -0,0 +1,5 @@ +--- +"@fast-check/packaged": minor +--- + +✨(packaged) Replace keepNodeModules with flexible keep patterns diff --git a/.github/workflows/build-status.yml b/.github/workflows/build-status.yml index ec91a897..ef9ef1b3 100644 --- a/.github/workflows/build-status.yml +++ b/.github/workflows/build-status.yml @@ -406,9 +406,7 @@ jobs: - name: Unpack production packages run: node --run unpack:all - name: Alter internals to behave as if published - run: pnpm --filter {./packages/**} --parallel exec $(pnpm bin)/packaged --keep-node-modules - - name: Retrieve potentially dropped test-bundle - run: pnpm --filter {./packages/**} --parallel -c --workspace-concurrency=1 exec "git restore -s@ -SW -- test-bundle || true" + run: pnpm --filter {./packages/**} --parallel exec $(pnpm bin)/packaged --keep node_modules --keep test-bundle - name: Check publication lint run: node --run publint:all - name: Check bundles @@ -471,9 +469,7 @@ jobs: - name: Unpack production packages run: node --run unpack:all - name: Alter internals to behave as if published - run: pnpm --filter {./packages/**} --parallel exec $(pnpm bin)/packaged --keep-node-modules - - name: Retrieve dropped test-types - run: pnpm --filter {./packages/**} --parallel -c --workspace-concurrency=1 exec "git restore -s@ -SW -- test-types" + run: pnpm --filter {./packages/**} --parallel exec $(pnpm bin)/packaged --keep node_modules --keep test-types - name: Switch folder to CommonJS run: pnpm --filter {./packages/**} --parallel -c exec "cd test-types && ../../../.github/scripts/rename.sh ts cts" - name: Check in CommonJS mode diff --git a/packages/packaged/README.md b/packages/packaged/README.md index fb9690f5..ec062cce 100644 --- a/packages/packaged/README.md +++ b/packages/packaged/README.md @@ -32,7 +32,7 @@ yarn dlx -p @fast-check/packaged packaged It also comes with some extra flags: - `--dry-run`: do not drop any file or directory from the file system and only print what would have been removed -- `--keep-node-modules`: keep the `node_modules` directory if any at the root of the directory +- `--keep `: keep a root-level file or directory matching the exact name (not a glob pattern, can be specified multiple times, e.g. `--keep node_modules --keep src`) ## Simple API @@ -48,7 +48,7 @@ const publishedFilesRoot = await computePublishedFiles('.'); const publishedFilesSubDirectory = await computePublishedFiles('./sub-directory'); // Run the deletion of unwanted files -const { kept, removed } = await removeNonPublishedFiles('.', { dryRun: false, keepNodeModules: false }); +const { kept, removed } = await removeNonPublishedFiles('.', { dryRun: false, keep: [] }); // kept and removed are arrays of strings // they may contain files or directories ``` diff --git a/packages/packaged/bin/packaged.js b/packages/packaged/bin/packaged.js index 1ffc5255..99a0ea59 100755 --- a/packages/packaged/bin/packaged.js +++ b/packages/packaged/bin/packaged.js @@ -11,13 +11,26 @@ function run(args) { console.log(' if published to npm registry'); console.log('- packaged --dry-run'); console.log(' No removal, just printing'); - console.log('- packaged --keep-node-modules'); - console.log(' Keep root level node_modules if any'); + console.log('- packaged --keep '); + console.log(' Keep files/directories matching the glob pattern (can be specified multiple times)'); return; } - const dryRun = args.includes('--dry-run'); - const keepNodeModules = args.includes('--keep-node-modules'); - removeNonPublishedFiles('.', { dryRun, keepNodeModules }).then( + let dryRun = false; + const keep = []; + for (let i = 0; i < args.length; ++i) { + if (args[i] === '--keep') { + if (i + 1 >= args.length) { + throw new Error('Missing value for --keep'); + } + keep.push(args[i + 1]); + ++i; + } else if (args[i] === '--dry-run') { + dryRun = true; + } else { + throw new Error(`Unknown flag: ${args[i]}`); + } + } + removeNonPublishedFiles('.', { dryRun, keep }).then( (out) => { if (dryRun) { console.log('Those files would have been kept:'); diff --git a/packages/packaged/src/packaged.ts b/packages/packaged/src/packaged.ts index ce66f16b..1128cc1c 100644 --- a/packages/packaged/src/packaged.ts +++ b/packages/packaged/src/packaged.ts @@ -52,17 +52,21 @@ async function traverseAndRemoveNonPublishedFiles( currentPath: string, out: { kept: string[]; removed: string[] }, opts: { - rootNodeModulesPath: string | undefined; dryRun: boolean; publishedDirectories: Set; publishedFiles: Set; + packageRoot: string; + keepPatterns: string[]; }, ): Promise { const awaitedTasks: Promise[] = []; const content = await fs.readdir(currentPath); for (const itemName of content) { const itemPath = path.join(currentPath, itemName); - if (itemPath === opts.rootNodeModulesPath) { + if ( + opts.keepPatterns.length !== 0 && + opts.keepPatterns.includes(path.normalize(path.relative(opts.packageRoot, itemPath))) + ) { out.kept.push(itemPath); } else if (opts.publishedDirectories.has(itemPath)) { out.kept.push(itemPath); @@ -85,7 +89,7 @@ async function traverseAndRemoveNonPublishedFiles( */ export async function removeNonPublishedFiles( packageRoot: string, - opts: { dryRun?: boolean; keepNodeModules?: boolean } = {}, + opts: { dryRun?: boolean; keep?: string[] } = {}, ): Promise<{ kept: string[]; removed: string[] }> { const publishedFiles = await computePublishedFiles(packageRoot); @@ -94,11 +98,13 @@ export async function removeNonPublishedFiles( const normalizedPublishedFiles = publishedFiles.map((filename) => path.join(normalizedPackageRoot, filename)); const normalizedPublishedFilesSet = new Set(normalizedPublishedFiles); const normalizedPublishedDirectoriesSet = buildNormalizedPublishedDirectoriesSet(normalizedPublishedFiles); + const keepPatterns = opts.keep ?? []; const traverseOpts = { - rootNodeModulesPath: opts.keepNodeModules ? path.join(normalizedPackageRoot, 'node_modules') : undefined, dryRun: !!opts.dryRun, publishedDirectories: normalizedPublishedDirectoriesSet, publishedFiles: normalizedPublishedFilesSet, + packageRoot: normalizedPackageRoot, + keepPatterns, }; await traverseAndRemoveNonPublishedFiles(normalizedPackageRoot, out, traverseOpts); return out; diff --git a/packages/packaged/test-types/main.mts b/packages/packaged/test-types/main.mts index 471b9439..be931888 100644 --- a/packages/packaged/test-types/main.mts +++ b/packages/packaged/test-types/main.mts @@ -3,6 +3,6 @@ import { computePublishedFiles, removeNonPublishedFiles } from '@fast-check/pack computePublishedFiles('.').then((_publishedFilesRoot) => { // not implemented }); -removeNonPublishedFiles('.', { dryRun: false, keepNodeModules: false }).then(({ kept: _kept, removed: _removed }) => { +removeNonPublishedFiles('.', { dryRun: false, keep: [] }).then(({ kept: _kept, removed: _removed }) => { // not implemented }); diff --git a/packages/packaged/test/packaged.spec.ts b/packages/packaged/test/packaged.spec.ts index 25bd0db6..0fa230ab 100644 --- a/packages/packaged/test/packaged.spec.ts +++ b/packages/packaged/test/packaged.spec.ts @@ -11,16 +11,22 @@ afterAll(async () => { describe('removeNonPublishedFiles', () => { it.each` - name | dryRun | keepNodeModules | pathStyle - ${'only keep published files by default'} | ${false} | ${false} | ${'absolute'} - ${'only keep published files and node_modules at root when requested'} | ${false} | ${true} | ${'absolute'} - ${'not clean anything in dryRun mode even without keepNodeModules'} | ${true} | ${false} | ${'absolute'} - ${'not clean anything in dryRun mode even with keepNodeModules'} | ${true} | ${true} | ${'absolute'} - ${'handle relative paths such as .'} | ${false} | ${false} | ${'.'} - ${'handle relative paths such as ./package-name'} | ${false} | ${false} | ${'./package-name'} - ${'handle relative paths such as ./a/package-name'} | ${false} | ${false} | ${'./a/package-name'} - ${'handle relative paths such as ./a/../a/package-name/'} | ${false} | ${false} | ${'./a/../a/package-name/'} - `('should $name', async ({ dryRun, keepNodeModules, pathStyle }) => { + name | dryRun | keep | pathStyle + ${'only keep published files by default'} | ${false} | ${[]} | ${'absolute'} + ${'only keep published files and node_modules at root when requested'} | ${false} | ${['node_modules']} | ${'absolute'} + ${'only keep published files and src at root when requested'} | ${false} | ${['src']} | ${'absolute'} + ${'only keep published files and {src,node_modules} at root when requested'} | ${false} | ${['src', 'node_modules']} | ${'absolute'} + ${'only keep published files and nothing else if nothing match thing from the keep array'} | ${false} | ${['node*', 'node_mod']} | ${'absolute'} + ${'only keep published files and nothing else if requested non-root entries'} | ${false} | ${['src/node_modules']} | ${'absolute'} + ${'not clean anything in dryRun mode even with keep='} | ${true} | ${[]} | ${'absolute'} + ${'not clean anything in dryRun mode even with keep=node_modules'} | ${true} | ${['node_modules']} | ${'absolute'} + ${'not clean anything in dryRun mode even with keep=src'} | ${false} | ${['src']} | ${'absolute'} + ${'not clean anything in dryRun mode even with keep=src,node_modules'} | ${false} | ${['src', 'node_modules']} | ${'absolute'} + ${'handle relative paths such as .'} | ${false} | ${[]} | ${'.'} + ${'handle relative paths such as ./package-name'} | ${false} | ${[]} | ${'./package-name'} + ${'handle relative paths such as ./a/package-name'} | ${false} | ${[]} | ${'./a/package-name'} + ${'handle relative paths such as ./a/../a/package-name/'} | ${false} | ${[]} | ${'./a/../a/package-name/'} + `('should $name', async ({ dryRun, keep, pathStyle }) => { await runPackageTest(async (fileSystem) => { // Arrange const packageJsonContent = { @@ -63,31 +69,38 @@ describe('removeNonPublishedFiles', () => { default: throw new Error(`Unsupported style ${pathStyle}`); } - const { kept, removed } = await removeNonPublishedFiles(requestedPath, { dryRun, keepNodeModules }); + const { kept, removed } = await removeNonPublishedFiles(requestedPath, { dryRun, keep }); // Assert - // Returns arrays having the expected sizes - if (!keepNodeModules) { + const keepsRootNodeModules = keep.includes('node_modules'); + const keepsRootSrc = keep.includes('src'); + if (keepsRootSrc && keepsRootNodeModules) { + expect(kept).toHaveLength(5); // package.json, lib/main.js, lib, src, node_modules + expect(removed).toHaveLength(1); // test + } else if (keepsRootSrc) { + expect(kept).toHaveLength(4); // package.json, lib/main.js, lib, src/* + expect(removed).toHaveLength(2); // node_modules, test + } else if (keepsRootNodeModules) { + expect(kept).toHaveLength(4); // package.json, lib/main.js, lib, node_modules/* + expect(removed).toHaveLength(2); // src, test + } else { expect(kept).toHaveLength(3); // package.json, lib/main.js, lib expect(removed).toHaveLength(3); // src, test, node_modules - } else { - expect(kept).toHaveLength(4); // package.json, lib/main.js, lib, node_modules/* - expect(removed).toHaveLength(2); // src, test } // Remove unpublished files and keep published ones expect(await fileSystem.exists(['package.json'])).toBe(true); expect(await fileSystem.exists(['lib', 'main.js'])).toBe(true); - expect(await fileSystem.exists(['src', 'main.js'])).toBe(dryRun); - expect(await fileSystem.exists(['src', 'node_modules', 'wtf', 'main.js'])).toBe(dryRun); + expect(await fileSystem.exists(['src', 'main.js'])).toBe(dryRun || keepsRootSrc); + expect(await fileSystem.exists(['src', 'node_modules', 'wtf', 'main.js'])).toBe(dryRun || keepsRootSrc); expect(await fileSystem.exists(['test', 'main.js'])).toBe(dryRun); - expect(await fileSystem.exists(['node_modules', 'dep-a', 'main.js'])).toBe(dryRun || keepNodeModules); + expect(await fileSystem.exists(['node_modules', 'dep-a', 'main.js'])).toBe(dryRun || keepsRootNodeModules); // Remove empty folders - expect(await fileSystem.exists(['src'])).toBe(dryRun); - expect(await fileSystem.exists(['src', 'node_modules'])).toBe(dryRun); - expect(await fileSystem.exists(['src', 'node_modules', 'wtf'])).toBe(dryRun); + expect(await fileSystem.exists(['src'])).toBe(dryRun || keepsRootSrc); + expect(await fileSystem.exists(['src', 'node_modules'])).toBe(dryRun || keepsRootSrc); + expect(await fileSystem.exists(['src', 'node_modules', 'wtf'])).toBe(dryRun || keepsRootSrc); expect(await fileSystem.exists(['test'])).toBe(dryRun); - expect(await fileSystem.exists(['node_modules'])).toBe(dryRun || keepNodeModules); - expect(await fileSystem.exists(['node_modules', 'dep-a'])).toBe(dryRun || keepNodeModules); + expect(await fileSystem.exists(['node_modules'])).toBe(dryRun || keepsRootNodeModules); + expect(await fileSystem.exists(['node_modules', 'dep-a'])).toBe(dryRun || keepsRootNodeModules); }); }); -- 2.51.2