Skip to content

Commit

Permalink
feat(nm): Add support for user-defined <workspace>/node_modules symli…
Browse files Browse the repository at this point in the history
…nks (#6416)

## What's the problem this PR addresses?

<!-- Describe the rationale of your PR. -->
<!-- Link all issues that it closes. (Closes/Resolves #xxxx.) -->

Fixes: #6415 

## How did you fix it?

<!-- A detailed description of your implementation. -->

Now the `node-modules` linker do not try to delete or recreate
`<any_workspace>/node_modules` directories if they are symlinks and the
underlying dependencies were removed or newly added.

## Checklist

<!--- Don't worry if you miss something, chores are automatically
tested. -->
<!--- This checklist exists to help you remember doing the chores when
you submit a PR. -->
<!--- Put an `x` in all the boxes that apply. -->
- [x] I have read the [Contributing
Guide](https://yarnpkg.com/advanced/contributing).

<!-- See
https://yarnpkg.com/advanced/contributing#preparing-your-pr-to-be-released
for more details. -->
<!-- Check with `yarn version check` and fix with `yarn version check
-i` -->
- [x] I have set the packages that need to be released for my changes to
be effective.

<!-- The "Testing chores" workflow validates that your PR follows our
guidelines. -->
<!-- If it doesn't pass, click on it to see details as to what your PR
might be missing. -->
- [x] I will check that all automated PR checks pass before the PR gets
reviewed.
  • Loading branch information
larixer committed Aug 11, 2024
1 parent 7495114 commit 7fab4f1
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 9 deletions.
23 changes: 23 additions & 0 deletions .yarn/versions/b764a694.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
releases:
"@yarnpkg/cli": patch
"@yarnpkg/plugin-nm": patch

declined:
- "@yarnpkg/plugin-compat"
- "@yarnpkg/plugin-constraints"
- "@yarnpkg/plugin-dlx"
- "@yarnpkg/plugin-essentials"
- "@yarnpkg/plugin-init"
- "@yarnpkg/plugin-interactive-tools"
- "@yarnpkg/plugin-npm-cli"
- "@yarnpkg/plugin-pack"
- "@yarnpkg/plugin-patch"
- "@yarnpkg/plugin-pnp"
- "@yarnpkg/plugin-pnpm"
- "@yarnpkg/plugin-stage"
- "@yarnpkg/plugin-typescript"
- "@yarnpkg/plugin-version"
- "@yarnpkg/plugin-workspace-tools"
- "@yarnpkg/builder"
- "@yarnpkg/core"
- "@yarnpkg/doctor"
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ Features in `master` can be tried out by running `yarn set version from sources`
:::

- Fixes `preferInteractive` forcing interactive mode in non-TTY environments.
- `node-modules` linker now honors user-defined symlinks for `<workspace>/node_modules` directories

## 4.1.0

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1855,6 +1855,47 @@ describe(`Node_Modules`, () => {
}),
);

it(`should work with user-created <workspace>/node_modules symlinks`,
makeTemporaryEnv(
{
workspaces: [`ws`],
dependencies: {
},
},
{
nodeLinker: `node-modules`,
nmHoistingLimits: `workspaces`,
},
async ({path, run}) => {
await xfs.mkdirpPromise(ppath.join(path, `ws`));
const trueInstallDir = ppath.resolve(path, `target`);
await xfs.mkdirPromise(trueInstallDir);

await xfs.writeJsonPromise(ppath.join(path, `ws/${Filename.manifest}`), {
name: `ws`,
devDependencies: {
[`no-deps`]: `1.0.0`,
},
});

await xfs.symlinkPromise(trueInstallDir, ppath.join(path, `ws/node_modules`));

await run(`install`);

expect(xfs.existsSync(ppath.join(trueInstallDir, `no-deps`))).toBeTruthy();
expect(xfs.lstatSync(ppath.join(path, `ws/node_modules`)).isSymbolicLink()).toBeTruthy();

await xfs.writeJsonPromise(ppath.join(path, `ws/${Filename.manifest}`), {
name: `ws`,
});

await run(`install`);

expect(xfs.existsSync(ppath.join(trueInstallDir, `no-deps`))).toBeFalsy();
expect(xfs.lstatSync(ppath.join(path, `ws/node_modules`)).isSymbolicLink()).toBeTruthy();
}),
);

it(`should support supportedArchitectures`,
makeTemporaryEnv(
{
Expand Down
20 changes: 11 additions & 9 deletions packages/plugin-nm/sources/NodeModulesLinker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -525,15 +525,15 @@ async function findInstallState(project: Project, {unrollAliases = false}: {unro
return {locatorMap, binSymlinks, locationTree: buildLocationTree(locatorMap, {skipPrefix: project.cwd}), nmMode, mtimeMs: stats.mtimeMs};
}

const removeDir = async (dir: PortablePath, options: {contentsOnly: boolean, innerLoop?: boolean, allowSymlink?: boolean}): Promise<any> => {
const removeDir = async (dir: PortablePath, options: {contentsOnly: boolean, innerLoop?: boolean, isWorkspaceDir?: boolean}): Promise<any> => {
if (dir.split(ppath.sep).indexOf(NODE_MODULES) < 0)
throw new Error(`Assertion failed: trying to remove dir that doesn't contain node_modules: ${dir}`);

try {
let dirStats;
if (!options.innerLoop) {
const stats = options.allowSymlink ? await xfs.statPromise(dir) : await xfs.lstatPromise(dir);
if (options.allowSymlink && !stats.isDirectory() ||
(!options.allowSymlink && stats.isSymbolicLink())) {
dirStats = await xfs.lstatPromise(dir);
if ((!dirStats.isDirectory() && !dirStats.isSymbolicLink()) || (dirStats.isSymbolicLink() && !options.isWorkspaceDir)) {
await xfs.unlinkPromise(dir);
return;
}
Expand All @@ -549,7 +549,9 @@ const removeDir = async (dir: PortablePath, options: {contentsOnly: boolean, inn
await xfs.unlinkPromise(targetPath);
}
}
if (!options.contentsOnly) {

const isExternalWorkspaceSymlink = !options.innerLoop && options.isWorkspaceDir && dirStats?.isSymbolicLink();
if (!options.contentsOnly && !isExternalWorkspaceSymlink) {
await xfs.rmdirPromise(dir);
}
} catch (e) {
Expand Down Expand Up @@ -1133,8 +1135,8 @@ async function persistNodeModules(preinstallState: InstallState, installState: N
if (prevNode.children.has(NODE_MODULES))
await removeDir(ppath.join(location, NODE_MODULES), {contentsOnly: false});

const isRootNmLocation = ppath.basename(location) === NODE_MODULES && locationTree.has(ppath.join(ppath.dirname(location), ppath.sep));
await removeDir(location, {contentsOnly: location === rootNmDirPath, allowSymlink: isRootNmLocation});
const isWorkspaceNmLocation = ppath.basename(location) === NODE_MODULES && prevLocationTree.has(ppath.join(ppath.dirname(location)));
await removeDir(location, {contentsOnly: location === rootNmDirPath, isWorkspaceDir: isWorkspaceNmLocation});
} else {
for (const [segment, prevChildNode] of prevNode.children) {
const childNode = node.children.get(segment);
Expand Down Expand Up @@ -1164,8 +1166,8 @@ async function persistNodeModules(preinstallState: InstallState, installState: N

// 1. If new directory is a symlink, we need to remove it fully
// 2. If new directory is a hardlink - we just need to clean it up
const isRootNmLocation = ppath.basename(location) === NODE_MODULES && locationTree.has(ppath.join(ppath.dirname(location), ppath.sep));
await removeDir(location, {contentsOnly: node.linkType === LinkType.HARD, allowSymlink: isRootNmLocation});
const isWorkspaceNmLocation = ppath.basename(location) === NODE_MODULES && locationTree.has(ppath.join(ppath.dirname(location)));
await removeDir(location, {contentsOnly: node.linkType === LinkType.HARD, isWorkspaceDir: isWorkspaceNmLocation});
} else {
if (!areRealLocatorsEqual(node.locator, prevNode.locator))
await removeDir(location, {contentsOnly: node.linkType === LinkType.HARD});
Expand Down

0 comments on commit 7fab4f1

Please sign in to comment.