diff --git a/.changeset/cold-pigs-burn.md b/.changeset/cold-pigs-burn.md new file mode 100644 index 00000000000..e2ffe199865 --- /dev/null +++ b/.changeset/cold-pigs-burn.md @@ -0,0 +1,8 @@ +--- +"@pnpm/fs.indexed-pkg-importer": minor +"@pnpm/cafs-types": minor +"@pnpm/create-cafs-store": minor +"@pnpm/store-controller-types": minor +--- + +A new option added to package importer for keeping modules directory: `keepModulesDir`. When this is set to true, if a package already exist at the target location and it has a node_modules directory, then that node_modules directory is moved to the newly imported dependency. This is only needed when node-linker=hoisted is used. diff --git a/.changeset/late-swans-impress.md b/.changeset/late-swans-impress.md new file mode 100644 index 00000000000..c8e565fb394 --- /dev/null +++ b/.changeset/late-swans-impress.md @@ -0,0 +1,7 @@ +--- +"pnpm": minor +"@pnpm/core": minor +"@pnpm/headless": minor +--- + +When the hoisted node linker is used, preserve `node_modules` directories when linking new dependencies. This improves performance, when installing in a project that already has a `node_modules` directory [#5795](https://github.com/pnpm/pnpm/pull/5795). diff --git a/.changeset/wet-coins-greet.md b/.changeset/wet-coins-greet.md new file mode 100644 index 00000000000..caaa22f37e9 --- /dev/null +++ b/.changeset/wet-coins-greet.md @@ -0,0 +1,5 @@ +--- +"@pnpm/modules-yaml": minor +--- + +New field saved in the modules state file: `hoistedLocations`. This field maps the locations of dependencies, when `node-linker=hoisted` is used. diff --git a/fs/indexed-pkg-importer/src/importIndexedDir.ts b/fs/indexed-pkg-importer/src/importIndexedDir.ts index 40c331f161d..5b5994bfbef 100644 --- a/fs/indexed-pkg-importer/src/importIndexedDir.ts +++ b/fs/indexed-pkg-importer/src/importIndexedDir.ts @@ -14,11 +14,18 @@ export type ImportFile = (src: string, dest: string) => Promise export async function importIndexedDir ( importFile: ImportFile, newDir: string, - filenames: Record + filenames: Record, + opts: { + keepModulesDir?: boolean + } ) { const stage = pathTemp(path.dirname(newDir)) try { await tryImportIndexedDir(importFile, stage, filenames) + if (opts.keepModulesDir) { + // Keeping node_modules is needed only when the hoisted node linker is used. + await moveOrMergeModulesDirs(path.join(newDir, 'node_modules'), path.join(stage, 'node_modules')) + } await renameOverwrite(stage, newDir) } catch (err: any) { // eslint-disable-line try { @@ -37,7 +44,7 @@ export async function importIndexedDir ( 'which is an issue on case-insensitive filesystems. ' + `The conflicting file names are: ${JSON.stringify(conflictingFileNames)}` ) - await importIndexedDir(importFile, newDir, uniqueFileMap) + await importIndexedDir(importFile, newDir, uniqueFileMap, opts) return } if (err['code'] === 'ENOENT') { @@ -47,7 +54,7 @@ export async function importIndexedDir ( The package linked to "${path.relative(process.cwd(), newDir)}" had \ files with invalid names: ${invalidFilenames.join(', ')}. \ They were renamed.`) - await importIndexedDir(importFile, newDir, sanitizedFilenames) + await importIndexedDir(importFile, newDir, sanitizedFilenames, opts) return } throw err @@ -108,3 +115,29 @@ function getUniqueFileMap (fileMap: Record) { uniqueFileMap, } } + +async function moveOrMergeModulesDirs (src: string, dest: string) { + try { + await fs.rename(src, dest) + } catch (err: any) { // eslint-disable-line + switch (err.code) { + case 'ENOENT': + // If src directory doesn't exist, there is nothing to do + return + case 'ENOTEMPTY': + case 'EPERM': // This error code is thrown on Windows + // The newly added dependency might have node_modules if it has bundled dependencies. + await mergeModulesDirs(src, dest) + return + default: + throw err + } + } +} + +async function mergeModulesDirs (src: string, dest: string) { + const srcFiles = await fs.readdir(src) + const destFiles = new Set(await fs.readdir(dest)) + const filesToMove = srcFiles.filter((file) => !destFiles.has(file)) + await Promise.all(filesToMove.map((file) => fs.rename(path.join(src, file), path.join(dest, file)))) +} diff --git a/fs/indexed-pkg-importer/src/index.ts b/fs/indexed-pkg-importer/src/index.ts index d48d8609165..c84950762a3 100644 --- a/fs/indexed-pkg-importer/src/index.ts +++ b/fs/indexed-pkg-importer/src/index.ts @@ -110,7 +110,7 @@ async function clonePkg ( const pkgJsonPath = path.join(to, 'package.json') if (!opts.fromStore || opts.force || !await exists(pkgJsonPath)) { - await importIndexedDir(cloneFile, to, opts.filesMap) + await importIndexedDir(cloneFile, to, opts.filesMap, opts) return 'clone' } return undefined @@ -130,7 +130,7 @@ async function hardlinkPkg ( opts.force || !await pkgLinkedToStore(opts.filesMap, to) ) { - await importIndexedDir(importFile, to, opts.filesMap) + await importIndexedDir(importFile, to, opts.filesMap, opts) return 'hardlink' } return undefined @@ -188,7 +188,7 @@ export async function copyPkg ( const pkgJsonPath = path.join(to, 'package.json') if (!opts.fromStore || opts.force || !await exists(pkgJsonPath)) { - await importIndexedDir(fs.copyFile, to, opts.filesMap) + await importIndexedDir(fs.copyFile, to, opts.filesMap, opts) return 'copy' } return undefined diff --git a/fs/indexed-pkg-importer/test/importIndexedDir.test.ts b/fs/indexed-pkg-importer/test/importIndexedDir.test.ts new file mode 100644 index 00000000000..c8c6ccda5a2 --- /dev/null +++ b/fs/indexed-pkg-importer/test/importIndexedDir.test.ts @@ -0,0 +1,21 @@ +import { tempDir } from '@pnpm/prepare' +import { promises as fs, mkdirSync, writeFileSync } from 'fs' +import path from 'path' +import { importIndexedDir } from '../src/importIndexedDir' + +test('importIndexedDir() keepModulesDir merges node_modules', async () => { + const tmp = tempDir() + mkdirSync(path.join(tmp, 'src/node_modules/a'), { recursive: true }) + writeFileSync(path.join(tmp, 'src/node_modules/a/index.js'), 'module.exports = 1') + + mkdirSync(path.join(tmp, 'dest/node_modules/b'), { recursive: true }) + writeFileSync(path.join(tmp, 'dest/node_modules/b/index.js'), 'module.exports = 1') + + const newDir = path.join(tmp, 'dest') + const filenames = { + 'node_modules/a/index.js': path.join(tmp, 'src/node_modules/a/index.js'), + } + await importIndexedDir(fs.link, newDir, filenames, { keepModulesDir: true }) + + expect(await fs.readdir(path.join(newDir, 'node_modules'))).toEqual(['a', 'b']) +}) diff --git a/pkg-manager/core/src/install/index.ts b/pkg-manager/core/src/install/index.ts index a44047f38c9..a4ea065d367 100644 --- a/pkg-manager/core/src/install/index.ts +++ b/pkg-manager/core/src/install/index.ts @@ -344,6 +344,7 @@ export async function mutateModules ( nodeVersion: opts.nodeVersion, pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '', }, + currentHoistedLocations: ctx.modulesFile?.hoistedLocations, patchedDependencies: patchedDependenciesWithResolvedPath, selectedProjectDirs: projects.map((project) => project.rootDir), allProjects: ctx.projects, @@ -1177,6 +1178,7 @@ const installInContext: InstallFunction = async (projects, ctx, opts) => { nodeVersion: opts.nodeVersion, pnpmVersion: opts.packageManager.name === 'pnpm' ? opts.packageManager.version : '', }, + currentHoistedLocations: ctx.modulesFile?.hoistedLocations, selectedProjectDirs: projects.map((project) => project.rootDir), allProjects: ctx.projects, prunedAt: ctx.modulesFile?.prunedAt, diff --git a/pkg-manager/core/src/install/link.ts b/pkg-manager/core/src/install/link.ts index 63ccf8fdc8b..d655b6836c2 100644 --- a/pkg-manager/core/src/install/link.ts +++ b/pkg-manager/core/src/install/link.ts @@ -46,9 +46,7 @@ export async function linkPackages ( opts: { currentLockfile: Lockfile dedupeDirectDeps: boolean - dependenciesByProjectId: { - [id: string]: { [alias: string]: string } - } + dependenciesByProjectId: Record> force: boolean depsStateCache: DepsStateCache extraNodePaths: string[] @@ -61,7 +59,7 @@ export async function linkPackages ( linkedDependenciesByProjectId: Record lockfileDir: string makePartialCurrentLockfile: boolean - outdatedDependencies: { [pkgId: string]: string } + outdatedDependencies: Record pruneStore: boolean pruneVirtualStore: boolean registries: Registries diff --git a/pkg-manager/headless/src/index.ts b/pkg-manager/headless/src/index.ts index 4ab7dd70a36..88ebbb1645e 100644 --- a/pkg-manager/headless/src/index.ts +++ b/pkg-manager/headless/src/index.ts @@ -120,6 +120,7 @@ export interface HeadlessOptions { hoistedDependencies: HoistedDependencies hoistPattern?: string[] publicHoistPattern?: string[] + currentHoistedLocations?: Record lockfileDir: string modulesDir?: string virtualStoreDir?: string @@ -283,6 +284,7 @@ export async function headlessInstall (opts: HeadlessOptions) { directDependenciesByImporterId, graph, hierarchy, + hoistedLocations, pkgLocationsByDepPath, prevGraph, symlinkedDirectDependenciesByImporterId, @@ -528,6 +530,7 @@ export async function headlessInstall (opts: HeadlessOptions) { included: opts.include, injectedDeps, layoutVersion: LAYOUT_VERSION, + hoistedLocations, nodeLinker: opts.nodeLinker, packageManager: `${opts.packageManager.name}@${opts.packageManager.version}`, pendingBuilds: opts.pendingBuilds, diff --git a/pkg-manager/headless/src/linkHoistedModules.ts b/pkg-manager/headless/src/linkHoistedModules.ts index 5d0fb4bac42..4e539801d28 100644 --- a/pkg-manager/headless/src/linkHoistedModules.ts +++ b/pkg-manager/headless/src/linkHoistedModules.ts @@ -95,36 +95,39 @@ async function linkAllPkgsInOrder ( await Promise.all( Object.entries(hierarchy).map(async ([dir, deps]) => { const depNode = graph[dir] - let filesResponse!: PackageFilesResponse - try { - filesResponse = await depNode.fetchingFiles() - } catch (err: any) { // eslint-disable-line - if (depNode.optional) return - throw err - } + if (depNode.fetchingFiles) { + let filesResponse!: PackageFilesResponse + try { + filesResponse = await depNode.fetchingFiles() + } catch (err: any) { // eslint-disable-line + if (depNode.optional) return + throw err + } - let sideEffectsCacheKey: string | undefined - if (opts.sideEffectsCacheRead && filesResponse.sideEffects && !isEmpty(filesResponse.sideEffects)) { - sideEffectsCacheKey = _calcDepState(dir, { - isBuilt: !opts.ignoreScripts && depNode.requiresBuild, - patchFileHash: depNode.patchFile?.hash, - }) - } - const { importMethod, isBuilt } = await storeController.importPackage(depNode.dir, { - filesResponse, - force: opts.force || depNode.depPath !== prevGraph[dir]?.depPath, - requiresBuild: depNode.requiresBuild || depNode.patchFile != null, - sideEffectsCacheKey, - }) - if (importMethod) { - progressLogger.debug({ - method: importMethod, - requester: opts.lockfileDir, - status: 'imported', - to: depNode.dir, + let sideEffectsCacheKey: string | undefined + if (opts.sideEffectsCacheRead && filesResponse.sideEffects && !isEmpty(filesResponse.sideEffects)) { + sideEffectsCacheKey = _calcDepState(dir, { + isBuilt: !opts.ignoreScripts && depNode.requiresBuild, + patchFileHash: depNode.patchFile?.hash, + }) + } + const { importMethod, isBuilt } = await storeController.importPackage(depNode.dir, { + filesResponse, + force: opts.force || depNode.depPath !== prevGraph[dir]?.depPath, + keepModulesDir: true, + requiresBuild: depNode.requiresBuild || depNode.patchFile != null, + sideEffectsCacheKey, }) + if (importMethod) { + progressLogger.debug({ + method: importMethod, + requester: opts.lockfileDir, + status: 'imported', + to: depNode.dir, + }) + } + depNode.isBuilt = isBuilt } - depNode.isBuilt = isBuilt return linkAllPkgsInOrder(storeController, graph, prevGraph, deps, dir, opts) }) ) diff --git a/pkg-manager/headless/src/lockfileToDepGraph.ts b/pkg-manager/headless/src/lockfileToDepGraph.ts index 0680a9ff62a..bbd68ac6900 100644 --- a/pkg-manager/headless/src/lockfileToDepGraph.ts +++ b/pkg-manager/headless/src/lockfileToDepGraph.ts @@ -80,6 +80,7 @@ export interface LockfileToDepGraphResult { directDependenciesByImporterId: DirectDependenciesByImporterId graph: DependenciesGraph hierarchy?: DepHierarchy + hoistedLocations?: Record symlinkedDirectDependenciesByImporterId?: DirectDependenciesByImporterId prevGraph?: DependenciesGraph pkgLocationsByDepPath?: Record diff --git a/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts b/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts index 7745af6001c..ce8463a41d2 100644 --- a/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts +++ b/pkg-manager/headless/src/lockfileToHoistedDepGraph.ts @@ -32,6 +32,7 @@ export interface LockfileToHoistedDepGraphOptions { externalDependencies?: Set importerIds: string[] include: IncludedDependencies + currentHoistedLocations?: Record lockfileDir: string nodeVersion: string pnpmVersion: string @@ -73,6 +74,7 @@ async function _lockfileToHoistedDepGraph ( lockfile, graph, pkgLocationsByDepPath: {}, + hoistedLocations: {} as Record, } const hierarchy = { [opts.lockfileDir]: await fetchDeps(fetchDepsOpts, modulesDir, tree.dependencies), @@ -102,6 +104,7 @@ async function _lockfileToHoistedDepGraph ( hierarchy, pkgLocationsByDepPath: fetchDepsOpts.pkgLocationsByDepPath, symlinkedDirectDependenciesByImporterId, + hoistedLocations: fetchDepsOpts.hoistedLocations, } } @@ -136,6 +139,7 @@ async function fetchDeps ( graph: DependenciesGraph lockfile: Lockfile pkgLocationsByDepPath: Record + hoistedLocations: Record } & LockfileToHoistedDepGraphOptions, modules: string, deps: Set @@ -173,25 +177,31 @@ async function fetchDeps ( return } const dir = path.join(modules, dep.name) + const depLocation = path.relative(opts.lockfileDir, dir) const resolution = pkgSnapshotToResolution(depPath, pkgSnapshot, opts.registries) let fetchResponse!: ReturnType - try { - fetchResponse = opts.storeController.fetchPackage({ - force: false, - lockfileDir: opts.lockfileDir, - pkg: { - id: packageId, - resolution, - }, - expectedPkg: { - name: pkgName, - version: pkgVersion, - }, - }) - if (fetchResponse instanceof Promise) fetchResponse = await fetchResponse - } catch (err: any) { // eslint-disable-line - if (pkgSnapshot.optional) return - throw err + const skipFetch = opts.currentHoistedLocations?.[depPath]?.includes(depLocation) + if (skipFetch) { + fetchResponse = {} as any // eslint-disable-line @typescript-eslint/no-explicit-any + } else { + try { + fetchResponse = opts.storeController.fetchPackage({ + force: false, + lockfileDir: opts.lockfileDir, + pkg: { + id: packageId, + resolution, + }, + expectedPkg: { + name: pkgName, + version: pkgVersion, + }, + }) + if (fetchResponse instanceof Promise) fetchResponse = await fetchResponse + } catch (err: any) { // eslint-disable-line + if (pkgSnapshot.optional) return + throw err + } } opts.graph[dir] = { alias: dep.name, @@ -216,6 +226,10 @@ async function fetchDeps ( } opts.pkgLocationsByDepPath[depPath].push(dir) depHierarchy[dir] = await fetchDeps(opts, path.join(dir, 'node_modules'), dep.dependencies) + if (!opts.hoistedLocations[depPath]) { + opts.hoistedLocations[depPath] = [] + } + opts.hoistedLocations[depPath].push(depLocation) opts.graph[dir].children = getChildren(pkgSnapshot, opts.pkgLocationsByDepPath, opts) })) return depHierarchy diff --git a/pkg-manager/modules-yaml/src/index.ts b/pkg-manager/modules-yaml/src/index.ts index ffc62241c0a..0029cc669fd 100644 --- a/pkg-manager/modules-yaml/src/index.ts +++ b/pkg-manager/modules-yaml/src/index.ts @@ -31,6 +31,7 @@ export interface Modules { storeDir: string virtualStoreDir: string injectedDeps?: Record + hoistedLocations?: Record } export async function readModulesManifest (modulesDir: string): Promise { diff --git a/store/cafs-types/src/index.ts b/store/cafs-types/src/index.ts index c2b8c5cb960..1f7f718e8c0 100644 --- a/store/cafs-types/src/index.ts +++ b/store/cafs-types/src/index.ts @@ -30,6 +30,7 @@ export interface ImportPackageOpts { sideEffectsCacheKey?: string filesResponse: PackageFilesResponse force: boolean + keepModulesDir?: boolean } export type ImportPackageFunction = ( diff --git a/store/create-cafs-store/src/index.ts b/store/create-cafs-store/src/index.ts index 5fa7b45a2fe..fd77b6a147a 100644 --- a/store/create-cafs-store/src/index.ts +++ b/store/create-cafs-store/src/index.ts @@ -33,7 +33,12 @@ function createPackageImporter ( ? 'clone-or-copy' : (opts.filesResponse.packageImportMethod ?? packageImportMethod) const impPkg = cachedImporterCreator(pkgImportMethod) - const importMethod = await impPkg(to, { filesMap, fromStore: opts.filesResponse.fromStore, force: opts.force }) + const importMethod = await impPkg(to, { + filesMap, + fromStore: opts.filesResponse.fromStore, + force: opts.force, + keepModulesDir: Boolean(opts.keepModulesDir), + }) return { importMethod, isBuilt } } } diff --git a/store/store-controller-types/src/index.ts b/store/store-controller-types/src/index.ts index 7271f16ec7b..4ea73acc4a0 100644 --- a/store/store-controller-types/src/index.ts +++ b/store/store-controller-types/src/index.ts @@ -147,6 +147,7 @@ export interface ImportOptions { filesMap: FilesMap force: boolean fromStore: boolean + keepModulesDir?: boolean } export type ImportIndexedPackage = (to: string, opts: ImportOptions) => Promise