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.

Sign up for GitHub

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

Jump to bottom

Apply "files" npm whitelist at build time #11257

Closed
gaearon opened this issue Oct 17, 2017 · 8 comments
Closed

Apply "files" npm whitelist at build time #11257

gaearon opened this issue Oct 17, 2017 · 8 comments
Labels
Component: Build Infrastructure Difficulty: starter good first issue (taken)

Comments

@gaearon
Copy link
Collaborator

gaearon commented Oct 17, 2017

We should make our build step apply the same logic as npm does with whitelisting "files" field. This way issues like #11254 would get caught early.
@apravink
Copy link

Hi I'd like to work on this. Im a beginner and I would appreciate any pointers on where to start

@gaearon
Copy link
Collaborator Author

gaearon commented Oct 17, 2017

If you run yarn build in the repo root, it should start the build process.
The relevant part is probably here.

You will see that build/packages/ gets populated with content of npm packages.

I want to make sure that we respect "files" entry in package.json of each package, and skip files/folders that aren’t in the "files" whitelist. This way, if something is missing in "files" of package in packages/*/package.json, it won’t be copied to build/packages/*.

You can check that your change works with this example: #11254

yarn build -- react-art

Before the change, build/packages/react-art/lib/* would exist.

After the change, the lib folder would not be copied over to build/packages/react-art/ because it wasn’t in the "files" whitelist in packages/react-art/package.json.

@apravink
Copy link

Thanks for that!. That was really helpful!

The solution I am thinking involves parsing the package.json file for each of the packages and comparing the files/directories that are copied over to this parsed array.

Am I on the right track?

@gaearon
Copy link
Collaborator Author

gaearon commented Oct 18, 2017

Sounds about right.

yu-tian113 pushed a commit to yu-tian113/react that referenced this issue Oct 20, 2017
Issue facebook#11257 - compare files/folders in package/npm to "files…
81c2773 
…" in package.json, only copy whitelisted files/folders to build
@yu-tian113
Copy link
Contributor

Sorry, didn't mean to hijack the issue from @apravink, was just playing with a possible fix and pushed to my repo :).
Happy to discuss.

@apravink
Copy link

@yu-tian113 Haven't had a chance to work on this so if you have a working solution, please go ahead :)

@yu-tian113
Copy link
Contributor

Thanks for informing @apravink, I'll pick it up then. Please feel free to share any ideas or suggestions.

yu-tian113 pushed a commit to yu-tian113/react that referenced this issue Oct 31, 2017
Issue facebook#11257 - compare files/folders in package/npm to "files…
5dba04c 
…" in package.json, only copy whitelisted files/folders to build
yu-tian113 pushed a commit to yu-tian113/react that referenced this issue Oct 31, 2017
Issue facebook#11257 - compare files/folders in package/npm to "files…
deb06a0 
…" in package.json, only copy whitelisted files/folders to build (+1 squashed commit)

Squashed commits:
[d400198] handle directories in files list - e.g. "libs/"
yu-tian113 pushed a commit to yu-tian113/react that referenced this issue Nov 2, 2017
@yu-tian113
Issue facebook#11257 - compare files/folders in package/npm to "files…
1873fb1 
…" in package.json, only copy whitelisted files/folders to build (+1 squashed commit)

Squashed commits:
[d400198] handle directories in files list - e.g. "libs/"
yu-tian113 pushed a commit to yu-tian113/react that referenced this issue Nov 2, 2017
@yu-tian113
Issue facebook#11257 - compare files/folders in package/npm to "files…
d044250 
…" in package.json, only copy whitelisted files/folders to build (+1 squashed commit)

Squashed commits:
[d400198] handle directories in files list - e.g. "libs/"
yu-tian113 added a commit to yu-tian113/react that referenced this issue Nov 2, 2017
@yu-tian113
Issue facebook#11257 - compare files/folders in package/npm to "files…
af7f44f 
…" in package.json, only copy whitelisted files/folders to build (+1 squashed commit)

Squashed commits:
[d400198] handle directories in files list - e.g. "libs/" (+1 squashed commit)
Squashed commits:
[52a4d3b] Add in file check to make sure all entry points have equivalent files in ./npm folder.
Fail the build if any equivalent missing.
yu-tian113 added a commit to yu-tian113/react that referenced this issue Nov 2, 2017
@yu-tian113
Issue facebook#11257 - compare files/folders in package/npm to "files…
9a0189b 
…" in package.json, only copy whitelisted files/folders to build (+1 squashed commit)

Squashed commits:
[d400198] handle directories in files list - e.g. "libs/" (+1 squashed commit)
Squashed commits:
[52a4d3b] Add in file check to make sure all entry points have equivalent files in ./npm folder.
Fail the build if any equivalent missing.
yu-tian113 pushed a commit to yu-tian113/react that referenced this issue Nov 2, 2017
Issue facebook#11257 - compare files/folders in package/npm to "files…
507357f 
…" in package.json, only copy whitelisted files/folders to build (+1 squashed commit)

Squashed commits:
[d400198] handle directories in files list - e.g. "libs/" (+1 squashed commit)
Squashed commits:
[52a4d3b] Add in file check to make sure all entry points have equivalent files in ./npm folder.
Fail the build if any equivalent missing. (+1 squashed commit)
Squashed commits:
[e2d1516] Update glob to exclude *.fb.js, replace the check in if statement
yu-tian113 pushed a commit to yu-tian113/react that referenced this issue Nov 4, 2017
@yu-tian113
Issue facebook#11257 - compare files/folders in package/npm to "files…
3b4a6d8 
…" in package.json, only copy whitelisted files/folders to build (+1 squashed commit)

Squashed commits:
[d400198] handle directories in files list - e.g. "libs/" (+1 squashed commit)
Squashed commits:
[52a4d3b] Add in file check to make sure all entry points have equivalent files in ./npm folder.
Fail the build if any equivalent missing. (+1 squashed commit)
Squashed commits:
[e2d1516] Update glob to exclude *.fb.js, replace the check in if statement
yu-tian113 added a commit to yu-tian113/react that referenced this issue Nov 4, 2017
@yu-tian113
Issue facebook#11257 - compare files/folders in package/npm to "files…
cf2656b 
…" in package.json, only copy whitelisted files/folders to build (+1 squashed commit)

Squashed commits:
[d400198] handle directories in files list - e.g. "libs/" (+1 squashed commit)
Squashed commits:
[52a4d3b] Add in file check to make sure all entry points have equivalent files in ./npm folder.
Fail the build if any equivalent missing. (+1 squashed commit)
Squashed commits:
[e2d1516] Update glob to exclude *.fb.js, replace the check in if statement (+1 squashed commit)
Squashed commits:
[b4ddd28] Code refactor as per peer review.
yu-tian113 added a commit to yu-tian113/react that referenced this issue Nov 7, 2017
@yu-tian113
Issue facebook#11257 - compare files/folders in package/npm to "files…
90ea433 
…" in package.json, only copy whitelisted files/folders to build (+1 squashed commit)

Squashed commits:
[d400198] handle directories in files list - e.g. "libs/" (+1 squashed commit)
Squashed commits:
[52a4d3b] Add in file check to make sure all entry points have equivalent files in ./npm folder.
Fail the build if any equivalent missing. (+1 squashed commit)
Squashed commits:
[e2d1516] Update glob to exclude *.fb.js, replace the check in if statement (+1 squashed commit)
Squashed commits:
[b4ddd28] Code refactor as per peer review.
yu-tian113 added a commit to yu-tian113/react that referenced this issue Nov 13, 2017
@yu-tian113
Issue facebook#11257 - compare files/folders in package/npm to "files…
8a509ae 
…" in package.json, only copy whitelisted files/folders to build (+1 squashed commit)

Squashed commits:
[d400198] handle directories in files list - e.g. "libs/" (+1 squashed commit)
Squashed commits:
[52a4d3b] Add in file check to make sure all entry points have equivalent files in ./npm folder.
Fail the build if any equivalent missing. (+1 squashed commit)
Squashed commits:
[e2d1516] Update glob to exclude *.fb.js, replace the check in if statement (+1 squashed commit)
Squashed commits:
[b4ddd28] Code refactor as per peer review.
yu-tian113 added a commit to yu-tian113/react that referenced this issue Nov 13, 2017
@yu-tian113
# This is a combination of 2 commits.
7d08cbf 
# This is the 1st commit message:

Issue facebook#11257 - compare files/folders in package/npm to "files" in package.json, only copy whitelisted files/folders to build (+1 squashed commit)
Squashed commits:
[d400198] handle directories in files list - e.g. "libs/" (+1 squashed commit)
Squashed commits:
[52a4d3b] Add in file check to make sure all entry points have equivalent files in ./npm folder.
Fail the build if any equivalent missing. (+1 squashed commit)
Squashed commits:
[e2d1516] Update glob to exclude *.fb.js, replace the check in if statement (+1 squashed commit)
Squashed commits:
[b4ddd28] Code refactor as per peer review. (+2 squashed commits)
Squashed commits:
[854373f] Remove the else after the hard exit, make the code after unintended.
[e97dff1] Update to handle patterns in 'files' field, terminate the build if entry point not whitelisted, terminate the build if 'files' field is missing from package.json

# This is the commit message facebook#2:

Remove the else after the hard exit, make the code after unintended.
yu-tian113 added a commit to yu-tian113/react that referenced this issue Nov 26, 2017
@yu-tian113
Merge branch 'upstream/master' into facebook#11257-Apply-files-npm-wh…
1781fbc 
…itelist-at-build-time
@yu-tian113 yu-tian113 mentioned this issue Dec 2, 2017
Merged
gaearon pushed a commit that referenced this issue Dec 5, 2017
@yu-tian113 @gaearon
Issue #11257(Updated) - Change build process to include npm pack and …
6d24290 
…unpacking (#11750)

* Change build process to include npm pack and unpacking generated packages to corresponding build directories.

* Update function name, change to use os's default temp directory

* appending uuid to temp npm packaging directory.
@gaearon
Copy link
Collaborator Author

gaearon commented Dec 5, 2017

Fixed by #11750.

@gaearon gaearon closed this as completed Dec 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Component: Build Infrastructure Difficulty: starter good first issue (taken)
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@gaearon @yu-tian113 @apravink