Skip to content

Commit

Permalink
Do no move node_modules when generating NPM lockfile (#85)
Browse files Browse the repository at this point in the history
  • Loading branch information
0x80 authored May 19, 2024
1 parent 40f1ecf commit 7483eb0
Show file tree
Hide file tree
Showing 2 changed files with 10 additions and 49 deletions.
24 changes: 5 additions & 19 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,8 @@ integrated, check out [mono-ts](https://github.com/0x80/mono-ts)

Run `pnpm install isolate-package --dev` or the equivalent for `npm` or `yarn`.

It is recommended to use `pnpm` over `npm` or `yarn`. Apart from being fast and
efficient, PNPM has better support for monorepos, and the lockfile isolation is
solid and works in parallel for multiple packages, [unlike NPM](#npm)
I recommended using `pnpm` over `npm` or `yarn`. Besides being fast and
efficient, PNPM has better support for monorepos.

## Usage

Expand Down Expand Up @@ -331,17 +330,8 @@ each package manager, with NPM currently being the least attractive.
### NPM

For NPM we use a tool called Arborist, which is an integral part of the NPM
codebase. It is executed in the isolate output directory and requires the
adapted lockfile and the `node_modules` directory from the root of the
repository. As this directory is typically quite large, copying it over as part
of the isolate flow is not very desirable.

To work around this, we move it to the isolate output and then move it back
after Arborist has finished doing its thing.

> !! Warning: This will not be compatible with setups that run multiple
> isolation processes in parallel. Hopefully a future update to NPM Arborist
> (the part the generates the lockfile) will solve this.
codebase. It is executed in the isolate output directory with an adaptation
adapted manifest file.

### PNPM

Expand All @@ -355,11 +345,7 @@ dependencies of internally linked packages are not installed by PNPM.
### Classic Yarn

For Yarn v1 we can simply copy the root lockfile to the isolate output, and run
a `yarn install` to prune that lockfile. The command finds the installed node
modules in the root of the monorepo so versions are preserved.

> Note: I expect this to break down if you configure the isolate output
> directory to be located outside the monorepo tree.
a `yarn install` to prune that lockfile.

### Modern Yarn

Expand Down
35 changes: 5 additions & 30 deletions src/lib/lockfile/helpers/generate-npm-lockfile.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import Arborist from "@npmcli/arborist";
import fs from "fs-extra";
import path from "node:path";
import { useLogger } from "~/lib/logger";
import { getErrorMessage, inspectValue } from "~/lib/utils";
import { getErrorMessage } from "~/lib/utils";

/**
* Generate an isolated / pruned lockfile, based on the contents of installed
Expand All @@ -20,28 +20,15 @@ export async function generateNpmLockfile({

log.debug("Generating NPM lockfile...");

const origRootNodeModulesPath = path.join(workspaceRootDir, "node_modules");
const tempRootNodeModulesPath = path.join(isolateDir, "node_modules");

let hasMovedNodeModules = false;

let hasError = false;
const nodeModulesPath = path.join(workspaceRootDir, "node_modules");

try {
if (!fs.existsSync(origRootNodeModulesPath)) {
throw new Error(
`Failed to find node_modules at ${origRootNodeModulesPath}`
);
if (!fs.existsSync(nodeModulesPath)) {
throw new Error(`Failed to find node_modules at ${nodeModulesPath}`);
}

log.debug(`Temporarily moving node_modules to the isolate output`);

await fs.move(origRootNodeModulesPath, tempRootNodeModulesPath);
hasMovedNodeModules = true;

const arborist = new Arborist({ path: isolateDir });

log.debug(`Building tree...`);
const { meta } = await arborist.buildIdealTree();

meta?.commit();
Expand All @@ -52,18 +39,6 @@ export async function generateNpmLockfile({

log.debug("Created lockfile at", lockfilePath);
} catch (err) {
console.error(inspectValue(err));
log.error(`Failed to generate lockfile: ${getErrorMessage(err)}`);
hasError = true;
} finally {
/** @todo We should be able to use the new "using" keyword for this I think. */
if (hasMovedNodeModules) {
log.debug(`Restoring node_modules to the workspace root`);
await fs.move(tempRootNodeModulesPath, origRootNodeModulesPath);
}
}

if (hasError) {
throw new Error("Failed to generate lockfile");
throw new Error(`Failed to generate lockfile: ${getErrorMessage(err)}`);
}
}

0 comments on commit 7483eb0

Please sign in to comment.