Use npmignore file on fetch #8010
Open
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 6 commits
March 23, 2020 02:48
cpojer
reviewed
Mar 23, 2020
| @@ -87,6 +92,20 @@ export async function fetchOneRemote( | |||
| } | |||
| } | |||
|
|
|||
| export async function unlinkIgnoredFiles(dest: string): Promise<void> { | |||
There was a problem hiding this comment.
Thank you for working on this! I'm not entirely sure if this is the right place to apply this filter. Could we instead apply this while we unpack so that ignored files are never even written? If we scope it to git dependencies only it shouldn't have an impact on install time.
There was a problem hiding this comment.
Your welcome! Okay, I will work on it.
There was a problem hiding this comment.
Thanks for this, how'd you get on? :)
There was a problem hiding this comment.
hey! sorry, i have been super busy, dealing with the conditions that comes with Covid, but I started to working on again
merceyz
added
the
fixed-in-modern
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
I created this PR for fix 2090
Normally yarn doesn't use the .npmignore and install unnecessary files to our node_modules In this branch, while fetching process, yarn read the .npmignore file and delete ignored files with respecting the globs.
I added the del package to delete files with respecting globs, on the other hand, the other way was using the fs.unlink with a loop of file list on npmignore. But I am not sure if it lets us do the proccess without any error, for example deleting a directory or files except the ones that wanted to be kept with NEGATE mark
Test plan
I wrote the test on /tests/fetchers.js/ file. You can run it with
alias testnow="node --max_old_space_size=4096 node_modules/jest/bin/jest.js --verbose __tests__/fetchers.js"(it will only make test of fetchers.js run)