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

Whitelist files to publish + new GitHub workflow to auto tag / release / publish #153

Merged
merged 3 commits into from Oct 26, 2021

Conversation

MrChocolatine
Copy link
Member

@MrChocolatine MrChocolatine commented Oct 19, 2021

Build

Whitelist files to include on publish (#153)

To reduce the final bundle size.

Before (see workflow run) After (see workflow run)
npm notice === Tarball Details ===
npm notice package size:  7.8 kB
npm notice unpacked size: 21.7 kB
npm notice total files:   22
npm notice === Tarball Details ===
npm notice package size:  6.7 kB
npm notice unpacked size: 18.4 kB
npm notice total files:   18

CI

New GitHub workflow to automatically create a new tag, a new release and publish to NPM (#153)

Anytime something is merged to master, this new workflow will be triggered:

  1. If a new version is detected (based on the version entry of the package.json), it is used to create a new git tag.

  2. A new GitHub release is created, based on this tag ⬆️

  3. The add-on is published to NPM

@MrChocolatine MrChocolatine force-pushed the ci-create-workflow-publish-npm branch 10 times, most recently from a108679 to f7e7289 Compare October 19, 2021 17:55
@MrChocolatine MrChocolatine changed the title ci: create workflow publish-npm Auto npm publish on new release + reduce size of published files Oct 19, 2021
@MrChocolatine MrChocolatine marked this pull request as ready for review October 19, 2021 17:59
@MrChocolatine MrChocolatine requested a review from a team as a code owner October 19, 2021 17:59
@MrChocolatine MrChocolatine changed the title Auto npm publish on new release + reduce size of published files Auto npm publish on new release + whitelist files to publish Oct 19, 2021
@GreatWizard
Copy link
Member

Form your dry run :

npm notice 0 addon/.gitkeep
npm notice 0 app/.gitkeep
npm notice 79B app/initializers/embedded.js
npm notice 88B app/instance-initializers/embedded.js
npm notice 63B app/services/embedded.js
npm notice 87B config/environment.js
npm notice 615B index.js
npm notice 4.0kB package.json
npm notice 1.2kB tsconfig.json
npm notice 1.1kB LICENSE.md
npm notice 363B app/README.md
npm notice 5.5kB README.md
npm notice 240B initializers/embedded.d.ts
npm notice 246B instance-initializers/embedded.d.ts
npm notice 354B services/embedded.d.ts
npm notice 3.0kB addon/initializers/embedded.ts
npm notice 379B addon/instance-initializers/embedded.ts
npm notice 851B addon/services/embedded.ts
npm notice 199B types/global.d.ts
npm notice 126B types/dummy/index.d.ts

I think we can keep the npmignore to remove the .gitkeep files
and same about the readme.md ? I don't think anyone is gonna read it from node_modules tbh

@MrChocolatine
Copy link
Member Author

I think we can keep the npmignore to remove the .gitkeep files

I thought about that but when I made some tests when having a .npmignore that ignores .gitkeep files and the entry files in the package.json, the .gitkeep files were still included in the output.

And because .gitkeep files size is 0 bytes, I thought it would not be a big deal to actually keep them.

@MrChocolatine
Copy link
Member Author

MrChocolatine commented Oct 20, 2021

and same about the readme.md ? I don't think anyone is gonna read it from node_modules tbh

This file, and some others, are always and automatically included during the publish, no matter what settings we use. (I also wanted to ignore it at first...)

https://docs.npmjs.com/cli/v6/configuring-npm/package-json#files

@MrChocolatine MrChocolatine marked this pull request as draft October 20, 2021 10:14
@MrChocolatine MrChocolatine added the ci Continuous Integration label Oct 20, 2021
@MrChocolatine MrChocolatine force-pushed the ci-create-workflow-publish-npm branch 2 times, most recently from b248cb0 to 5689f5a Compare October 20, 2021 10:24
Comment on lines +30 to +36
"files": [
"/addon",
"/app",
"/config/environment.js",
"/initializers",
"/instance-initializers",
"/services",
"/types",
"/index.js",
"/tsconfig.json"
],
Copy link
Member Author

Choose a reason for hiding this comment

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

As a reminder about what is going to happen, "what will be included in the publish?".

Initially, when this PR started and we did not have this entry package.json/files, but only our .npmignore, the files included in the publish were:

https://github.com/peopledoc/ember-cli-embedded/runs/3941184198?check_suite_focus=true#step:5:17

npm notice === Tarball Contents ===
npm notice 22B   .github/CODEOWNERS
npm notice 79B   app/initializers/embedded.js
npm notice 88B   app/instance-initializers/embedded.js
npm notice 63B   app/services/embedded.js
npm notice 87B   config/environment.js
npm notice 615B  index.js
npm notice 3.8kB package.json
npm notice 1.2kB tsconfig.json
npm notice 1.1kB LICENSE.md
npm notice 363B  app/README.md
npm notice 5.5kB README.md
npm notice 240B  initializers/embedded.d.ts
npm notice 246B  instance-initializers/embedded.d.ts
npm notice 354B  services/embedded.d.ts
npm notice 3.0kB addon/initializers/embedded.ts
npm notice 379B  addon/instance-initializers/embedded.ts
npm notice 851B  addon/services/embedded.ts
npm notice 199B  types/global.d.ts
npm notice 126B  types/dummy/index.d.ts
npm notice 2.1kB .github/workflows/ci.yml
npm notice 691B  .github/dependabot.yml
npm notice 683B  .github/workflows/publish-npm.yml

Some files of these files are always and automatically included during a publish, no matter what settings we apply (this is a rule from npm itself, we can't do anything):

https://docs.npmjs.com/cli/v6/configuring-npm/package-json#files

package.json
README
CHANGES / CHANGELOG / HISTORY
LICENSE / LICENCE
NOTICE
The file in the "main" field

Now, if we apply this rule from npm to our initial list of files + our whitelist from package.json/files, we should end up with:

npm notice === Tarball Contents ===
npm notice 79B   app/initializers/embedded.js
npm notice 88B   app/instance-initializers/embedded.js
npm notice 63B   app/services/embedded.js
npm notice 87B   config/environment.js
npm notice 615B  index.js
npm notice 3.8kB package.json
npm notice 1.2kB tsconfig.json
npm notice 1.1kB LICENSE.md
npm notice 363B  app/README.md
npm notice 5.5kB README.md
npm notice 240B  initializers/embedded.d.ts
npm notice 246B  instance-initializers/embedded.d.ts
npm notice 354B  services/embedded.d.ts
npm notice 3.0kB addon/initializers/embedded.ts
npm notice 379B  addon/instance-initializers/embedded.ts
npm notice 851B  addon/services/embedded.ts
npm notice 199B  types/global.d.ts
npm notice 126B  types/dummy/index.d.ts

@MrChocolatine MrChocolatine force-pushed the ci-create-workflow-publish-npm branch 6 times, most recently from 631b6e7 to 5c39c9a Compare October 20, 2021 11:55
@MrChocolatine MrChocolatine force-pushed the ci-create-workflow-publish-npm branch 11 times, most recently from d4c9fdc to 27cd419 Compare October 21, 2021 07:52
@MrChocolatine MrChocolatine changed the title Auto npm publish on new release + whitelist files to publish Whitelist files to publish + GitHub workflow to auto tag / release / publish Oct 21, 2021
@MrChocolatine MrChocolatine marked this pull request as ready for review October 21, 2021 08:09
@MrChocolatine
Copy link
Member Author

MrChocolatine commented Oct 21, 2021

@MrChocolatine MrChocolatine marked this pull request as draft October 25, 2021 07:43
@MrChocolatine MrChocolatine force-pushed the ci-create-workflow-publish-npm branch 3 times, most recently from 68dd4ed to 0183775 Compare October 26, 2021 09:50
@MrChocolatine MrChocolatine marked this pull request as ready for review October 26, 2021 09:56
@MrChocolatine MrChocolatine changed the title Whitelist files to publish + GitHub workflow to auto tag / release / publish Whitelist files to publish + new GitHub workflow to auto tag / release / publish Oct 26, 2021
1. Reduce the size of the final output.

2. Overall, a whitelist of files should be prefer over a blacklist to
   make things explicit about what exactly is going to be published.

Useful readings:
- https://docs.npmjs.com/cli/v6/configuring-npm/package-json#files
- https://docs.npmjs.com/cli/v6/commands/npm-publish
@MrChocolatine MrChocolatine merged commit 23c3e94 into master Oct 26, 2021
@MrChocolatine MrChocolatine deleted the ci-create-workflow-publish-npm branch October 26, 2021 15:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ci Continuous Integration
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants