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

feat: add support for installing instead of symlinking file: dependencies #4745

Merged
merged 6 commits into from
Apr 19, 2022

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 feat: add support for installing instead of symlinking file: dependencies 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
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants