Skip to content

Fix #2822: Respect files package content when using file: dependencies #8249

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

kav
Copy link

@kav kav commented Jul 22, 2020

Summary
Fixes #2822

When using file: dependencies yarn ignores the files reference in the package.json and copies over the whole folder. This causes all kinds of issues for folks. See #2822 and related for the various ways it plays havok.

This change addresses that by checking the package for a files array and if present only copying the files found there, along with the package.json file itself. I did update the broken contributing links mentioned below as well but there is another pr with those changes so I dropped those

Test plan
Added several new tests to fetchers.js. For a concrete test and repro see the original issue here: #2822

Notes
I'm new to the codebase so I had a few notes/questions.

  1. should I be hardcoding package.json? I saw a few examples of this in the git-fetcher so figured it's ok but it seems like I should ask the manifest for its _filename or something.
  2. I'd love to refactor the fs.copy command to correctly handle sources as files and destinations as directories as is idiomatic but given this is a first PR and that seems core I simply made sure I was copying files to files here.
  3. Ideally we should be following a similar pattern to the git-fetcher and packing and unpacking to get full ignore and build support. Happy to refactor to that next but figured it might be a bit much for this change.
  4. Generally, this seems like a pretty straightforward issue. Any ideas why none of the commenters submitted a PR over the three years it's been open? Other than a broken link or two, so far, contributing to yarn seems fairly frictionless.

@kav kav force-pushed the copy-fetch-respects-files branch 3 times, most recently from 8d5cbb0 to 0e2c2cf Compare July 22, 2020 00:19
@kav kav changed the title Respect files package content when using file: dependencies Fix #2822: Respect files package content when using file: dependencies Jul 22, 2020
@kav kav force-pushed the copy-fetch-respects-files branch 2 times, most recently from 504d763 to fd96470 Compare July 22, 2020 15:39
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.

yarn install ignores files from local dependencies
1 participant