`npm copy` copy module files and dependencies into a deployable folder #4082

everett1992 · 2021-11-22T18:19:53Z

This idea was discussed in npm/rfcs#493 and the 2021-11-17 RFC meeting. The gist is there's a requirement to create a production copy of a package or workspace for deployment (zip archives for lambda, COPY into docker image) and existing npm commands have shortcomings, specifically regarding workspaces.

This draft CR will give a foundation for discussions.

what scripts, if any should be run when a workspace package is copied?
should this command respect --omit and --production flags?
could pack, install or another command support this usecase instead of adding a new command?
what should this command or flag be called?

I'll work on tests and documentation after more of the command is finalized

Goals (still up for discussion):

support workspace and non workspace packages
in a workspace a subset of workspace packages can be copied
command should not install new packages from the registry, instead it should manipulate the existing node_modules directory
command should copy install script generated files (should not need to run install scripts for packages copied from node_modules)

wraithgar · 2021-11-29T19:21:41Z

What is fs-extra providing that https://github.com/npm/fs isn't?

everett1992 · 2021-11-29T19:55:32Z

What is fs-extra providing that https://github.com/npm/fs isn't?

I used fs-extra because I couldn't find whatever fs lib npm preferred I wasn't aware of @npmcli/fs.

Is this a polyfil / monkey patch? Should I require fs or @npmcli/fs
@npmcli/fs isn't a dependency of npm/cli should I add it to package.json?
@npmcli/fs doesn't include recursive copy, fs.cp is Stability: 1 in node 16 and 17

wraithgar · 2021-11-29T20:05:41Z

* Is this a polyfil / monkey patch? Should I require fs or @npmcli/fs

It's a polyfill. Good examples of how we are using it are in cacache

* @npmcli/fs isn't a dependency of npm/cli should I add it to package.json?

It's already a subdependency of cacache so adding it to the top shouldn't change much.

* @npmcli/fs doesn't include recursive copy, `fs.cp` is Stability: 1 in node 16 and 17

Ah ok this would be the gap we'd need to fill in @npmcli/fs then.

everett1992 · 2021-11-29T20:32:17Z

Ah ok this would be the gap we'd need to fill in @npmcli/fs then.

Is that something you'll work on or should I start a PR?

wraithgar · 2021-11-29T21:06:29Z

Is that something you'll work on or should I start a PR?

Please start a PR. It's not something we'd be able to get to internally for some time.

everett1992 · 2021-11-29T21:16:46Z

Will you be happy with the experimental fs.cp interface?
Is there a recursive copy in npm dependencies or that I could add instead of contributing to @npmcli/fs? I'm not sure how much effort it is to implement a robust library-level recursive cp that matches fs.cp.

wraithgar · 2021-11-29T21:37:46Z

Yes matching what is likely to land in node itself would be the best option if you went down that path.

There's no current implementation of this functionality in npm itself. Arborist effects this by duplicating the tree itself and then reifying it to another destination. This obviously won't work for module files.

I believe that if you paired the recursive mkdir in @npmcli/fs with path.basename you could then iterate through everything packlist sends you, make sure the directory exists with the same ownership assumptions that npm already has baked into the other things it does, and use fs.cp as-is. It would leave yourself with a pretty small code footprint. It may be easier than implementing a recursive cp option that maintains parity w/ node.

everett1992 · 2021-12-01T18:19:55Z

I want to wait for more feedback on general implementation before I start working on replacing fs-extra.

I think we need a recursive copy. packlist can be replaced with mkdir -p and copy, but copying node_modules dependencies requires recursive copy. Maybe we could use arborist to reify a modified tree, but I would need help. I couldn't tell how to change the root of a tree.

I need `fs.cp` in `npm copy` to copy node_modules files. I'm adapting node's [lib/internal/fs/cp/cp.js][0]. I'm checking in the original so I can record changes in git. ref npm/cli#4082 [0]: https://github.com/nodejs/node/blob/1fa507f098ca7a89012f76f0c849fa698e73a1a1/lib/internal/fs/cp/cp.js

nlf · 2022-01-12T17:54:04Z

fwiw, @npmcli/fs does provide a recursive copy. it polyfills the recursive flag for fs.cp provided in newer node versions so that it will be usable in any version.

nlf · 2022-01-12T17:54:57Z

fwiw, @npmcli/fs does provide a recursive copy. it polyfills the recursive flag for fs.cp provided in newer node versions so that it will be usable in any version.

which i'm just now remembering was you that implemented! 🤦

