Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Feature: Allow Arborist to configure node_modules and manifest locations independently #7214

Open
2 tasks done
0x80 opened this issue Feb 10, 2024 · 4 comments
Open
2 tasks done
Labels
Needs Triage needs review for next steps

Comments

@0x80
Copy link

0x80 commented Feb 10, 2024

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

This is not a bug report, but I didn't see a way to do a feature request.

For Arborist to generate a tree based on node_modules and a manifest, it currently expects them both in the same location and outputs its lockfile there. This is problematic for isolate-package, which wants to generate a pruned lockfile based on the node_modules from the monorepo root, plus an adapted manifest living in a different (isolate) directory.

To make things work, I need to temporarily move the node_modules folder into the isolate directory before running Arborist, and then move it back when finished.

Apart from it being an ugly hack, it prevents multiple processes from executing in parallel. This is a problem because in a monorepo it's likely you want to isolate and deploy multiple packages in parallel.

Generating pruned lockfiles for Yarn and PNPM is not so problematic, but for NPM this is currently the only way to do it AFAIK.

Here is a link to the code with the workaround:
https://github.com/0x80/isolate-package/blob/main/src/lib/lockfile/helpers/generate-npm-lockfile.ts

Expected Behavior

Make Arborist accept a separate configuration for locations of the node_modules and manifest files. Please? 🙏

Steps To Reproduce

No response

Environment

No response

@0x80 0x80 added Bug thing that needs fixing Needs Triage needs review for next steps Release 10.x labels Feb 10, 2024
@ljharb
Copy link
Contributor

ljharb commented Feb 10, 2024

npm commands that affect the disk are not safe to run in parallel, regardless. In a monorepo or not, all these things have to be in serial anyways.

@0x80
Copy link
Author

0x80 commented Feb 12, 2024

@ljharb Each monorepo package that is targeted for isolation gets its own temp and output directory. The sources for the target plus each internal dependency are copied to those directories, and an adapted manifest file is written there before Arborist is executed.

Would you still consider that unsafe?

Unless Arborist mutates the node_modules folder when it runs, I don't see how it would be unsafe to execute in parallel.

@ljharb
Copy link
Contributor

ljharb commented Feb 12, 2024

npm's own cache would still be in play.

@0x80
Copy link
Author

0x80 commented Feb 12, 2024

Ah, right. Do you think the cache mutates even when I'm only calling buildIdealTree and the node_modules folder is expected to be complete?

If it would be possible to configure Arborist to take the manifest + target location via configuration, that would still be an improvement for me nonetheless, because having to move the node_modules directory to and from the target directory, as I do currently, doesn't feel great.

@0x80 0x80 changed the title Feature: Update Arborist configure node_modules and manifest locations independently Feature: Allow Arborist to configure node_modules and manifest locations independently Feb 12, 2024
@milaninfy milaninfy removed the Bug thing that needs fixing label Jul 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Triage needs review for next steps
Projects
None yet
Development

No branches or pull requests

4 participants