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

[everett1992]

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]

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

[everett1992]

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]

* 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]

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]

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]

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]

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]

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.

[nlf]

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]

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! :facepalm:

[pose]

Hi,

I was wondering if there were any news on this issue. @jeanbmar is there anything @everett1992 or I could help with to expedite this review?

Thanks,

[raphaelboukara]

Hi 😀, what's the status of this issue? thanks

[pose]

Hi @jeanbmar,

Sorry for the direct ping, do you have any updates on how we can make sure this PR moves forward?

Thanks again,

[jeanbmar]

Hi @pose, I don’t work with the npm team :). I need this too!

[pose]

Hi @nlf,

Sorry for the direct ping, we are trying to find the right owner for this review. Do you think you could help us out?

Thanks,

[ghost]

I can see that https://github.com/everett1992/rfcs/blob/npm-copy/accepted/0000-npm-copy.md mentions --production flag, but it is not covered in the code change. Is this intended?

[everett1992]

@amazlite I haven't updated the RFC as npm has evolved. This PR uses the --omit flag rather than --production to match npm 7+.

npm install --production
npm WARN config production Use `--omit=dev` instead.

[jeanbmar]

@everett1992 I've created a repo to show people reading this PR how to use your work on npm copy while we are waiting for npm to move forward: https://github.com/jeanbmar/npm-copy-test.

I've noticed a problem though. The RFC says that the command "copies the current project's files and dependencies to" the destination. If you test my repo, you will notice that project's files aren't copied to destination. The current project is copied instead as a dependency in destination's node_modules.

[jeanbmar]

@everett1992 I've noticed an issue with symlinks. To reproduce :

1. Install @tensorflow/tfjs-node in /opt/tfjs
2. Run npm copy /var/task from /opt/tfjs
3. Run ls -la /var/task/node_modules/@tensorflow/tfjs-node/deps/lib

Symlinks point to /opt/tfjs binaries

Symlinks are fine when doing mv /opt/tfjs/* /var/task/ instead of npm copy

[jeanbmar]

@everett1992 Have you lost interest in this PR? @wraithgar @darcyclarke Sorry for direct ping, what's the status of this PR and what can we do to push it further?