Skip to content

Conversation

nlf
Copy link
Contributor

@nlf nlf commented Apr 13, 2022

this adds an installLinks flag to arborist, which when set instructs arborist to pack and extract file: links rather than creating a symlink to their directory. this also causes us to reify the dependencies of the directory.

a follow up pull request will add the config definition and documentation for the flag. i changed my mind about a separate pull request, i'll be adding some commits to this one instead.

for #2339
for npm/rfcs#150

@nlf nlf requested a review from a team as a code owner April 13, 2022 22:11
@nlf nlf force-pushed the nlf/arborist-pack-links branch from 9c1d7e0 to c5adba2 Compare April 14, 2022 17:06
@nlf nlf changed the title feat(arborist): add support for installLinks Apr 14, 2022
Copy link
Contributor

@ljharb ljharb left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd really prefer if link: is for symlinks and file: is for copying files, like it used to be before npm 5 broke it, but another way to get file copying is still great

@nlf
Copy link
Contributor Author

nlf commented Apr 14, 2022

I'd really prefer if link: is for symlinks and file: is for copying files, like it used to be before npm 5 broke it, but another way to get file copying is still great

that would be great, but also a breaking change. i'd personally really like to see npm@9 go back to pack+install for file: deps and require the use of npm link to create a link. this is an interim feature to unblock folks who depend on the old behavior without breaking folks who have already adjusted to the new.

nlf added 6 commits April 14, 2022 11:31
when set, installLinks instructs arborist to pack and extract a file:
dependency rather than creating a symlink to it. this has the effect of
also installing the dependencies for the linked dependency, though if
local changes are made it also requires the user to reinstall the
package
@nlf nlf force-pushed the nlf/arborist-pack-links branch from 1765ead to cdec8eb Compare April 14, 2022 18:33
@lukekarrys
Copy link
Contributor

This looks great! 🎉

@nlf Does it close either #2239 or npm/rfcs#150?

@nlf
Copy link
Contributor Author

nlf commented Apr 19, 2022

@nlf Does it close either #2239 or npm/rfcs#150?

in theory, yes, but i didn't want it to automatically close the issues when it lands so i can follow up in both places

@lukekarrys lukekarrys merged commit 0004be8 into latest Apr 19, 2022
@lukekarrys lukekarrys deleted the nlf/arborist-pack-links branch April 19, 2022 23:22
@ruyadorno ruyadorno mentioned this pull request Apr 26, 2022
@ThisIsMissEm
Copy link

Also perhaps related to @ljharb's comment: npm/feedback#667

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

6 participants