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

misc: npmignore all of dist/ except standalone report #12855

Merged
merged 2 commits into from
Aug 4, 2021
Merged

Conversation

brendankenny
Copy link
Member

fixes #8084

we have to explicitly ignore dist/* (not just dist/) to then be able to unignore dist/report/standalone.js

@brendankenny brendankenny requested a review from a team as a code owner August 4, 2021 00:49
@brendankenny brendankenny requested review from adamraine and removed request for a team August 4, 2021 00:49
@google-cla google-cla bot added the cla: yes label Aug 4, 2021
.npmignore Outdated
@@ -54,6 +53,8 @@ results.html
*.lcov

# generated files needed for publish
dist/*
dist/report/*
Copy link
Member Author

Choose a reason for hiding this comment

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

this lets us ignore the other files in dist/report/, which I assume we want to do

npm notice 177.4kB dist/report/bundle.js                                                               
npm notice 165.6kB dist/report/psi.js                                                                  
npm notice 74.7kB  dist/report/standalone.js                                                           
npm notice 12.3kB  dist/report/treemap.js                                                              
npm notice 188.3kB dist/report/viewer.js  

Copy link
Collaborator

@connorjclark connorjclark left a comment

Choose a reason for hiding this comment

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

wondering how this works, reading the docs (and making the assumption that npmignore works like gitignore) ...

From my reading of this page, nothing suggests the behavior we saw. This is the closest related thing mentioned:

[... about the !include negation pattern ...] It is not possible to re-include a file if a parent directory of that file is excluded. Git doesn’t list excluded directories for performance reasons, so any patterns on contained files have no effect, no matter where they are defined.

https://git-scm.com/docs/gitignore#:~:text=it%20is%20not%20possible%20to%20re-include%20a%20file%20if%20a%20parent%20directory%20of%20that%20file%20is%20excluded.

While I find this paragraph difficult to parse, I don't think it explains the behavior we saw, which is basically "negating a file inside a directory that has been ignored will unignore the directory"

@brendankenny
Copy link
Member Author

Yeah, the docs aren't clear at all. It's more like if re-including a file the parent directory is no longer ignored.

dist/* bypasses it by matching paths and not the directory, but now reading it, looks like dist/** would let us skip the extra dist/report/*.

But seriously, the explanations on that page are terrible :)

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

Successfully merging this pull request may close these issues.

audit what we publish to npm
4 participants