From c31efc21cb7c2d1e002973c39d69b1ec2b5782f8 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 21:11:31 +0100 Subject: [PATCH 01/24] Copy PNPM patches to isolated directory When includePatchedDependencies is enabled, copy relevant patch files from the workspace root to the isolated directory and update the isolated package.json with patch references. Patches are filtered based on the target package's dependencies, respecting the includeDevDependencies configuration option. --- src/isolate.ts | 23 ++++ .../helpers/generate-pnpm-lockfile.ts | 90 ++++++++++++- src/lib/patches/copy-patches.ts | 126 ++++++++++++++++++ src/lib/types.ts | 4 + 4 files changed, 238 insertions(+), 5 deletions(-) create mode 100644 src/lib/patches/copy-patches.ts diff --git a/src/isolate.ts b/src/isolate.ts index 6223d94..b80b38d 100644 --- a/src/isolate.ts +++ b/src/isolate.ts @@ -22,6 +22,7 @@ import { } from "./lib/output"; import { detectPackageManager, shouldUsePnpmPack } from "./lib/package-manager"; import { getVersion } from "./lib/package-manager/helpers/infer-from-files"; +import { copyPatches } from "./lib/patches/copy-patches"; import { createPackagesRegistry, listInternalPackages } from "./lib/registry"; import type { PackageManifest } from "./lib/types"; import { @@ -211,6 +212,28 @@ export function createIsolator(config?: IsolateConfig) { config, }); + /** Copy patch files if includePatchedDependencies is enabled */ + const copiedPatches = await copyPatches({ + workspaceRootDir, + targetPackageManifest, + isolateDir, + includePatchedDependencies: config.includePatchedDependencies, + includeDevDependencies: config.includeDevDependencies, + }); + + /** Add copied patches to the isolated package.json */ + if (Object.keys(copiedPatches).length > 0) { + const manifest = await readManifest(isolateDir); + if (!manifest.pnpm) { + manifest.pnpm = {}; + } + manifest.pnpm.patchedDependencies = copiedPatches; + await writeManifest(isolateDir, manifest); + log.debug( + `Added ${Object.keys(copiedPatches).length} patches to isolated package.json` + ); + } + if (usedFallbackToNpm) { /** * When we fall back to NPM, we set the manifest package manager to the diff --git a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts index 6e317d8..58bd45b 100644 --- a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts +++ b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts @@ -13,11 +13,86 @@ import { import { pruneLockfile as pruneLockfile_v8 } from "pnpm_prune_lockfile_v8"; import { pruneLockfile as pruneLockfile_v9 } from "pnpm_prune_lockfile_v9"; import { pick } from "remeda"; +import type { Logger } from "~/lib/logger"; import { useLogger } from "~/lib/logger"; import type { PackageManifest, PackagesRegistry } from "~/lib/types"; import { getErrorMessage, isRushWorkspace } from "~/lib/utils"; import { pnpmMapImporter } from "./pnpm-map-importer"; +/** + * Extracts the package name from a package spec like "chalk@5.3.0" or + * "@firebase/app@1.2.3" + */ +function getPackageName(packageSpec: string): string { + if (packageSpec.startsWith("@")) { + // Scoped packages: @scope/package@version -> @scope/package + const parts = packageSpec.split("@"); + return `@${parts[1] ?? ""}`; + } + // Regular packages: package@version -> package + return packageSpec.split("@")[0] ?? ""; +} + +/** + * Filters patched dependencies to only include patches for packages that will + * actually be present in the isolated lockfile based on dependency type. + */ +function filterPatchedDependencies( + patchedDependencies: Record | undefined, + targetPackageManifest: PackageManifest, + includeDevDependencies: boolean, + log: Logger +): Record | undefined { + if (!patchedDependencies || typeof patchedDependencies !== "object") { + return undefined; + } + + const filteredPatches: Record = {}; + let includedCount = 0; + let excludedCount = 0; + + for (const [packageSpec, patchInfo] of Object.entries(patchedDependencies)) { + const packageName = getPackageName(packageSpec); + + // Check if it's a production dependency + if (targetPackageManifest.dependencies?.[packageName]) { + filteredPatches[packageSpec] = patchInfo; + includedCount++; + log.debug( + `Including production dependency patch in lockfile: ${packageSpec}` + ); + continue; + } + + // Check if it's a dev dependency and we should include dev dependencies + if (targetPackageManifest.devDependencies?.[packageName]) { + if (includeDevDependencies) { + filteredPatches[packageSpec] = patchInfo; + includedCount++; + log.debug(`Including dev dependency patch in lockfile: ${packageSpec}`); + } else { + excludedCount++; + log.debug( + `Excluding dev dependency patch from lockfile: ${packageSpec}` + ); + } + continue; + } + + // Package not found in dependencies or devDependencies + log.debug( + `Excluding patch from lockfile: ${packageSpec} (package "${packageName}" not found in target dependencies)` + ); + excludedCount++; + } + + log.debug( + `Filtered patched dependencies: ${includedCount} included, ${excludedCount} excluded` + ); + + return Object.keys(filteredPatches).length > 0 ? filteredPatches : undefined; +} + export async function generatePnpmLockfile({ workspaceRootDir, targetPackageDir, @@ -163,13 +238,18 @@ export async function generatePnpmLockfile({ } /** - * Don't know how to map the patched dependencies yet, so we just include - * them but I don't think it would work like this. The important thing for - * now is that they are omitted by default, because that is the most common - * use case. + * Filter patched dependencies to only include patches for packages that + * will actually be present in the isolated lockfile based on dependency + * type. We read patchedDependencies from workspace root lockfile, but + * filter based on target package dependencies. */ const patchedDependencies = includePatchedDependencies - ? lockfile.patchedDependencies + ? filterPatchedDependencies( + lockfile.patchedDependencies, + targetPackageManifest, + includeDevDependencies, + log + ) : undefined; if (useVersion9) { diff --git a/src/lib/patches/copy-patches.ts b/src/lib/patches/copy-patches.ts new file mode 100644 index 0000000..b10c73c --- /dev/null +++ b/src/lib/patches/copy-patches.ts @@ -0,0 +1,126 @@ +import fs from "fs-extra"; +import path from "node:path"; +import { useLogger } from "~/lib/logger"; +import type { PackageManifest } from "~/lib/types"; +import { getRootRelativeLogPath, readTypedJson } from "~/lib/utils"; + +/** + * Extracts the package name from a package spec like "chalk@5.3.0" or + * "@firebase/app@1.2.3" + */ +function getPackageName(packageSpec: string): string { + if (packageSpec.startsWith("@")) { + // Scoped packages: @scope/package@version -> @scope/package + const parts = packageSpec.split("@"); + return `@${parts[1] ?? ""}`; + } + // Regular packages: package@version -> package + return packageSpec.split("@")[0] ?? ""; +} + +export async function copyPatches({ + workspaceRootDir, + targetPackageManifest, + isolateDir, + includePatchedDependencies, + includeDevDependencies, +}: { + workspaceRootDir: string; + targetPackageManifest: PackageManifest; + isolateDir: string; + includePatchedDependencies: boolean; + includeDevDependencies: boolean; +}): Promise> { + const log = useLogger(); + + if (!includePatchedDependencies) { + log.debug("Skipping patch copying (includePatchedDependencies is false)"); + return {}; + } + + let workspaceRootManifest: PackageManifest; + try { + workspaceRootManifest = await readTypedJson( + path.join(workspaceRootDir, "package.json") + ); + } catch (error) { + log.warn( + `Could not read workspace root package.json: ${error instanceof Error ? error.message : String(error)}` + ); + return {}; + } + + const patchedDependencies = workspaceRootManifest.pnpm?.patchedDependencies; + + if (!patchedDependencies || Object.keys(patchedDependencies).length === 0) { + log.debug("No patched dependencies found in workspace root package.json"); + return {}; + } + + const patchesDir = path.join(isolateDir, "patches"); + await fs.ensureDir(patchesDir); + + log.debug( + `Found ${Object.keys(patchedDependencies).length} patched dependencies in workspace` + ); + + const filteredPatches = Object.entries(patchedDependencies).filter( + ([packageSpec]) => { + const packageName = getPackageName(packageSpec); + + // Check if it's a production dependency + if (targetPackageManifest.dependencies?.[packageName]) { + log.debug(`Including production dependency patch: ${packageSpec}`); + return true; + } + + // Check if it's a dev dependency and we should include dev dependencies + if (targetPackageManifest.devDependencies?.[packageName]) { + if (includeDevDependencies) { + log.debug(`Including dev dependency patch: ${packageSpec}`); + return true; + } + log.debug( + `Excluding dev dependency patch: ${packageSpec} (includeDevDependencies=false)` + ); + return false; + } + + log.debug( + `Excluding patch ${packageSpec}: package "${packageName}" not found in target dependencies` + ); + return false; + } + ); + + log.debug( + `Copying ${filteredPatches.length} patches (filtered from ${Object.keys(patchedDependencies).length})` + ); + + const copiedPatches: Record = {}; + + for (const [packageSpec, patchPath] of filteredPatches) { + const sourcePatchPath = path.resolve(workspaceRootDir, patchPath); + const targetPatchPath = path.join(patchesDir, path.basename(patchPath)); + + if (!fs.existsSync(sourcePatchPath)) { + log.warn( + `Patch file not found: ${getRootRelativeLogPath(sourcePatchPath, workspaceRootDir)}` + ); + continue; + } + + await fs.copy(sourcePatchPath, targetPatchPath); + log.debug(`Copied patch for ${packageSpec}: ${path.basename(patchPath)}`); + + copiedPatches[packageSpec] = `patches/${path.basename(patchPath)}`; + } + + if (Object.keys(copiedPatches).length > 0) { + log.debug( + `Patches copied to ${getRootRelativeLogPath(patchesDir, isolateDir)}` + ); + } + + return copiedPatches; +} diff --git a/src/lib/types.ts b/src/lib/types.ts index 7a75b32..d6b558b 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -2,6 +2,10 @@ import type { PackageManifest as PnpmPackageManifest } from "@pnpm/types"; export type PackageManifest = PnpmPackageManifest & { packageManager?: string; + pnpm?: { + patchedDependencies?: Record; + [key: string]: unknown; + }; }; export type WorkspacePackageInfo = { From 13436784177f93f9da2cae25e2825d0fbd0582fc Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 21:29:51 +0100 Subject: [PATCH 02/24] 1.27.0-0 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4c96ee2..4133af7 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "isolate-package", - "version": "1.26.1", + "version": "1.27.0-0", "description": "Isolate a monorepo package with its shared dependencies to form a self-contained directory, compatible with Firebase deploy", "author": "Thijs Koerselman", "license": "MIT", From 3dd34f0bd44f8573549b02819ec942f367ac3120 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 21:44:14 +0100 Subject: [PATCH 03/24] Use jsdoc comments consistently --- .claude/CLAUDE.md | 4 ++++ src/isolate.ts | 2 +- src/lib/lockfile/helpers/generate-pnpm-lockfile.ts | 10 +++++----- src/lib/lockfile/helpers/pnpm-map-importer.ts | 2 +- src/lib/patches/copy-patches.ts | 8 ++++---- 5 files changed, 15 insertions(+), 11 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 80c3a48..9f0dbb3 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -89,3 +89,7 @@ The codebase uses `~/` as path alias for `src/` (configured in tsconfig.json). ## Testing Tests use Vitest and are co-located with source files (`*.test.ts`). + +## Code Style + +- Use JSDoc style comments (`/** ... */`) for all comments, including single-line comments diff --git a/src/isolate.ts b/src/isolate.ts index b80b38d..67c9caa 100644 --- a/src/isolate.ts +++ b/src/isolate.ts @@ -309,7 +309,7 @@ export function createIsolator(config?: IsolateConfig) { }; } -// Keep the original function for backward compatibility +/** Keep the original function for backward compatibility */ export async function isolate(config?: IsolateConfig): Promise { return createIsolator(config)(); } diff --git a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts index 58bd45b..3a70baf 100644 --- a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts +++ b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts @@ -25,11 +25,11 @@ import { pnpmMapImporter } from "./pnpm-map-importer"; */ function getPackageName(packageSpec: string): string { if (packageSpec.startsWith("@")) { - // Scoped packages: @scope/package@version -> @scope/package + /** Scoped packages: @scope/package@version -> @scope/package */ const parts = packageSpec.split("@"); return `@${parts[1] ?? ""}`; } - // Regular packages: package@version -> package + /** Regular packages: package@version -> package */ return packageSpec.split("@")[0] ?? ""; } @@ -54,7 +54,7 @@ function filterPatchedDependencies( for (const [packageSpec, patchInfo] of Object.entries(patchedDependencies)) { const packageName = getPackageName(packageSpec); - // Check if it's a production dependency + /** Check if it's a production dependency */ if (targetPackageManifest.dependencies?.[packageName]) { filteredPatches[packageSpec] = patchInfo; includedCount++; @@ -64,7 +64,7 @@ function filterPatchedDependencies( continue; } - // Check if it's a dev dependency and we should include dev dependencies + /** Check if it's a dev dependency and we should include dev dependencies */ if (targetPackageManifest.devDependencies?.[packageName]) { if (includeDevDependencies) { filteredPatches[packageSpec] = patchInfo; @@ -79,7 +79,7 @@ function filterPatchedDependencies( continue; } - // Package not found in dependencies or devDependencies + /** Package not found in dependencies or devDependencies */ log.debug( `Excluding patch from lockfile: ${packageSpec} (package "${packageName}" not found in target dependencies)` ); diff --git a/src/lib/lockfile/helpers/pnpm-map-importer.ts b/src/lib/lockfile/helpers/pnpm-map-importer.ts index 2b703ea..11d484c 100644 --- a/src/lib/lockfile/helpers/pnpm-map-importer.ts +++ b/src/lib/lockfile/helpers/pnpm-map-importer.ts @@ -50,7 +50,7 @@ function pnpmMapDependenciesLinks( return value; } - // Replace backslashes with forward slashes to support Windows Git Bash + /** Replace backslashes with forward slashes to support Windows Git Bash */ const relativePath = path .relative(importerPath, got(directoryByPackageName, key)) .replace(path.sep, path.posix.sep); diff --git a/src/lib/patches/copy-patches.ts b/src/lib/patches/copy-patches.ts index b10c73c..b6e11c7 100644 --- a/src/lib/patches/copy-patches.ts +++ b/src/lib/patches/copy-patches.ts @@ -10,11 +10,11 @@ import { getRootRelativeLogPath, readTypedJson } from "~/lib/utils"; */ function getPackageName(packageSpec: string): string { if (packageSpec.startsWith("@")) { - // Scoped packages: @scope/package@version -> @scope/package + /** Scoped packages: @scope/package@version -> @scope/package */ const parts = packageSpec.split("@"); return `@${parts[1] ?? ""}`; } - // Regular packages: package@version -> package + /** Regular packages: package@version -> package */ return packageSpec.split("@")[0] ?? ""; } @@ -68,13 +68,13 @@ export async function copyPatches({ ([packageSpec]) => { const packageName = getPackageName(packageSpec); - // Check if it's a production dependency + /** Check if it's a production dependency */ if (targetPackageManifest.dependencies?.[packageName]) { log.debug(`Including production dependency patch: ${packageSpec}`); return true; } - // Check if it's a dev dependency and we should include dev dependencies + /** Check if it's a dev dependency and we should include dev dependencies */ if (targetPackageManifest.devDependencies?.[packageName]) { if (includeDevDependencies) { log.debug(`Including dev dependency patch: ${packageSpec}`); From 1f8a8c2b9490c2347c14f0f060fe0e30408a79d2 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 21:45:51 +0100 Subject: [PATCH 04/24] Extract utility --- .../helpers/generate-pnpm-lockfile.ts | 20 +++++-------------- src/lib/patches/copy-patches.ts | 20 +++++-------------- src/lib/utils/get-package-name.ts | 13 ++++++++++++ src/lib/utils/index.ts | 1 + 4 files changed, 24 insertions(+), 30 deletions(-) create mode 100644 src/lib/utils/get-package-name.ts diff --git a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts index 3a70baf..4e08078 100644 --- a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts +++ b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts @@ -16,23 +16,13 @@ import { pick } from "remeda"; import type { Logger } from "~/lib/logger"; import { useLogger } from "~/lib/logger"; import type { PackageManifest, PackagesRegistry } from "~/lib/types"; -import { getErrorMessage, isRushWorkspace } from "~/lib/utils"; +import { + getErrorMessage, + getPackageName, + isRushWorkspace, +} from "~/lib/utils"; import { pnpmMapImporter } from "./pnpm-map-importer"; -/** - * Extracts the package name from a package spec like "chalk@5.3.0" or - * "@firebase/app@1.2.3" - */ -function getPackageName(packageSpec: string): string { - if (packageSpec.startsWith("@")) { - /** Scoped packages: @scope/package@version -> @scope/package */ - const parts = packageSpec.split("@"); - return `@${parts[1] ?? ""}`; - } - /** Regular packages: package@version -> package */ - return packageSpec.split("@")[0] ?? ""; -} - /** * Filters patched dependencies to only include patches for packages that will * actually be present in the isolated lockfile based on dependency type. diff --git a/src/lib/patches/copy-patches.ts b/src/lib/patches/copy-patches.ts index b6e11c7..44c85ca 100644 --- a/src/lib/patches/copy-patches.ts +++ b/src/lib/patches/copy-patches.ts @@ -2,21 +2,11 @@ import fs from "fs-extra"; import path from "node:path"; import { useLogger } from "~/lib/logger"; import type { PackageManifest } from "~/lib/types"; -import { getRootRelativeLogPath, readTypedJson } from "~/lib/utils"; - -/** - * Extracts the package name from a package spec like "chalk@5.3.0" or - * "@firebase/app@1.2.3" - */ -function getPackageName(packageSpec: string): string { - if (packageSpec.startsWith("@")) { - /** Scoped packages: @scope/package@version -> @scope/package */ - const parts = packageSpec.split("@"); - return `@${parts[1] ?? ""}`; - } - /** Regular packages: package@version -> package */ - return packageSpec.split("@")[0] ?? ""; -} +import { + getPackageName, + getRootRelativeLogPath, + readTypedJson, +} from "~/lib/utils"; export async function copyPatches({ workspaceRootDir, diff --git a/src/lib/utils/get-package-name.ts b/src/lib/utils/get-package-name.ts new file mode 100644 index 0000000..1b93756 --- /dev/null +++ b/src/lib/utils/get-package-name.ts @@ -0,0 +1,13 @@ +/** + * Extracts the package name from a package spec like "chalk@5.3.0" or + * "@firebase/app@1.2.3" + */ +export function getPackageName(packageSpec: string): string { + if (packageSpec.startsWith("@")) { + /** Scoped packages: @scope/package@version -> @scope/package */ + const parts = packageSpec.split("@"); + return `@${parts[1] ?? ""}`; + } + /** Regular packages: package@version -> package */ + return packageSpec.split("@")[0] ?? ""; +} diff --git a/src/lib/utils/index.ts b/src/lib/utils/index.ts index f4e011a..9cbe59b 100644 --- a/src/lib/utils/index.ts +++ b/src/lib/utils/index.ts @@ -1,6 +1,7 @@ export * from "./filter-object-undefined"; export * from "./get-dirname"; export * from "./get-error-message"; +export * from "./get-package-name"; export * from "./inspect-value"; export * from "./is-present"; export * from "./is-rush-workspace"; From 979705d1fec67a11800cc15a6a94335a0e3dfda6 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 21:47:44 +0100 Subject: [PATCH 05/24] Extract filtering logic --- .../helpers/generate-pnpm-lockfile.ts | 63 +------------------ src/lib/patches/copy-patches.ts | 48 ++++---------- src/lib/utils/filter-patched-dependencies.ts | 59 +++++++++++++++++ src/lib/utils/index.ts | 1 + 4 files changed, 74 insertions(+), 97 deletions(-) create mode 100644 src/lib/utils/filter-patched-dependencies.ts diff --git a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts index 4e08078..2f5a02c 100644 --- a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts +++ b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts @@ -13,76 +13,15 @@ import { import { pruneLockfile as pruneLockfile_v8 } from "pnpm_prune_lockfile_v8"; import { pruneLockfile as pruneLockfile_v9 } from "pnpm_prune_lockfile_v9"; import { pick } from "remeda"; -import type { Logger } from "~/lib/logger"; import { useLogger } from "~/lib/logger"; import type { PackageManifest, PackagesRegistry } from "~/lib/types"; import { + filterPatchedDependencies, getErrorMessage, - getPackageName, isRushWorkspace, } from "~/lib/utils"; import { pnpmMapImporter } from "./pnpm-map-importer"; -/** - * Filters patched dependencies to only include patches for packages that will - * actually be present in the isolated lockfile based on dependency type. - */ -function filterPatchedDependencies( - patchedDependencies: Record | undefined, - targetPackageManifest: PackageManifest, - includeDevDependencies: boolean, - log: Logger -): Record | undefined { - if (!patchedDependencies || typeof patchedDependencies !== "object") { - return undefined; - } - - const filteredPatches: Record = {}; - let includedCount = 0; - let excludedCount = 0; - - for (const [packageSpec, patchInfo] of Object.entries(patchedDependencies)) { - const packageName = getPackageName(packageSpec); - - /** Check if it's a production dependency */ - if (targetPackageManifest.dependencies?.[packageName]) { - filteredPatches[packageSpec] = patchInfo; - includedCount++; - log.debug( - `Including production dependency patch in lockfile: ${packageSpec}` - ); - continue; - } - - /** Check if it's a dev dependency and we should include dev dependencies */ - if (targetPackageManifest.devDependencies?.[packageName]) { - if (includeDevDependencies) { - filteredPatches[packageSpec] = patchInfo; - includedCount++; - log.debug(`Including dev dependency patch in lockfile: ${packageSpec}`); - } else { - excludedCount++; - log.debug( - `Excluding dev dependency patch from lockfile: ${packageSpec}` - ); - } - continue; - } - - /** Package not found in dependencies or devDependencies */ - log.debug( - `Excluding patch from lockfile: ${packageSpec} (package "${packageName}" not found in target dependencies)` - ); - excludedCount++; - } - - log.debug( - `Filtered patched dependencies: ${includedCount} included, ${excludedCount} excluded` - ); - - return Object.keys(filteredPatches).length > 0 ? filteredPatches : undefined; -} - export async function generatePnpmLockfile({ workspaceRootDir, targetPackageDir, diff --git a/src/lib/patches/copy-patches.ts b/src/lib/patches/copy-patches.ts index 44c85ca..2303d9a 100644 --- a/src/lib/patches/copy-patches.ts +++ b/src/lib/patches/copy-patches.ts @@ -3,7 +3,7 @@ import path from "node:path"; import { useLogger } from "~/lib/logger"; import type { PackageManifest } from "~/lib/types"; import { - getPackageName, + filterPatchedDependencies, getRootRelativeLogPath, readTypedJson, } from "~/lib/utils"; @@ -47,49 +47,27 @@ export async function copyPatches({ return {}; } - const patchesDir = path.join(isolateDir, "patches"); - await fs.ensureDir(patchesDir); - log.debug( `Found ${Object.keys(patchedDependencies).length} patched dependencies in workspace` ); - const filteredPatches = Object.entries(patchedDependencies).filter( - ([packageSpec]) => { - const packageName = getPackageName(packageSpec); - - /** Check if it's a production dependency */ - if (targetPackageManifest.dependencies?.[packageName]) { - log.debug(`Including production dependency patch: ${packageSpec}`); - return true; - } - - /** Check if it's a dev dependency and we should include dev dependencies */ - if (targetPackageManifest.devDependencies?.[packageName]) { - if (includeDevDependencies) { - log.debug(`Including dev dependency patch: ${packageSpec}`); - return true; - } - log.debug( - `Excluding dev dependency patch: ${packageSpec} (includeDevDependencies=false)` - ); - return false; - } - - log.debug( - `Excluding patch ${packageSpec}: package "${packageName}" not found in target dependencies` - ); - return false; - } + const filteredPatches = filterPatchedDependencies( + patchedDependencies, + targetPackageManifest, + includeDevDependencies, + log ); - log.debug( - `Copying ${filteredPatches.length} patches (filtered from ${Object.keys(patchedDependencies).length})` - ); + if (!filteredPatches) { + return {}; + } + + const patchesDir = path.join(isolateDir, "patches"); + await fs.ensureDir(patchesDir); const copiedPatches: Record = {}; - for (const [packageSpec, patchPath] of filteredPatches) { + for (const [packageSpec, patchPath] of Object.entries(filteredPatches)) { const sourcePatchPath = path.resolve(workspaceRootDir, patchPath); const targetPatchPath = path.join(patchesDir, path.basename(patchPath)); diff --git a/src/lib/utils/filter-patched-dependencies.ts b/src/lib/utils/filter-patched-dependencies.ts new file mode 100644 index 0000000..7457697 --- /dev/null +++ b/src/lib/utils/filter-patched-dependencies.ts @@ -0,0 +1,59 @@ +import type { Logger } from "~/lib/logger"; +import type { PackageManifest } from "~/lib/types"; +import { getPackageName } from "./get-package-name"; + +/** + * Filters patched dependencies to only include patches for packages that are + * present in the target package's dependencies based on dependency type. + */ +export function filterPatchedDependencies( + patchedDependencies: Record | undefined, + targetPackageManifest: PackageManifest, + includeDevDependencies: boolean, + log: Logger +): Record | undefined { + if (!patchedDependencies || typeof patchedDependencies !== "object") { + return undefined; + } + + const filteredPatches: Record = {}; + let includedCount = 0; + let excludedCount = 0; + + for (const [packageSpec, patchInfo] of Object.entries(patchedDependencies)) { + const packageName = getPackageName(packageSpec); + + /** Check if it's a production dependency */ + if (targetPackageManifest.dependencies?.[packageName]) { + filteredPatches[packageSpec] = patchInfo; + includedCount++; + log.debug(`Including production dependency patch: ${packageSpec}`); + continue; + } + + /** Check if it's a dev dependency and we should include dev dependencies */ + if (targetPackageManifest.devDependencies?.[packageName]) { + if (includeDevDependencies) { + filteredPatches[packageSpec] = patchInfo; + includedCount++; + log.debug(`Including dev dependency patch: ${packageSpec}`); + } else { + excludedCount++; + log.debug(`Excluding dev dependency patch: ${packageSpec}`); + } + continue; + } + + /** Package not found in dependencies or devDependencies */ + log.debug( + `Excluding patch: ${packageSpec} (package "${packageName}" not in target dependencies)` + ); + excludedCount++; + } + + log.debug( + `Filtered patches: ${includedCount} included, ${excludedCount} excluded` + ); + + return Object.keys(filteredPatches).length > 0 ? filteredPatches : undefined; +} diff --git a/src/lib/utils/index.ts b/src/lib/utils/index.ts index 9cbe59b..433ea94 100644 --- a/src/lib/utils/index.ts +++ b/src/lib/utils/index.ts @@ -1,4 +1,5 @@ export * from "./filter-object-undefined"; +export * from "./filter-patched-dependencies"; export * from "./get-dirname"; export * from "./get-error-message"; export * from "./get-package-name"; From f704afeee014e18beb048edb5de9651a5930140e Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 21:49:13 +0100 Subject: [PATCH 06/24] Avoid patch naming collisions --- src/lib/patches/copy-patches.ts | 28 +++++++++++++++++++++++++--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/src/lib/patches/copy-patches.ts b/src/lib/patches/copy-patches.ts index 2303d9a..486242d 100644 --- a/src/lib/patches/copy-patches.ts +++ b/src/lib/patches/copy-patches.ts @@ -66,10 +66,10 @@ export async function copyPatches({ await fs.ensureDir(patchesDir); const copiedPatches: Record = {}; + const usedFilenames = new Set(); for (const [packageSpec, patchPath] of Object.entries(filteredPatches)) { const sourcePatchPath = path.resolve(workspaceRootDir, patchPath); - const targetPatchPath = path.join(patchesDir, path.basename(patchPath)); if (!fs.existsSync(sourcePatchPath)) { log.warn( @@ -78,10 +78,32 @@ export async function copyPatches({ continue; } + /** Generate a unique filename to avoid collisions from different subdirectories */ + const basename = path.basename(patchPath); + let targetFilename = basename; + + if (usedFilenames.has(targetFilename)) { + const ext = path.extname(basename); + const name = path.basename(basename, ext); + let counter = 1; + + do { + targetFilename = `${name}-${counter}${ext}`; + counter++; + } while (usedFilenames.has(targetFilename)); + + log.debug( + `Renamed patch ${basename} to ${targetFilename} to avoid collision` + ); + } + + usedFilenames.add(targetFilename); + + const targetPatchPath = path.join(patchesDir, targetFilename); await fs.copy(sourcePatchPath, targetPatchPath); - log.debug(`Copied patch for ${packageSpec}: ${path.basename(patchPath)}`); + log.debug(`Copied patch for ${packageSpec}: ${targetFilename}`); - copiedPatches[packageSpec] = `patches/${path.basename(patchPath)}`; + copiedPatches[packageSpec] = `patches/${targetFilename}`; } if (Object.keys(copiedPatches).length > 0) { From 02659995c76b3a5fd8f5fb69049286e408c30fbe Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 21:50:48 +0100 Subject: [PATCH 07/24] Add tests for filter patched deps --- .../utils/filter-patched-dependencies.test.ts | 186 ++++++++++++++++++ 1 file changed, 186 insertions(+) create mode 100644 src/lib/utils/filter-patched-dependencies.test.ts diff --git a/src/lib/utils/filter-patched-dependencies.test.ts b/src/lib/utils/filter-patched-dependencies.test.ts new file mode 100644 index 0000000..8542379 --- /dev/null +++ b/src/lib/utils/filter-patched-dependencies.test.ts @@ -0,0 +1,186 @@ +import { describe, expect, it, vi } from "vitest"; +import type { Logger } from "~/lib/logger"; +import type { PackageManifest } from "~/lib/types"; +import { filterPatchedDependencies } from "./filter-patched-dependencies"; + +const createMockLogger = (): Logger => ({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), +}); + +describe("filterPatchedDependencies", () => { + it("should return undefined when patchedDependencies is undefined", () => { + const manifest: PackageManifest = { name: "test", version: "1.0.0" }; + const log = createMockLogger(); + + const result = filterPatchedDependencies(undefined, manifest, false, log); + + expect(result).toBeUndefined(); + }); + + it("should return undefined when patchedDependencies is empty", () => { + const manifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { lodash: "^4.0.0" }, + }; + const log = createMockLogger(); + + const result = filterPatchedDependencies({}, manifest, false, log); + + expect(result).toBeUndefined(); + }); + + it("should include patches for production dependencies", () => { + const manifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { lodash: "^4.0.0" }, + }; + const patches = { "lodash@4.17.21": "patches/lodash.patch" }; + const log = createMockLogger(); + + const result = filterPatchedDependencies(patches, manifest, false, log); + + expect(result).toEqual({ "lodash@4.17.21": "patches/lodash.patch" }); + }); + + it("should include patches for dev dependencies when includeDevDependencies is true", () => { + const manifest: PackageManifest = { + name: "test", + version: "1.0.0", + devDependencies: { vitest: "^1.0.0" }, + }; + const patches = { "vitest@1.0.0": "patches/vitest.patch" }; + const log = createMockLogger(); + + const result = filterPatchedDependencies(patches, manifest, true, log); + + expect(result).toEqual({ "vitest@1.0.0": "patches/vitest.patch" }); + }); + + it("should exclude patches for dev dependencies when includeDevDependencies is false", () => { + const manifest: PackageManifest = { + name: "test", + version: "1.0.0", + devDependencies: { vitest: "^1.0.0" }, + }; + const patches = { "vitest@1.0.0": "patches/vitest.patch" }; + const log = createMockLogger(); + + const result = filterPatchedDependencies(patches, manifest, false, log); + + expect(result).toBeUndefined(); + }); + + it("should exclude patches for packages not in target dependencies", () => { + const manifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { lodash: "^4.0.0" }, + }; + const patches = { "other-package@1.0.0": "patches/other.patch" }; + const log = createMockLogger(); + + const result = filterPatchedDependencies(patches, manifest, false, log); + + expect(result).toBeUndefined(); + }); + + it("should handle scoped package names correctly", () => { + const manifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { "@firebase/app": "^1.0.0" }, + }; + const patches = { "@firebase/app@1.2.3": "patches/firebase-app.patch" }; + const log = createMockLogger(); + + const result = filterPatchedDependencies(patches, manifest, false, log); + + expect(result).toEqual({ + "@firebase/app@1.2.3": "patches/firebase-app.patch", + }); + }); + + it("should filter mixed patches correctly", () => { + const manifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { lodash: "^4.0.0", "@firebase/app": "^1.0.0" }, + devDependencies: { vitest: "^1.0.0" }, + }; + const patches = { + "lodash@4.17.21": "patches/lodash.patch", + "@firebase/app@1.2.3": "patches/firebase-app.patch", + "vitest@1.0.0": "patches/vitest.patch", + "unknown@1.0.0": "patches/unknown.patch", + }; + const log = createMockLogger(); + + const result = filterPatchedDependencies(patches, manifest, false, log); + + expect(result).toEqual({ + "lodash@4.17.21": "patches/lodash.patch", + "@firebase/app@1.2.3": "patches/firebase-app.patch", + }); + }); + + it("should include dev dependency patches when includeDevDependencies is true in mixed scenario", () => { + const manifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { lodash: "^4.0.0" }, + devDependencies: { vitest: "^1.0.0" }, + }; + const patches = { + "lodash@4.17.21": "patches/lodash.patch", + "vitest@1.0.0": "patches/vitest.patch", + }; + const log = createMockLogger(); + + const result = filterPatchedDependencies(patches, manifest, true, log); + + expect(result).toEqual({ + "lodash@4.17.21": "patches/lodash.patch", + "vitest@1.0.0": "patches/vitest.patch", + }); + }); + + it("should return undefined when all patches are filtered out", () => { + const manifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { lodash: "^4.0.0" }, + }; + const patches = { + "unknown-a@1.0.0": "patches/a.patch", + "unknown-b@2.0.0": "patches/b.patch", + }; + const log = createMockLogger(); + + const result = filterPatchedDependencies(patches, manifest, false, log); + + expect(result).toBeUndefined(); + }); + + it("should preserve patch value types", () => { + const manifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { lodash: "^4.0.0" }, + }; + const patches = { + "lodash@4.17.21": { path: "patches/lodash.patch", hash: "abc123" }, + }; + const log = createMockLogger(); + + const result = filterPatchedDependencies(patches, manifest, false, log); + + expect(result).toEqual({ + "lodash@4.17.21": { path: "patches/lodash.patch", hash: "abc123" }, + }); + }); +}); From b2ff870ae8a5d1ea61fbd8f78c14e575017f24e1 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 21:52:17 +0100 Subject: [PATCH 08/24] Something --- src/isolate.ts | 40 +++++++++++++++++++++------------------- 1 file changed, 21 insertions(+), 19 deletions(-) diff --git a/src/isolate.ts b/src/isolate.ts index 67c9caa..e456d33 100644 --- a/src/isolate.ts +++ b/src/isolate.ts @@ -221,28 +221,30 @@ export function createIsolator(config?: IsolateConfig) { includeDevDependencies: config.includeDevDependencies, }); - /** Add copied patches to the isolated package.json */ - if (Object.keys(copiedPatches).length > 0) { - const manifest = await readManifest(isolateDir); - if (!manifest.pnpm) { - manifest.pnpm = {}; - } - manifest.pnpm.patchedDependencies = copiedPatches; - await writeManifest(isolateDir, manifest); - log.debug( - `Added ${Object.keys(copiedPatches).length} patches to isolated package.json` - ); - } + const hasCopiedPatches = Object.keys(copiedPatches).length > 0; - if (usedFallbackToNpm) { - /** - * When we fall back to NPM, we set the manifest package manager to the - * available NPM version. - */ + /** Update manifest if patches were copied or npm fallback is needed */ + if (hasCopiedPatches || usedFallbackToNpm) { const manifest = await readManifest(isolateDir); - const npmVersion = getVersion("npm"); - manifest.packageManager = `npm@${npmVersion}`; + if (hasCopiedPatches) { + if (!manifest.pnpm) { + manifest.pnpm = {}; + } + manifest.pnpm.patchedDependencies = copiedPatches; + log.debug( + `Added ${Object.keys(copiedPatches).length} patches to isolated package.json` + ); + } + + if (usedFallbackToNpm) { + /** + * When we fall back to NPM, we set the manifest package manager to the + * available NPM version. + */ + const npmVersion = getVersion("npm"); + manifest.packageManager = `npm@${npmVersion}`; + } await writeManifest(isolateDir, manifest); } From e72cae4757cf1c12521561e30315d4e30bbd44f5 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 21:53:03 +0100 Subject: [PATCH 09/24] Use output manifest consistently --- src/isolate.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/isolate.ts b/src/isolate.ts index e456d33..881c3b0 100644 --- a/src/isolate.ts +++ b/src/isolate.ts @@ -215,7 +215,7 @@ export function createIsolator(config?: IsolateConfig) { /** Copy patch files if includePatchedDependencies is enabled */ const copiedPatches = await copyPatches({ workspaceRootDir, - targetPackageManifest, + targetPackageManifest: outputManifest, isolateDir, includePatchedDependencies: config.includePatchedDependencies, includeDevDependencies: config.includeDevDependencies, From 182a7cdd39b0c7867790ae91a6cf3146ad5b62e1 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 21:55:29 +0100 Subject: [PATCH 10/24] Add tests for copy patches --- src/lib/patches/copy-patches.test.ts | 389 +++++++++++++++++++++++++++ 1 file changed, 389 insertions(+) create mode 100644 src/lib/patches/copy-patches.test.ts diff --git a/src/lib/patches/copy-patches.test.ts b/src/lib/patches/copy-patches.test.ts new file mode 100644 index 0000000..cb0553c --- /dev/null +++ b/src/lib/patches/copy-patches.test.ts @@ -0,0 +1,389 @@ +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; +import type { PackageManifest } from "~/lib/types"; +import { copyPatches } from "./copy-patches"; + +/** Mock fs-extra */ +vi.mock("fs-extra", () => ({ + default: { + ensureDir: vi.fn(), + existsSync: vi.fn(), + copy: vi.fn(), + }, +})); + +/** Mock the utils */ +vi.mock("~/lib/utils", () => ({ + filterPatchedDependencies: vi.fn(), + getRootRelativeLogPath: vi.fn((p: string) => p), + readTypedJson: vi.fn(), +})); + +/** Mock the logger */ +vi.mock("~/lib/logger", () => ({ + useLogger: () => ({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), +})); + +const fs = vi.mocked((await import("fs-extra")).default); +const { filterPatchedDependencies, readTypedJson } = vi.mocked( + await import("~/lib/utils") +); + +describe("copyPatches", () => { + beforeEach(() => { + vi.clearAllMocks(); + }); + + afterEach(() => { + vi.restoreAllMocks(); + }); + + it("should return empty object when includePatchedDependencies is false", async () => { + const result = await copyPatches({ + workspaceRootDir: "/workspace", + targetPackageManifest: { name: "test", version: "1.0.0" }, + isolateDir: "/workspace/isolate", + includePatchedDependencies: false, + includeDevDependencies: false, + }); + + expect(result).toEqual({}); + expect(readTypedJson).not.toHaveBeenCalled(); + }); + + it("should return empty object when workspace root package.json cannot be read", async () => { + readTypedJson.mockRejectedValue(new Error("File not found")); + + const result = await copyPatches({ + workspaceRootDir: "/workspace", + targetPackageManifest: { name: "test", version: "1.0.0" }, + isolateDir: "/workspace/isolate", + includePatchedDependencies: true, + includeDevDependencies: false, + }); + + expect(result).toEqual({}); + }); + + it("should return empty object when no patchedDependencies in workspace root", async () => { + readTypedJson.mockResolvedValue({ + name: "root", + version: "1.0.0", + } as PackageManifest); + + const result = await copyPatches({ + workspaceRootDir: "/workspace", + targetPackageManifest: { name: "test", version: "1.0.0" }, + isolateDir: "/workspace/isolate", + includePatchedDependencies: true, + includeDevDependencies: false, + }); + + expect(result).toEqual({}); + }); + + it("should return empty object when all patches are filtered out", async () => { + readTypedJson.mockResolvedValue({ + name: "root", + version: "1.0.0", + pnpm: { + patchedDependencies: { + "lodash@4.17.21": "patches/lodash.patch", + }, + }, + } as PackageManifest); + filterPatchedDependencies.mockReturnValue(undefined); + + const result = await copyPatches({ + workspaceRootDir: "/workspace", + targetPackageManifest: { name: "test", version: "1.0.0" }, + isolateDir: "/workspace/isolate", + includePatchedDependencies: true, + includeDevDependencies: false, + }); + + expect(result).toEqual({}); + }); + + it("should copy patches for production dependencies", async () => { + const targetManifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { lodash: "^4.0.0" }, + }; + + readTypedJson.mockResolvedValue({ + name: "root", + version: "1.0.0", + pnpm: { + patchedDependencies: { + "lodash@4.17.21": "patches/lodash.patch", + }, + }, + } as PackageManifest); + + filterPatchedDependencies.mockReturnValue({ + "lodash@4.17.21": "patches/lodash.patch", + }); + + fs.existsSync.mockReturnValue(true); + + const result = await copyPatches({ + workspaceRootDir: "/workspace", + targetPackageManifest: targetManifest, + isolateDir: "/workspace/isolate", + includePatchedDependencies: true, + includeDevDependencies: false, + }); + + expect(result).toEqual({ + "lodash@4.17.21": "patches/lodash.patch", + }); + expect(fs.ensureDir).toHaveBeenCalledWith("/workspace/isolate/patches"); + expect(fs.copy).toHaveBeenCalledWith( + "/workspace/patches/lodash.patch", + "/workspace/isolate/patches/lodash.patch" + ); + }); + + it("should include dev dependency patches when includeDevDependencies is true", async () => { + const targetManifest: PackageManifest = { + name: "test", + version: "1.0.0", + devDependencies: { vitest: "^1.0.0" }, + }; + + readTypedJson.mockResolvedValue({ + name: "root", + version: "1.0.0", + pnpm: { + patchedDependencies: { + "vitest@1.0.0": "patches/vitest.patch", + }, + }, + } as PackageManifest); + + filterPatchedDependencies.mockReturnValue({ + "vitest@1.0.0": "patches/vitest.patch", + }); + + fs.existsSync.mockReturnValue(true); + + const result = await copyPatches({ + workspaceRootDir: "/workspace", + targetPackageManifest: targetManifest, + isolateDir: "/workspace/isolate", + includePatchedDependencies: true, + includeDevDependencies: true, + }); + + expect(result).toEqual({ + "vitest@1.0.0": "patches/vitest.patch", + }); + expect(filterPatchedDependencies).toHaveBeenCalledWith( + { "vitest@1.0.0": "patches/vitest.patch" }, + targetManifest, + true, + expect.any(Object) + ); + }); + + it("should skip missing patch files and log a warning", async () => { + const targetManifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { lodash: "^4.0.0" }, + }; + + readTypedJson.mockResolvedValue({ + name: "root", + version: "1.0.0", + pnpm: { + patchedDependencies: { + "lodash@4.17.21": "patches/lodash.patch", + }, + }, + } as PackageManifest); + + filterPatchedDependencies.mockReturnValue({ + "lodash@4.17.21": "patches/lodash.patch", + }); + + fs.existsSync.mockReturnValue(false); + + const result = await copyPatches({ + workspaceRootDir: "/workspace", + targetPackageManifest: targetManifest, + isolateDir: "/workspace/isolate", + includePatchedDependencies: true, + includeDevDependencies: false, + }); + + expect(result).toEqual({}); + expect(fs.copy).not.toHaveBeenCalled(); + }); + + it("should handle scoped package names correctly", async () => { + const targetManifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { "@firebase/app": "^1.0.0" }, + }; + + readTypedJson.mockResolvedValue({ + name: "root", + version: "1.0.0", + pnpm: { + patchedDependencies: { + "@firebase/app@1.2.3": "patches/firebase-app.patch", + }, + }, + } as PackageManifest); + + filterPatchedDependencies.mockReturnValue({ + "@firebase/app@1.2.3": "patches/firebase-app.patch", + }); + + fs.existsSync.mockReturnValue(true); + + const result = await copyPatches({ + workspaceRootDir: "/workspace", + targetPackageManifest: targetManifest, + isolateDir: "/workspace/isolate", + includePatchedDependencies: true, + includeDevDependencies: false, + }); + + expect(result).toEqual({ + "@firebase/app@1.2.3": "patches/firebase-app.patch", + }); + }); + + it("should handle filename collisions by renaming", async () => { + const targetManifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { "pkg-a": "^1.0.0", "pkg-b": "^1.0.0" }, + }; + + readTypedJson.mockResolvedValue({ + name: "root", + version: "1.0.0", + pnpm: { + patchedDependencies: { + "pkg-a@1.0.0": "patches/v1/fix.patch", + "pkg-b@1.0.0": "patches/v2/fix.patch", + }, + }, + } as PackageManifest); + + filterPatchedDependencies.mockReturnValue({ + "pkg-a@1.0.0": "patches/v1/fix.patch", + "pkg-b@1.0.0": "patches/v2/fix.patch", + }); + + fs.existsSync.mockReturnValue(true); + + const result = await copyPatches({ + workspaceRootDir: "/workspace", + targetPackageManifest: targetManifest, + isolateDir: "/workspace/isolate", + includePatchedDependencies: true, + includeDevDependencies: false, + }); + + expect(result).toEqual({ + "pkg-a@1.0.0": "patches/fix.patch", + "pkg-b@1.0.0": "patches/fix-1.patch", + }); + expect(fs.copy).toHaveBeenCalledTimes(2); + }); + + it("should transform patch paths correctly for isolated directory", async () => { + const targetManifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { lodash: "^4.0.0" }, + }; + + readTypedJson.mockResolvedValue({ + name: "root", + version: "1.0.0", + pnpm: { + patchedDependencies: { + "lodash@4.17.21": "some/nested/path/lodash.patch", + }, + }, + } as PackageManifest); + + filterPatchedDependencies.mockReturnValue({ + "lodash@4.17.21": "some/nested/path/lodash.patch", + }); + + fs.existsSync.mockReturnValue(true); + + const result = await copyPatches({ + workspaceRootDir: "/workspace", + targetPackageManifest: targetManifest, + isolateDir: "/workspace/isolate", + includePatchedDependencies: true, + includeDevDependencies: false, + }); + + /** The path should be flattened to patches/ directory */ + expect(result).toEqual({ + "lodash@4.17.21": "patches/lodash.patch", + }); + expect(fs.copy).toHaveBeenCalledWith( + "/workspace/some/nested/path/lodash.patch", + "/workspace/isolate/patches/lodash.patch" + ); + }); + + it("should copy multiple patches correctly", async () => { + const targetManifest: PackageManifest = { + name: "test", + version: "1.0.0", + dependencies: { + lodash: "^4.0.0", + "@firebase/app": "^1.0.0", + }, + }; + + readTypedJson.mockResolvedValue({ + name: "root", + version: "1.0.0", + pnpm: { + patchedDependencies: { + "lodash@4.17.21": "patches/lodash.patch", + "@firebase/app@1.2.3": "patches/firebase-app.patch", + }, + }, + } as PackageManifest); + + filterPatchedDependencies.mockReturnValue({ + "lodash@4.17.21": "patches/lodash.patch", + "@firebase/app@1.2.3": "patches/firebase-app.patch", + }); + + fs.existsSync.mockReturnValue(true); + + const result = await copyPatches({ + workspaceRootDir: "/workspace", + targetPackageManifest: targetManifest, + isolateDir: "/workspace/isolate", + includePatchedDependencies: true, + includeDevDependencies: false, + }); + + expect(result).toEqual({ + "lodash@4.17.21": "patches/lodash.patch", + "@firebase/app@1.2.3": "patches/firebase-app.patch", + }); + expect(fs.copy).toHaveBeenCalledTimes(2); + }); +}); From 41a52519ad3b29179b2c0d31c85cd29123304598 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 21:56:26 +0100 Subject: [PATCH 11/24] Add tests for get package name --- src/lib/utils/get-package-name.test.ts | 68 ++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 src/lib/utils/get-package-name.test.ts diff --git a/src/lib/utils/get-package-name.test.ts b/src/lib/utils/get-package-name.test.ts new file mode 100644 index 0000000..2805111 --- /dev/null +++ b/src/lib/utils/get-package-name.test.ts @@ -0,0 +1,68 @@ +import { describe, expect, it } from "vitest"; +import { getPackageName } from "./get-package-name"; + +describe("getPackageName", () => { + describe("scoped packages", () => { + it("should extract name from scoped package with version", () => { + expect(getPackageName("@firebase/app@1.2.3")).toBe("@firebase/app"); + }); + + it("should extract name from scoped package with complex version", () => { + expect(getPackageName("@types/node@20.10.0")).toBe("@types/node"); + }); + + it("should handle scoped package without version", () => { + expect(getPackageName("@firebase/app")).toBe("@firebase/app"); + }); + + it("should handle deeply nested scoped package", () => { + expect(getPackageName("@org/sub/package@1.0.0")).toBe("@org/sub/package"); + }); + }); + + describe("regular packages", () => { + it("should extract name from regular package with version", () => { + expect(getPackageName("lodash@4.17.21")).toBe("lodash"); + }); + + it("should extract name from regular package with complex version", () => { + expect(getPackageName("typescript@5.3.0-beta")).toBe("typescript"); + }); + + it("should handle regular package without version", () => { + expect(getPackageName("lodash")).toBe("lodash"); + }); + + it("should handle package with hyphenated name", () => { + expect(getPackageName("fs-extra@11.0.0")).toBe("fs-extra"); + }); + + it("should handle package with underscores", () => { + expect(getPackageName("some_package@1.0.0")).toBe("some_package"); + }); + }); + + describe("edge cases", () => { + it("should return empty string for empty input", () => { + expect(getPackageName("")).toBe(""); + }); + + it("should handle @ symbol only", () => { + expect(getPackageName("@")).toBe("@"); + }); + + it("should handle scoped package with only scope", () => { + expect(getPackageName("@scope/")).toBe("@scope/"); + }); + + it("should handle multiple @ symbols in version (edge case)", () => { + /** This is a malformed input but should not throw */ + expect(getPackageName("package@1.0.0@extra")).toBe("package"); + }); + + it("should handle scoped package with multiple @ in version", () => { + /** Scoped packages split on @ so this tests the behavior */ + expect(getPackageName("@scope/pkg@1.0.0@extra")).toBe("@scope/pkg"); + }); + }); +}); From 25fc4f2653b932e511231daf99723c1def8e65c6 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:02:14 +0100 Subject: [PATCH 12/24] Use correct log path function and fix formatting - Use getIsolateRelativeLogPath for patches directory logging - Add getIsolateRelativeLogPath to test mocks - Format CLAUDE.md code style section --- .claude/CLAUDE.md | 3 ++- src/lib/patches/copy-patches.test.ts | 1 + src/lib/patches/copy-patches.ts | 8 ++++++-- 3 files changed, 9 insertions(+), 3 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 9f0dbb3..9c57b34 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -92,4 +92,5 @@ Tests use Vitest and are co-located with source files (`*.test.ts`). ## Code Style -- Use JSDoc style comments (`/** ... */`) for all comments, including single-line comments +- Use JSDoc style comments (`/** ... */`) for all comments, including + single-line comments diff --git a/src/lib/patches/copy-patches.test.ts b/src/lib/patches/copy-patches.test.ts index cb0553c..0eba9fa 100644 --- a/src/lib/patches/copy-patches.test.ts +++ b/src/lib/patches/copy-patches.test.ts @@ -14,6 +14,7 @@ vi.mock("fs-extra", () => ({ /** Mock the utils */ vi.mock("~/lib/utils", () => ({ filterPatchedDependencies: vi.fn(), + getIsolateRelativeLogPath: vi.fn((p: string) => p), getRootRelativeLogPath: vi.fn((p: string) => p), readTypedJson: vi.fn(), })); diff --git a/src/lib/patches/copy-patches.ts b/src/lib/patches/copy-patches.ts index 486242d..574d682 100644 --- a/src/lib/patches/copy-patches.ts +++ b/src/lib/patches/copy-patches.ts @@ -4,6 +4,7 @@ import { useLogger } from "~/lib/logger"; import type { PackageManifest } from "~/lib/types"; import { filterPatchedDependencies, + getIsolateRelativeLogPath, getRootRelativeLogPath, readTypedJson, } from "~/lib/utils"; @@ -78,7 +79,10 @@ export async function copyPatches({ continue; } - /** Generate a unique filename to avoid collisions from different subdirectories */ + /** + * Generate a unique filename to avoid collisions from different + * subdirectories + */ const basename = path.basename(patchPath); let targetFilename = basename; @@ -108,7 +112,7 @@ export async function copyPatches({ if (Object.keys(copiedPatches).length > 0) { log.debug( - `Patches copied to ${getRootRelativeLogPath(patchesDir, isolateDir)}` + `Patches copied to ${getIsolateRelativeLogPath(patchesDir, isolateDir)}` ); } From f7896fefc784706e908ed5f10c93c43e094db2c2 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:15:49 +0100 Subject: [PATCH 13/24] Fix lockfile patch paths to match copied file locations The lockfile was being generated before patches were copied, causing paths in the lockfile to not match the actual copied files. Changes: - Reorder operations so copyPatches runs before processLockfile - Update copyPatches to return PatchFile format with path and hash - Read hashes from original lockfile to preserve them after copying - Pass pre-computed patched dependencies to lockfile generator - Add PatchFile interface to types.ts - Update tests to expect PatchFile format --- src/isolate.ts | 32 ++++++---- .../helpers/generate-pnpm-lockfile.ts | 33 +++-------- src/lib/lockfile/helpers/pnpm-map-importer.ts | 1 - src/lib/lockfile/process-lockfile.ts | 7 ++- src/lib/patches/copy-patches.test.ts | 16 ++--- src/lib/patches/copy-patches.ts | 59 +++++++++++++++++-- src/lib/types.ts | 9 +++ 7 files changed, 108 insertions(+), 49 deletions(-) diff --git a/src/isolate.ts b/src/isolate.ts index 881c3b0..66a1158 100644 --- a/src/isolate.ts +++ b/src/isolate.ts @@ -200,6 +200,19 @@ export function createIsolator(config?: IsolateConfig) { await writeManifest(isolateDir, outputManifest); + /** + * Copy patch files before generating lockfile so the lockfile contains + * the correct transformed paths (flattened to patches/ with collision + * avoidance). + */ + const copiedPatches = await copyPatches({ + workspaceRootDir, + targetPackageManifest: outputManifest, + isolateDir, + includePatchedDependencies: config.includePatchedDependencies, + includeDevDependencies: config.includeDevDependencies, + }); + /** Generate an isolated lockfile based on the original one */ const usedFallbackToNpm = await processLockfile({ workspaceRootDir, @@ -209,18 +222,11 @@ export function createIsolator(config?: IsolateConfig) { targetPackageDir, targetPackageName: targetPackageManifest.name, targetPackageManifest: outputManifest, + patchedDependencies: + Object.keys(copiedPatches).length > 0 ? copiedPatches : undefined, config, }); - /** Copy patch files if includePatchedDependencies is enabled */ - const copiedPatches = await copyPatches({ - workspaceRootDir, - targetPackageManifest: outputManifest, - isolateDir, - includePatchedDependencies: config.includePatchedDependencies, - includeDevDependencies: config.includeDevDependencies, - }); - const hasCopiedPatches = Object.keys(copiedPatches).length > 0; /** Update manifest if patches were copied or npm fallback is needed */ @@ -231,7 +237,13 @@ export function createIsolator(config?: IsolateConfig) { if (!manifest.pnpm) { manifest.pnpm = {}; } - manifest.pnpm.patchedDependencies = copiedPatches; + /** Extract just the paths for the manifest (lockfile needs full PatchFile) */ + manifest.pnpm.patchedDependencies = Object.fromEntries( + Object.entries(copiedPatches).map(([spec, patchFile]) => [ + spec, + patchFile.path, + ]) + ); log.debug( `Added ${Object.keys(copiedPatches).length} patches to isolated package.json` ); diff --git a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts index 2f5a02c..bc7f655 100644 --- a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts +++ b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts @@ -14,12 +14,8 @@ import { pruneLockfile as pruneLockfile_v8 } from "pnpm_prune_lockfile_v8"; import { pruneLockfile as pruneLockfile_v9 } from "pnpm_prune_lockfile_v9"; import { pick } from "remeda"; import { useLogger } from "~/lib/logger"; -import type { PackageManifest, PackagesRegistry } from "~/lib/types"; -import { - filterPatchedDependencies, - getErrorMessage, - isRushWorkspace, -} from "~/lib/utils"; +import type { PackageManifest, PackagesRegistry, PatchFile } from "~/lib/types"; +import { getErrorMessage, isRushWorkspace } from "~/lib/utils"; import { pnpmMapImporter } from "./pnpm-map-importer"; export async function generatePnpmLockfile({ @@ -31,7 +27,7 @@ export async function generatePnpmLockfile({ targetPackageManifest, majorVersion, includeDevDependencies, - includePatchedDependencies, + patchedDependencies, }: { workspaceRootDir: string; targetPackageDir: string; @@ -41,7 +37,8 @@ export async function generatePnpmLockfile({ targetPackageManifest: PackageManifest; majorVersion: number; includeDevDependencies: boolean; - includePatchedDependencies: boolean; + /** Pre-computed patched dependencies with transformed paths from copyPatches */ + patchedDependencies?: Record; }) { /** * For now we will assume that the lockfile format might not change in the @@ -136,7 +133,6 @@ export async function generatePnpmLockfile({ ".", pnpmMapImporter(".", importer!, { includeDevDependencies, - includePatchedDependencies, directoryByPackageName, }), ]; @@ -147,8 +143,7 @@ export async function generatePnpmLockfile({ return [ importerId, pnpmMapImporter(importerId, importer!, { - includeDevDependencies: false, // Only include dev deps for target package - includePatchedDependencies, + includeDevDependencies: false, directoryByPackageName, }), ]; @@ -167,20 +162,10 @@ export async function generatePnpmLockfile({ } /** - * Filter patched dependencies to only include patches for packages that - * will actually be present in the isolated lockfile based on dependency - * type. We read patchedDependencies from workspace root lockfile, but - * filter based on target package dependencies. + * Use pre-computed patched dependencies with transformed paths. The paths + * are already adapted by copyPatches to match the isolated directory + * structure (flattened to patches/ with collision avoidance). */ - const patchedDependencies = includePatchedDependencies - ? filterPatchedDependencies( - lockfile.patchedDependencies, - targetPackageManifest, - includeDevDependencies, - log - ) - : undefined; - if (useVersion9) { await writeWantedLockfile_v9(isolateDir, { ...prunedLockfile, diff --git a/src/lib/lockfile/helpers/pnpm-map-importer.ts b/src/lib/lockfile/helpers/pnpm-map-importer.ts index 11d484c..9fae56f 100644 --- a/src/lib/lockfile/helpers/pnpm-map-importer.ts +++ b/src/lib/lockfile/helpers/pnpm-map-importer.ts @@ -16,7 +16,6 @@ export function pnpmMapImporter( directoryByPackageName, }: { includeDevDependencies: boolean; - includePatchedDependencies: boolean; directoryByPackageName: { [packageName: string]: string }; } ): ProjectSnapshot { diff --git a/src/lib/lockfile/process-lockfile.ts b/src/lib/lockfile/process-lockfile.ts index bd057c0..445c5a4 100644 --- a/src/lib/lockfile/process-lockfile.ts +++ b/src/lib/lockfile/process-lockfile.ts @@ -1,7 +1,7 @@ import type { IsolateConfigResolved } from "../config"; import { useLogger } from "../logger"; import { usePackageManager } from "../package-manager"; -import type { PackageManifest, PackagesRegistry } from "../types"; +import type { PackageManifest, PackagesRegistry, PatchFile } from "../types"; import { generateNpmLockfile, generatePnpmLockfile, @@ -22,6 +22,7 @@ export async function processLockfile({ internalDepPackageNames, targetPackageDir, targetPackageManifest, + patchedDependencies, config, }: { workspaceRootDir: string; @@ -31,6 +32,8 @@ export async function processLockfile({ targetPackageDir: string; targetPackageName: string; targetPackageManifest: PackageManifest; + /** Pre-computed patched dependencies with transformed paths from copyPatches */ + patchedDependencies?: Record; config: IsolateConfigResolved; }) { const log = useLogger(); @@ -89,7 +92,7 @@ export async function processLockfile({ targetPackageManifest, majorVersion, includeDevDependencies: config.includeDevDependencies, - includePatchedDependencies: config.includePatchedDependencies, + patchedDependencies, }); break; } diff --git a/src/lib/patches/copy-patches.test.ts b/src/lib/patches/copy-patches.test.ts index 0eba9fa..626e2d5 100644 --- a/src/lib/patches/copy-patches.test.ts +++ b/src/lib/patches/copy-patches.test.ts @@ -142,7 +142,7 @@ describe("copyPatches", () => { }); expect(result).toEqual({ - "lodash@4.17.21": "patches/lodash.patch", + "lodash@4.17.21": { path: "patches/lodash.patch", hash: "" }, }); expect(fs.ensureDir).toHaveBeenCalledWith("/workspace/isolate/patches"); expect(fs.copy).toHaveBeenCalledWith( @@ -183,7 +183,7 @@ describe("copyPatches", () => { }); expect(result).toEqual({ - "vitest@1.0.0": "patches/vitest.patch", + "vitest@1.0.0": { path: "patches/vitest.patch", hash: "" }, }); expect(filterPatchedDependencies).toHaveBeenCalledWith( { "vitest@1.0.0": "patches/vitest.patch" }, @@ -260,7 +260,7 @@ describe("copyPatches", () => { }); expect(result).toEqual({ - "@firebase/app@1.2.3": "patches/firebase-app.patch", + "@firebase/app@1.2.3": { path: "patches/firebase-app.patch", hash: "" }, }); }); @@ -298,8 +298,8 @@ describe("copyPatches", () => { }); expect(result).toEqual({ - "pkg-a@1.0.0": "patches/fix.patch", - "pkg-b@1.0.0": "patches/fix-1.patch", + "pkg-a@1.0.0": { path: "patches/fix.patch", hash: "" }, + "pkg-b@1.0.0": { path: "patches/fix-1.patch", hash: "" }, }); expect(fs.copy).toHaveBeenCalledTimes(2); }); @@ -337,7 +337,7 @@ describe("copyPatches", () => { /** The path should be flattened to patches/ directory */ expect(result).toEqual({ - "lodash@4.17.21": "patches/lodash.patch", + "lodash@4.17.21": { path: "patches/lodash.patch", hash: "" }, }); expect(fs.copy).toHaveBeenCalledWith( "/workspace/some/nested/path/lodash.patch", @@ -382,8 +382,8 @@ describe("copyPatches", () => { }); expect(result).toEqual({ - "lodash@4.17.21": "patches/lodash.patch", - "@firebase/app@1.2.3": "patches/firebase-app.patch", + "lodash@4.17.21": { path: "patches/lodash.patch", hash: "" }, + "@firebase/app@1.2.3": { path: "patches/firebase-app.patch", hash: "" }, }); expect(fs.copy).toHaveBeenCalledTimes(2); }); diff --git a/src/lib/patches/copy-patches.ts b/src/lib/patches/copy-patches.ts index 574d682..82b5a68 100644 --- a/src/lib/patches/copy-patches.ts +++ b/src/lib/patches/copy-patches.ts @@ -1,11 +1,19 @@ import fs from "fs-extra"; import path from "node:path"; +import { + readWantedLockfile as readWantedLockfile_v8, +} from "pnpm_lockfile_file_v8"; +import { + readWantedLockfile as readWantedLockfile_v9, +} from "pnpm_lockfile_file_v9"; import { useLogger } from "~/lib/logger"; -import type { PackageManifest } from "~/lib/types"; +import { usePackageManager } from "~/lib/package-manager"; +import type { PackageManifest, PatchFile } from "~/lib/types"; import { filterPatchedDependencies, getIsolateRelativeLogPath, getRootRelativeLogPath, + isRushWorkspace, readTypedJson, } from "~/lib/utils"; @@ -21,7 +29,7 @@ export async function copyPatches({ isolateDir: string; includePatchedDependencies: boolean; includeDevDependencies: boolean; -}): Promise> { +}): Promise> { const log = useLogger(); if (!includePatchedDependencies) { @@ -63,10 +71,15 @@ export async function copyPatches({ return {}; } + /** Read the lockfile to get the hashes for each patch */ + const lockfilePatchedDependencies = await readLockfilePatchedDependencies( + workspaceRootDir + ); + const patchesDir = path.join(isolateDir, "patches"); await fs.ensureDir(patchesDir); - const copiedPatches: Record = {}; + const copiedPatches: Record = {}; const usedFilenames = new Set(); for (const [packageSpec, patchPath] of Object.entries(filteredPatches)) { @@ -107,7 +120,18 @@ export async function copyPatches({ await fs.copy(sourcePatchPath, targetPatchPath); log.debug(`Copied patch for ${packageSpec}: ${targetFilename}`); - copiedPatches[packageSpec] = `patches/${targetFilename}`; + /** Get the hash from the original lockfile, or use empty string if not found */ + const originalPatchFile = lockfilePatchedDependencies?.[packageSpec]; + const hash = originalPatchFile?.hash ?? ""; + + if (!hash) { + log.warn(`No hash found for patch ${packageSpec} in lockfile`); + } + + copiedPatches[packageSpec] = { + path: `patches/${targetFilename}`, + hash, + }; } if (Object.keys(copiedPatches).length > 0) { @@ -118,3 +142,30 @@ export async function copyPatches({ return copiedPatches; } + +/** + * Read the patchedDependencies from the original lockfile to get the hashes. + * Since the file content is the same after copying, the hash remains valid. + */ +async function readLockfilePatchedDependencies( + workspaceRootDir: string +): Promise | undefined> { + try { + const { majorVersion } = usePackageManager(); + const useVersion9 = majorVersion >= 9; + const isRush = isRushWorkspace(workspaceRootDir); + + const lockfileDir = isRush + ? path.join(workspaceRootDir, "common/config/rush") + : workspaceRootDir; + + const lockfile = useVersion9 + ? await readWantedLockfile_v9(lockfileDir, { ignoreIncompatible: false }) + : await readWantedLockfile_v8(lockfileDir, { ignoreIncompatible: false }); + + return lockfile?.patchedDependencies; + } catch { + /** Package manager not detected or lockfile not readable */ + return undefined; + } +} diff --git a/src/lib/types.ts b/src/lib/types.ts index d6b558b..9e6a464 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -1,5 +1,14 @@ import type { PackageManifest as PnpmPackageManifest } from "@pnpm/types"; +/** + * Represents a patch file entry in the pnpm lockfile. + * Contains the path to the patch file and its content hash. + */ +export interface PatchFile { + path: string; + hash: string; +} + export type PackageManifest = PnpmPackageManifest & { packageManager?: string; pnpm?: { From 923da2e79cfd484f449da4e9521f451b747e5e36 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:19:55 +0100 Subject: [PATCH 14/24] 1.27.0-1 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 4133af7..c91c0e1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "isolate-package", - "version": "1.27.0-0", + "version": "1.27.0-1", "description": "Isolate a monorepo package with its shared dependencies to form a self-contained directory, compatible with Firebase deploy", "author": "Thijs Koerselman", "license": "MIT", From 52118ecdb787d40f030163b65f50dfba9aea49c1 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:21:32 +0100 Subject: [PATCH 15/24] Format code --- src/isolate.ts | 9 ++++++--- src/lib/patches/copy-patches.ts | 13 ++++--------- src/lib/types.ts | 4 ++-- 3 files changed, 12 insertions(+), 14 deletions(-) diff --git a/src/isolate.ts b/src/isolate.ts index 66a1158..8fee9c9 100644 --- a/src/isolate.ts +++ b/src/isolate.ts @@ -201,8 +201,8 @@ export function createIsolator(config?: IsolateConfig) { await writeManifest(isolateDir, outputManifest); /** - * Copy patch files before generating lockfile so the lockfile contains - * the correct transformed paths (flattened to patches/ with collision + * Copy patch files before generating lockfile so the lockfile contains the + * correct transformed paths (flattened to patches/ with collision * avoidance). */ const copiedPatches = await copyPatches({ @@ -237,7 +237,10 @@ export function createIsolator(config?: IsolateConfig) { if (!manifest.pnpm) { manifest.pnpm = {}; } - /** Extract just the paths for the manifest (lockfile needs full PatchFile) */ + /** + * Extract just the paths for the manifest (lockfile needs full + * PatchFile) + */ manifest.pnpm.patchedDependencies = Object.fromEntries( Object.entries(copiedPatches).map(([spec, patchFile]) => [ spec, diff --git a/src/lib/patches/copy-patches.ts b/src/lib/patches/copy-patches.ts index 82b5a68..58b4645 100644 --- a/src/lib/patches/copy-patches.ts +++ b/src/lib/patches/copy-patches.ts @@ -1,11 +1,7 @@ import fs from "fs-extra"; import path from "node:path"; -import { - readWantedLockfile as readWantedLockfile_v8, -} from "pnpm_lockfile_file_v8"; -import { - readWantedLockfile as readWantedLockfile_v9, -} from "pnpm_lockfile_file_v9"; +import { readWantedLockfile as readWantedLockfile_v8 } from "pnpm_lockfile_file_v8"; +import { readWantedLockfile as readWantedLockfile_v9 } from "pnpm_lockfile_file_v9"; import { useLogger } from "~/lib/logger"; import { usePackageManager } from "~/lib/package-manager"; import type { PackageManifest, PatchFile } from "~/lib/types"; @@ -72,9 +68,8 @@ export async function copyPatches({ } /** Read the lockfile to get the hashes for each patch */ - const lockfilePatchedDependencies = await readLockfilePatchedDependencies( - workspaceRootDir - ); + const lockfilePatchedDependencies = + await readLockfilePatchedDependencies(workspaceRootDir); const patchesDir = path.join(isolateDir, "patches"); await fs.ensureDir(patchesDir); diff --git a/src/lib/types.ts b/src/lib/types.ts index 9e6a464..5ea3003 100644 --- a/src/lib/types.ts +++ b/src/lib/types.ts @@ -1,8 +1,8 @@ import type { PackageManifest as PnpmPackageManifest } from "@pnpm/types"; /** - * Represents a patch file entry in the pnpm lockfile. - * Contains the path to the patch file and its content hash. + * Represents a patch file entry in the pnpm lockfile. Contains the path to the + * patch file and its content hash. */ export interface PatchFile { path: string; From 8b4702e47ef7976db3228e1012d9aa18a98bc1d6 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:27:16 +0100 Subject: [PATCH 16/24] Rename misleading test for malformed scoped package --- src/lib/utils/get-package-name.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/lib/utils/get-package-name.test.ts b/src/lib/utils/get-package-name.test.ts index 2805111..5d17afe 100644 --- a/src/lib/utils/get-package-name.test.ts +++ b/src/lib/utils/get-package-name.test.ts @@ -15,7 +15,8 @@ describe("getPackageName", () => { expect(getPackageName("@firebase/app")).toBe("@firebase/app"); }); - it("should handle deeply nested scoped package", () => { + it("should handle malformed scoped package with extra slashes", () => { + /** This is malformed input - real scoped packages only support @scope/name */ expect(getPackageName("@org/sub/package@1.0.0")).toBe("@org/sub/package"); }); }); From 8ad26f5a958f596705e98b2e373f9bf1b98e540f Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:28:08 +0100 Subject: [PATCH 17/24] Add explicit mocks for package manager and lockfile readers in tests --- src/lib/patches/copy-patches.test.ts | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/src/lib/patches/copy-patches.test.ts b/src/lib/patches/copy-patches.test.ts index 626e2d5..6cf3f97 100644 --- a/src/lib/patches/copy-patches.test.ts +++ b/src/lib/patches/copy-patches.test.ts @@ -16,9 +16,24 @@ vi.mock("~/lib/utils", () => ({ filterPatchedDependencies: vi.fn(), getIsolateRelativeLogPath: vi.fn((p: string) => p), getRootRelativeLogPath: vi.fn((p: string) => p), + isRushWorkspace: vi.fn(() => false), readTypedJson: vi.fn(), })); +/** Mock the package manager */ +vi.mock("~/lib/package-manager", () => ({ + usePackageManager: vi.fn(() => ({ majorVersion: 9 })), +})); + +/** Mock the pnpm lockfile readers */ +vi.mock("pnpm_lockfile_file_v8", () => ({ + readWantedLockfile: vi.fn(() => Promise.resolve(null)), +})); + +vi.mock("pnpm_lockfile_file_v9", () => ({ + readWantedLockfile: vi.fn(() => Promise.resolve(null)), +})); + /** Mock the logger */ vi.mock("~/lib/logger", () => ({ useLogger: () => ({ From 9b2beda181943ae3d4a65944f65b3cb7214c4a41 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:39:07 +0100 Subject: [PATCH 18/24] Refactor filterPatchedDependencies to use object parameter and internal logger --- src/lib/patches/copy-patches.test.ts | 11 +- src/lib/patches/copy-patches.ts | 5 +- .../utils/filter-patched-dependencies.test.ts | 130 +++++++++++------- src/lib/utils/filter-patched-dependencies.ts | 18 ++- 4 files changed, 97 insertions(+), 67 deletions(-) diff --git a/src/lib/patches/copy-patches.test.ts b/src/lib/patches/copy-patches.test.ts index 6cf3f97..b4f4c25 100644 --- a/src/lib/patches/copy-patches.test.ts +++ b/src/lib/patches/copy-patches.test.ts @@ -200,12 +200,11 @@ describe("copyPatches", () => { expect(result).toEqual({ "vitest@1.0.0": { path: "patches/vitest.patch", hash: "" }, }); - expect(filterPatchedDependencies).toHaveBeenCalledWith( - { "vitest@1.0.0": "patches/vitest.patch" }, - targetManifest, - true, - expect.any(Object) - ); + expect(filterPatchedDependencies).toHaveBeenCalledWith({ + patchedDependencies: { "vitest@1.0.0": "patches/vitest.patch" }, + targetPackageManifest: targetManifest, + includeDevDependencies: true, + }); }); it("should skip missing patch files and log a warning", async () => { diff --git a/src/lib/patches/copy-patches.ts b/src/lib/patches/copy-patches.ts index 58b4645..ac1a8e8 100644 --- a/src/lib/patches/copy-patches.ts +++ b/src/lib/patches/copy-patches.ts @@ -56,12 +56,11 @@ export async function copyPatches({ `Found ${Object.keys(patchedDependencies).length} patched dependencies in workspace` ); - const filteredPatches = filterPatchedDependencies( + const filteredPatches = filterPatchedDependencies({ patchedDependencies, targetPackageManifest, includeDevDependencies, - log - ); + }); if (!filteredPatches) { return {}; diff --git a/src/lib/utils/filter-patched-dependencies.test.ts b/src/lib/utils/filter-patched-dependencies.test.ts index 8542379..f9a738b 100644 --- a/src/lib/utils/filter-patched-dependencies.test.ts +++ b/src/lib/utils/filter-patched-dependencies.test.ts @@ -1,21 +1,26 @@ import { describe, expect, it, vi } from "vitest"; -import type { Logger } from "~/lib/logger"; import type { PackageManifest } from "~/lib/types"; import { filterPatchedDependencies } from "./filter-patched-dependencies"; -const createMockLogger = (): Logger => ({ - debug: vi.fn(), - info: vi.fn(), - warn: vi.fn(), - error: vi.fn(), -}); +/** Mock the logger */ +vi.mock("~/lib/logger", () => ({ + useLogger: () => ({ + debug: vi.fn(), + info: vi.fn(), + warn: vi.fn(), + error: vi.fn(), + }), +})); describe("filterPatchedDependencies", () => { it("should return undefined when patchedDependencies is undefined", () => { const manifest: PackageManifest = { name: "test", version: "1.0.0" }; - const log = createMockLogger(); - const result = filterPatchedDependencies(undefined, manifest, false, log); + const result = filterPatchedDependencies({ + patchedDependencies: undefined, + targetPackageManifest: manifest, + includeDevDependencies: false, + }); expect(result).toBeUndefined(); }); @@ -26,9 +31,12 @@ describe("filterPatchedDependencies", () => { version: "1.0.0", dependencies: { lodash: "^4.0.0" }, }; - const log = createMockLogger(); - const result = filterPatchedDependencies({}, manifest, false, log); + const result = filterPatchedDependencies({ + patchedDependencies: {}, + targetPackageManifest: manifest, + includeDevDependencies: false, + }); expect(result).toBeUndefined(); }); @@ -39,10 +47,12 @@ describe("filterPatchedDependencies", () => { version: "1.0.0", dependencies: { lodash: "^4.0.0" }, }; - const patches = { "lodash@4.17.21": "patches/lodash.patch" }; - const log = createMockLogger(); - const result = filterPatchedDependencies(patches, manifest, false, log); + const result = filterPatchedDependencies({ + patchedDependencies: { "lodash@4.17.21": "patches/lodash.patch" }, + targetPackageManifest: manifest, + includeDevDependencies: false, + }); expect(result).toEqual({ "lodash@4.17.21": "patches/lodash.patch" }); }); @@ -53,10 +63,12 @@ describe("filterPatchedDependencies", () => { version: "1.0.0", devDependencies: { vitest: "^1.0.0" }, }; - const patches = { "vitest@1.0.0": "patches/vitest.patch" }; - const log = createMockLogger(); - const result = filterPatchedDependencies(patches, manifest, true, log); + const result = filterPatchedDependencies({ + patchedDependencies: { "vitest@1.0.0": "patches/vitest.patch" }, + targetPackageManifest: manifest, + includeDevDependencies: true, + }); expect(result).toEqual({ "vitest@1.0.0": "patches/vitest.patch" }); }); @@ -67,10 +79,12 @@ describe("filterPatchedDependencies", () => { version: "1.0.0", devDependencies: { vitest: "^1.0.0" }, }; - const patches = { "vitest@1.0.0": "patches/vitest.patch" }; - const log = createMockLogger(); - const result = filterPatchedDependencies(patches, manifest, false, log); + const result = filterPatchedDependencies({ + patchedDependencies: { "vitest@1.0.0": "patches/vitest.patch" }, + targetPackageManifest: manifest, + includeDevDependencies: false, + }); expect(result).toBeUndefined(); }); @@ -81,10 +95,12 @@ describe("filterPatchedDependencies", () => { version: "1.0.0", dependencies: { lodash: "^4.0.0" }, }; - const patches = { "other-package@1.0.0": "patches/other.patch" }; - const log = createMockLogger(); - const result = filterPatchedDependencies(patches, manifest, false, log); + const result = filterPatchedDependencies({ + patchedDependencies: { "other-package@1.0.0": "patches/other.patch" }, + targetPackageManifest: manifest, + includeDevDependencies: false, + }); expect(result).toBeUndefined(); }); @@ -95,10 +111,14 @@ describe("filterPatchedDependencies", () => { version: "1.0.0", dependencies: { "@firebase/app": "^1.0.0" }, }; - const patches = { "@firebase/app@1.2.3": "patches/firebase-app.patch" }; - const log = createMockLogger(); - const result = filterPatchedDependencies(patches, manifest, false, log); + const result = filterPatchedDependencies({ + patchedDependencies: { + "@firebase/app@1.2.3": "patches/firebase-app.patch", + }, + targetPackageManifest: manifest, + includeDevDependencies: false, + }); expect(result).toEqual({ "@firebase/app@1.2.3": "patches/firebase-app.patch", @@ -112,15 +132,17 @@ describe("filterPatchedDependencies", () => { dependencies: { lodash: "^4.0.0", "@firebase/app": "^1.0.0" }, devDependencies: { vitest: "^1.0.0" }, }; - const patches = { - "lodash@4.17.21": "patches/lodash.patch", - "@firebase/app@1.2.3": "patches/firebase-app.patch", - "vitest@1.0.0": "patches/vitest.patch", - "unknown@1.0.0": "patches/unknown.patch", - }; - const log = createMockLogger(); - const result = filterPatchedDependencies(patches, manifest, false, log); + const result = filterPatchedDependencies({ + patchedDependencies: { + "lodash@4.17.21": "patches/lodash.patch", + "@firebase/app@1.2.3": "patches/firebase-app.patch", + "vitest@1.0.0": "patches/vitest.patch", + "unknown@1.0.0": "patches/unknown.patch", + }, + targetPackageManifest: manifest, + includeDevDependencies: false, + }); expect(result).toEqual({ "lodash@4.17.21": "patches/lodash.patch", @@ -135,13 +157,15 @@ describe("filterPatchedDependencies", () => { dependencies: { lodash: "^4.0.0" }, devDependencies: { vitest: "^1.0.0" }, }; - const patches = { - "lodash@4.17.21": "patches/lodash.patch", - "vitest@1.0.0": "patches/vitest.patch", - }; - const log = createMockLogger(); - const result = filterPatchedDependencies(patches, manifest, true, log); + const result = filterPatchedDependencies({ + patchedDependencies: { + "lodash@4.17.21": "patches/lodash.patch", + "vitest@1.0.0": "patches/vitest.patch", + }, + targetPackageManifest: manifest, + includeDevDependencies: true, + }); expect(result).toEqual({ "lodash@4.17.21": "patches/lodash.patch", @@ -155,13 +179,15 @@ describe("filterPatchedDependencies", () => { version: "1.0.0", dependencies: { lodash: "^4.0.0" }, }; - const patches = { - "unknown-a@1.0.0": "patches/a.patch", - "unknown-b@2.0.0": "patches/b.patch", - }; - const log = createMockLogger(); - const result = filterPatchedDependencies(patches, manifest, false, log); + const result = filterPatchedDependencies({ + patchedDependencies: { + "unknown-a@1.0.0": "patches/a.patch", + "unknown-b@2.0.0": "patches/b.patch", + }, + targetPackageManifest: manifest, + includeDevDependencies: false, + }); expect(result).toBeUndefined(); }); @@ -172,12 +198,14 @@ describe("filterPatchedDependencies", () => { version: "1.0.0", dependencies: { lodash: "^4.0.0" }, }; - const patches = { - "lodash@4.17.21": { path: "patches/lodash.patch", hash: "abc123" }, - }; - const log = createMockLogger(); - const result = filterPatchedDependencies(patches, manifest, false, log); + const result = filterPatchedDependencies({ + patchedDependencies: { + "lodash@4.17.21": { path: "patches/lodash.patch", hash: "abc123" }, + }, + targetPackageManifest: manifest, + includeDevDependencies: false, + }); expect(result).toEqual({ "lodash@4.17.21": { path: "patches/lodash.patch", hash: "abc123" }, diff --git a/src/lib/utils/filter-patched-dependencies.ts b/src/lib/utils/filter-patched-dependencies.ts index 7457697..8ceb70c 100644 --- a/src/lib/utils/filter-patched-dependencies.ts +++ b/src/lib/utils/filter-patched-dependencies.ts @@ -1,4 +1,4 @@ -import type { Logger } from "~/lib/logger"; +import { useLogger } from "~/lib/logger"; import type { PackageManifest } from "~/lib/types"; import { getPackageName } from "./get-package-name"; @@ -6,12 +6,16 @@ import { getPackageName } from "./get-package-name"; * Filters patched dependencies to only include patches for packages that are * present in the target package's dependencies based on dependency type. */ -export function filterPatchedDependencies( - patchedDependencies: Record | undefined, - targetPackageManifest: PackageManifest, - includeDevDependencies: boolean, - log: Logger -): Record | undefined { +export function filterPatchedDependencies({ + patchedDependencies, + targetPackageManifest, + includeDevDependencies, +}: { + patchedDependencies: Record | undefined; + targetPackageManifest: PackageManifest; + includeDevDependencies: boolean; +}): Record | undefined { + const log = useLogger(); if (!patchedDependencies || typeof patchedDependencies !== "object") { return undefined; } From 6041e9b4b7565cd89b3b09470972f28175ef2100 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:43:55 +0100 Subject: [PATCH 19/24] Remove includePatchedDependencies config option Patches are now automatically copied if they exist in the workspace. There's no need for an explicit opt-in setting. --- src/isolate.ts | 1 - src/lib/config.ts | 2 -- src/lib/patches/copy-patches.test.ts | 23 ----------------------- src/lib/patches/copy-patches.ts | 7 ------- 4 files changed, 33 deletions(-) diff --git a/src/isolate.ts b/src/isolate.ts index 8fee9c9..7b19d4c 100644 --- a/src/isolate.ts +++ b/src/isolate.ts @@ -209,7 +209,6 @@ export function createIsolator(config?: IsolateConfig) { workspaceRootDir, targetPackageManifest: outputManifest, isolateDir, - includePatchedDependencies: config.includePatchedDependencies, includeDevDependencies: config.includeDevDependencies, }); diff --git a/src/lib/config.ts b/src/lib/config.ts index d8f5f07..2128ba2 100644 --- a/src/lib/config.ts +++ b/src/lib/config.ts @@ -7,7 +7,6 @@ import { inspectValue, readTypedJsonSync } from "./utils"; export type IsolateConfigResolved = { buildDirName?: string; includeDevDependencies: boolean; - includePatchedDependencies: boolean; isolateDirName: string; logLevel: LogLevel; targetPackagePath?: string; @@ -25,7 +24,6 @@ export type IsolateConfig = Partial; const configDefaults: IsolateConfigResolved = { buildDirName: undefined, includeDevDependencies: false, - includePatchedDependencies: false, isolateDirName: "isolate", logLevel: "info", targetPackagePath: undefined, diff --git a/src/lib/patches/copy-patches.test.ts b/src/lib/patches/copy-patches.test.ts index b4f4c25..9e3c821 100644 --- a/src/lib/patches/copy-patches.test.ts +++ b/src/lib/patches/copy-patches.test.ts @@ -58,19 +58,6 @@ describe("copyPatches", () => { vi.restoreAllMocks(); }); - it("should return empty object when includePatchedDependencies is false", async () => { - const result = await copyPatches({ - workspaceRootDir: "/workspace", - targetPackageManifest: { name: "test", version: "1.0.0" }, - isolateDir: "/workspace/isolate", - includePatchedDependencies: false, - includeDevDependencies: false, - }); - - expect(result).toEqual({}); - expect(readTypedJson).not.toHaveBeenCalled(); - }); - it("should return empty object when workspace root package.json cannot be read", async () => { readTypedJson.mockRejectedValue(new Error("File not found")); @@ -78,7 +65,6 @@ describe("copyPatches", () => { workspaceRootDir: "/workspace", targetPackageManifest: { name: "test", version: "1.0.0" }, isolateDir: "/workspace/isolate", - includePatchedDependencies: true, includeDevDependencies: false, }); @@ -95,7 +81,6 @@ describe("copyPatches", () => { workspaceRootDir: "/workspace", targetPackageManifest: { name: "test", version: "1.0.0" }, isolateDir: "/workspace/isolate", - includePatchedDependencies: true, includeDevDependencies: false, }); @@ -118,7 +103,6 @@ describe("copyPatches", () => { workspaceRootDir: "/workspace", targetPackageManifest: { name: "test", version: "1.0.0" }, isolateDir: "/workspace/isolate", - includePatchedDependencies: true, includeDevDependencies: false, }); @@ -152,7 +136,6 @@ describe("copyPatches", () => { workspaceRootDir: "/workspace", targetPackageManifest: targetManifest, isolateDir: "/workspace/isolate", - includePatchedDependencies: true, includeDevDependencies: false, }); @@ -193,7 +176,6 @@ describe("copyPatches", () => { workspaceRootDir: "/workspace", targetPackageManifest: targetManifest, isolateDir: "/workspace/isolate", - includePatchedDependencies: true, includeDevDependencies: true, }); @@ -234,7 +216,6 @@ describe("copyPatches", () => { workspaceRootDir: "/workspace", targetPackageManifest: targetManifest, isolateDir: "/workspace/isolate", - includePatchedDependencies: true, includeDevDependencies: false, }); @@ -269,7 +250,6 @@ describe("copyPatches", () => { workspaceRootDir: "/workspace", targetPackageManifest: targetManifest, isolateDir: "/workspace/isolate", - includePatchedDependencies: true, includeDevDependencies: false, }); @@ -307,7 +287,6 @@ describe("copyPatches", () => { workspaceRootDir: "/workspace", targetPackageManifest: targetManifest, isolateDir: "/workspace/isolate", - includePatchedDependencies: true, includeDevDependencies: false, }); @@ -345,7 +324,6 @@ describe("copyPatches", () => { workspaceRootDir: "/workspace", targetPackageManifest: targetManifest, isolateDir: "/workspace/isolate", - includePatchedDependencies: true, includeDevDependencies: false, }); @@ -391,7 +369,6 @@ describe("copyPatches", () => { workspaceRootDir: "/workspace", targetPackageManifest: targetManifest, isolateDir: "/workspace/isolate", - includePatchedDependencies: true, includeDevDependencies: false, }); diff --git a/src/lib/patches/copy-patches.ts b/src/lib/patches/copy-patches.ts index ac1a8e8..f6df271 100644 --- a/src/lib/patches/copy-patches.ts +++ b/src/lib/patches/copy-patches.ts @@ -17,22 +17,15 @@ export async function copyPatches({ workspaceRootDir, targetPackageManifest, isolateDir, - includePatchedDependencies, includeDevDependencies, }: { workspaceRootDir: string; targetPackageManifest: PackageManifest; isolateDir: string; - includePatchedDependencies: boolean; includeDevDependencies: boolean; }): Promise> { const log = useLogger(); - if (!includePatchedDependencies) { - log.debug("Skipping patch copying (includePatchedDependencies is false)"); - return {}; - } - let workspaceRootManifest: PackageManifest; try { workspaceRootManifest = await readTypedJson( From af0855107981ed8c3d4c15b22fb43f84f2ee84bf Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:50:49 +0100 Subject: [PATCH 20/24] Simplify patch copying to preserve original folder structure Instead of flattening all patches to patches/ with collision avoidance, preserve the original directory structure when copying patch files. This eliminates unnecessary complexity: - Remove collision detection and filename renaming logic - Paths in patchedDependencies remain unchanged from workspace root - Simpler code, no edge cases for naming conflicts --- .claude/CLAUDE.md | 11 ++++++-- README.md | 19 +++++++++++++ src/lib/patches/copy-patches.test.ts | 31 ++++++++++++++++----- src/lib/patches/copy-patches.ts | 41 ++++------------------------ 4 files changed, 58 insertions(+), 44 deletions(-) diff --git a/.claude/CLAUDE.md b/.claude/CLAUDE.md index 9c57b34..065a2ed 100644 --- a/.claude/CLAUDE.md +++ b/.claude/CLAUDE.md @@ -64,12 +64,18 @@ by name. - `unpack-dependencies.ts` - Extracts packed tarballs to isolate directory - `process-build-output-files.ts` - Copies target package build output +**patches/** - Handles PNPM patched dependencies: + +- `copy-patches.ts` - Copies relevant patch files from workspace root to isolate + directory, filtering based on target package dependencies + ### Key Types (`src/lib/types.ts`) - `PackageManifest` - Extended pnpm package manifest type - `PackagesRegistry` - Maps package names to their paths and manifests - `WorkspacePackageInfo` - Package metadata (absoluteDir, rootRelativeDir, manifest) +- `PatchFile` - Represents a patch file entry with path and hash ### Process Flow @@ -79,8 +85,9 @@ by name. 4. Recursively find all internal dependencies 5. Pack and unpack internal dependencies to isolate directory 6. Adapt manifests to use `file:` references -7. Generate pruned lockfile for the isolated package -8. Copy workspace config files (.npmrc, pnpm-workspace.yaml) +7. Copy PNPM patched dependencies (if any exist) +8. Generate pruned lockfile for the isolated package +9. Copy workspace config files (.npmrc, pnpm-workspace.yaml) ## Path Alias diff --git a/README.md b/README.md index fae8a89..c9e45ed 100644 --- a/README.md +++ b/README.md @@ -7,6 +7,7 @@ - [Troubleshooting](#troubleshooting) - [Prerequisites](#prerequisites) - [Configuration Options](#configuration-options) +- [PNPM Patched Dependencies](#pnpm-patched-dependencies) - [API](#api) - [The internal packages strategy](#the-internal-packages-strategy) - [Firebase](#firebase) @@ -34,6 +35,7 @@ integrated, check out [mono-ts](https://github.com/0x80/mono-ts) - Optionally force output to use NPM with matching versions - Optionally include devDependencies in the isolated output - Optionally pick or omit scripts from the manifest +- Automatically copies PNPM patched dependencies to the isolated output - Compatible with the Firebase tools CLI, including 1st and 2nd generation Firebase Functions. For more information see [the Firebase instructions](./docs/firebase.md). @@ -325,6 +327,23 @@ services When you use the `targetPackagePath` option, this setting will be ignored. +## PNPM Patched Dependencies + +If your workspace uses PNPM's +[patched dependencies](https://pnpm.io/cli/patch) feature, `isolate` will +automatically copy the relevant patch files to the isolated output. + +Patches are filtered based on the target package's dependencies: + +- Patches for production dependencies are always included +- Patches for dev dependencies are only included when `includeDevDependencies` + is enabled +- Patches for packages not in the target's dependency tree are excluded + +The patch files are copied to the isolated output, preserving their original +directory structure. Both the `package.json` and `pnpm-lock.yaml` are updated +with the correct paths. + ## API Alternatively, `isolate` can be integrated in other programs by importing it as diff --git a/src/lib/patches/copy-patches.test.ts b/src/lib/patches/copy-patches.test.ts index 9e3c821..7e3e7e4 100644 --- a/src/lib/patches/copy-patches.test.ts +++ b/src/lib/patches/copy-patches.test.ts @@ -142,6 +142,7 @@ describe("copyPatches", () => { expect(result).toEqual({ "lodash@4.17.21": { path: "patches/lodash.patch", hash: "" }, }); + /** Should preserve original folder structure */ expect(fs.ensureDir).toHaveBeenCalledWith("/workspace/isolate/patches"); expect(fs.copy).toHaveBeenCalledWith( "/workspace/patches/lodash.patch", @@ -187,6 +188,10 @@ describe("copyPatches", () => { targetPackageManifest: targetManifest, includeDevDependencies: true, }); + expect(fs.copy).toHaveBeenCalledWith( + "/workspace/patches/vitest.patch", + "/workspace/isolate/patches/vitest.patch" + ); }); it("should skip missing patch files and log a warning", async () => { @@ -258,7 +263,7 @@ describe("copyPatches", () => { }); }); - it("should handle filename collisions by renaming", async () => { + it("should preserve nested folder structure when copying patches", async () => { const targetManifest: PackageManifest = { name: "test", version: "1.0.0", @@ -290,14 +295,23 @@ describe("copyPatches", () => { includeDevDependencies: false, }); + /** Should preserve original paths without renaming */ expect(result).toEqual({ - "pkg-a@1.0.0": { path: "patches/fix.patch", hash: "" }, - "pkg-b@1.0.0": { path: "patches/fix-1.patch", hash: "" }, + "pkg-a@1.0.0": { path: "patches/v1/fix.patch", hash: "" }, + "pkg-b@1.0.0": { path: "patches/v2/fix.patch", hash: "" }, }); expect(fs.copy).toHaveBeenCalledTimes(2); + expect(fs.copy).toHaveBeenCalledWith( + "/workspace/patches/v1/fix.patch", + "/workspace/isolate/patches/v1/fix.patch" + ); + expect(fs.copy).toHaveBeenCalledWith( + "/workspace/patches/v2/fix.patch", + "/workspace/isolate/patches/v2/fix.patch" + ); }); - it("should transform patch paths correctly for isolated directory", async () => { + it("should preserve deeply nested patch paths", async () => { const targetManifest: PackageManifest = { name: "test", version: "1.0.0", @@ -327,13 +341,16 @@ describe("copyPatches", () => { includeDevDependencies: false, }); - /** The path should be flattened to patches/ directory */ + /** The path should preserve the original directory structure */ expect(result).toEqual({ - "lodash@4.17.21": { path: "patches/lodash.patch", hash: "" }, + "lodash@4.17.21": { path: "some/nested/path/lodash.patch", hash: "" }, }); + expect(fs.ensureDir).toHaveBeenCalledWith( + "/workspace/isolate/some/nested/path" + ); expect(fs.copy).toHaveBeenCalledWith( "/workspace/some/nested/path/lodash.patch", - "/workspace/isolate/patches/lodash.patch" + "/workspace/isolate/some/nested/path/lodash.patch" ); }); diff --git a/src/lib/patches/copy-patches.ts b/src/lib/patches/copy-patches.ts index f6df271..2d819c5 100644 --- a/src/lib/patches/copy-patches.ts +++ b/src/lib/patches/copy-patches.ts @@ -7,7 +7,6 @@ import { usePackageManager } from "~/lib/package-manager"; import type { PackageManifest, PatchFile } from "~/lib/types"; import { filterPatchedDependencies, - getIsolateRelativeLogPath, getRootRelativeLogPath, isRushWorkspace, readTypedJson, @@ -63,11 +62,7 @@ export async function copyPatches({ const lockfilePatchedDependencies = await readLockfilePatchedDependencies(workspaceRootDir); - const patchesDir = path.join(isolateDir, "patches"); - await fs.ensureDir(patchesDir); - const copiedPatches: Record = {}; - const usedFilenames = new Set(); for (const [packageSpec, patchPath] of Object.entries(filteredPatches)) { const sourcePatchPath = path.resolve(workspaceRootDir, patchPath); @@ -79,33 +74,11 @@ export async function copyPatches({ continue; } - /** - * Generate a unique filename to avoid collisions from different - * subdirectories - */ - const basename = path.basename(patchPath); - let targetFilename = basename; - - if (usedFilenames.has(targetFilename)) { - const ext = path.extname(basename); - const name = path.basename(basename, ext); - let counter = 1; - - do { - targetFilename = `${name}-${counter}${ext}`; - counter++; - } while (usedFilenames.has(targetFilename)); - - log.debug( - `Renamed patch ${basename} to ${targetFilename} to avoid collision` - ); - } - - usedFilenames.add(targetFilename); - - const targetPatchPath = path.join(patchesDir, targetFilename); + /** Preserve original folder structure */ + const targetPatchPath = path.join(isolateDir, patchPath); + await fs.ensureDir(path.dirname(targetPatchPath)); await fs.copy(sourcePatchPath, targetPatchPath); - log.debug(`Copied patch for ${packageSpec}: ${targetFilename}`); + log.debug(`Copied patch for ${packageSpec}: ${patchPath}`); /** Get the hash from the original lockfile, or use empty string if not found */ const originalPatchFile = lockfilePatchedDependencies?.[packageSpec]; @@ -116,15 +89,13 @@ export async function copyPatches({ } copiedPatches[packageSpec] = { - path: `patches/${targetFilename}`, + path: patchPath, hash, }; } if (Object.keys(copiedPatches).length > 0) { - log.debug( - `Patches copied to ${getIsolateRelativeLogPath(patchesDir, isolateDir)}` - ); + log.debug(`Copied ${Object.keys(copiedPatches).length} patch files`); } return copiedPatches; From 39a37aade18309d159aeb9664906fe33cbf5448f Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:50:55 +0100 Subject: [PATCH 21/24] 1.27.0-2 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index c91c0e1..41348b1 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "isolate-package", - "version": "1.27.0-1", + "version": "1.27.0-2", "description": "Isolate a monorepo package with its shared dependencies to form a self-contained directory, compatible with Firebase deploy", "author": "Thijs Koerselman", "license": "MIT", From a94fde3c33f958bebf2a03a92f64a042110834f4 Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:55:43 +0100 Subject: [PATCH 22/24] Only copy patches when output uses pnpm Patched dependencies are a pnpm-specific feature. Skip copying patches when forceNpm is enabled or when the detected package manager is not pnpm. --- src/isolate.ts | 21 +++++++++++++-------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/src/isolate.ts b/src/isolate.ts index 7b19d4c..2001e53 100644 --- a/src/isolate.ts +++ b/src/isolate.ts @@ -202,15 +202,20 @@ export function createIsolator(config?: IsolateConfig) { /** * Copy patch files before generating lockfile so the lockfile contains the - * correct transformed paths (flattened to patches/ with collision - * avoidance). + * correct paths. Only copy patches when output uses pnpm, since patched + * dependencies are a pnpm-specific feature. */ - const copiedPatches = await copyPatches({ - workspaceRootDir, - targetPackageManifest: outputManifest, - isolateDir, - includeDevDependencies: config.includeDevDependencies, - }); + const shouldCopyPatches = + packageManager.name === "pnpm" && !config.forceNpm; + + const copiedPatches = shouldCopyPatches + ? await copyPatches({ + workspaceRootDir, + targetPackageManifest: outputManifest, + isolateDir, + includeDevDependencies: config.includeDevDependencies, + }) + : {}; /** Generate an isolated lockfile based on the original one */ const usedFallbackToNpm = await processLockfile({ From c979b933820b39fc3682818cfc04cb80cfb88b5b Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Thu, 27 Nov 2025 22:56:46 +0100 Subject: [PATCH 23/24] Format --- README.md | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index c9e45ed..5c572c6 100644 --- a/README.md +++ b/README.md @@ -329,9 +329,9 @@ When you use the `targetPackagePath` option, this setting will be ignored. ## PNPM Patched Dependencies -If your workspace uses PNPM's -[patched dependencies](https://pnpm.io/cli/patch) feature, `isolate` will -automatically copy the relevant patch files to the isolated output. +If your workspace uses PNPM's [patched dependencies](https://pnpm.io/cli/patch) +feature, `isolate` will automatically copy the relevant patch files to the +isolated output. Patches are filtered based on the target package's dependencies: From ec48d5e8c3787b03af0ae5038d5668494efc066d Mon Sep 17 00:00:00 2001 From: Thijs Koerselman Date: Fri, 28 Nov 2025 11:12:03 +0100 Subject: [PATCH 24/24] Update src/lib/lockfile/helpers/generate-pnpm-lockfile.ts Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/lib/lockfile/helpers/generate-pnpm-lockfile.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts index bc7f655..0fcf906 100644 --- a/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts +++ b/src/lib/lockfile/helpers/generate-pnpm-lockfile.ts @@ -164,7 +164,7 @@ export async function generatePnpmLockfile({ /** * Use pre-computed patched dependencies with transformed paths. The paths * are already adapted by copyPatches to match the isolated directory - * structure (flattened to patches/ with collision avoidance). + * structure, preserving the original folder structure (not flattened). */ if (useVersion9) { await writeWantedLockfile_v9(isolateDir, {