jeanbmar · 2022-02-08T15:22:23Z

lib/commands/copy.js

+}
+
+async function relativeSymlink (target, path) {
+  await fs.mkdir(dirname(path), { recursive: true })


mkdir fails on Windows when the directory already exists, even with recursive: true

i've spent more time on this EEXIST issue, and it seems that pushing fs.cp calls inside tasks isn't safe. mkdir operations happen concurrently under the hood multiple times for a same folder, which results in an error

We can drop parallel tasks and just run sequentially

everett1992 mentioned this pull request Nov 22, 2021

docs: add npm copy rfc npm/rfcs#493

Open

darcyclarke added Release 8.x semver:minor Needs Review pr: needs tests Needs Discussion labels Nov 30, 2021

everett1992 mentioned this pull request Dec 7, 2021

polyfill node's fs.cp. npm/fs#6

Merged

everett1992 force-pushed the copy branch from 9869596 to 9d7cca6 Compare Dec 16, 2021

everett1992 force-pushed the copy branch 2 times, most recently from 928384a to 06e0234 Compare Jan 4, 2022

everett1992 force-pushed the copy branch 3 times, most recently from 651dbbd to 7fc9416 Compare Jan 11, 2022

fritzy force-pushed the latest branch from 06bdcc2 to d8aac84 Compare Jan 13, 2022

wraithgar changed the base branch from latest to release-next Jan 26, 2022

jeanbmar reviewed Feb 8, 2022

View changes

wraithgar changed the base branch from release-next to latest Mar 9, 2022

jeanbmar mentioned this pull request Mar 21, 2022

electron-forge does not work with npm workspaces (npm v7) electron-userland/electron-forge#2306

Closed

3 tasks

darcyclarke removed Needs Discussion Needs Review labels Mar 31, 2022

everett1992 force-pushed the copy branch 2 times, most recently from 3fd4909 to 4933c68 Compare Apr 4, 2022

darcyclarke mentioned this pull request Apr 6, 2022

Open RFC Meeting - Wednesday, April 6, 2022, 2:00 PM EST npm/rfcs#565

Closed

everett1992 force-pushed the copy branch from 4933c68 to ddf68b9 Compare Apr 18, 2022

feat: new npm copy command

4da0872

everett1992 force-pushed the copy branch from ddf68b9 to 4da0872 Compare Apr 18, 2022

everett1992 marked this pull request as ready for review Apr 18, 2022

everett1992 requested a review from as a code owner Apr 18, 2022

npm / cli Public

`npm copy` copy module files and dependencies into a deployable folder #4082

`npm copy` copy module files and dependencies into a deployable folder #4082

everett1992 commented Nov 22, 2021 •

edited

wraithgar commented Nov 29, 2021

everett1992 commented Nov 29, 2021

wraithgar commented Nov 29, 2021

everett1992 commented Nov 29, 2021

wraithgar commented Nov 29, 2021

everett1992 commented Nov 29, 2021

wraithgar commented Nov 29, 2021 •

edited

everett1992 commented Dec 1, 2021

nlf commented Jan 12, 2022

nlf commented Jan 12, 2022

jeanbmar Feb 8, 2022 •

edited

jeanbmar Feb 8, 2022 •

edited

everett1992 Feb 10, 2022

npm / cli Public

npm copy copy module files and dependencies into a deployable folder #4082

Are you sure you want to change the base?

npm copy copy module files and dependencies into a deployable folder #4082

Conversation

everett1992 commented Nov 22, 2021 • edited

wraithgar commented Nov 29, 2021

everett1992 commented Nov 29, 2021

wraithgar commented Nov 29, 2021

everett1992 commented Nov 29, 2021

wraithgar commented Nov 29, 2021

everett1992 commented Nov 29, 2021

wraithgar commented Nov 29, 2021 • edited

everett1992 commented Dec 1, 2021

nlf commented Jan 12, 2022

nlf commented Jan 12, 2022

jeanbmar Feb 8, 2022 • edited

Choose a reason for hiding this comment

jeanbmar Feb 8, 2022 • edited

Choose a reason for hiding this comment

everett1992 Feb 10, 2022

Choose a reason for hiding this comment

`npm copy` copy module files and dependencies into a deployable folder #4082

`npm copy` copy module files and dependencies into a deployable folder #4082

everett1992 commented Nov 22, 2021 •

edited

wraithgar commented Nov 29, 2021 •

edited

jeanbmar Feb 8, 2022 •

edited

jeanbmar Feb 8, 2022 •

edited