Add output size check in CI #10090
Add output size check in CI #10090
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have you on file. In order for us to review and merge your code, please sign up at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need the corporate CLA signed. If you have received this in error or have any questions, please contact us at cla@fb.com. Thanks! |
|
Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Facebook open source project. Thanks! |
package.json
Outdated
| @@ -113,7 +114,7 @@ | |||
| "linc": "git diff --name-only --diff-filter=ACMRTUB `git merge-base HEAD master` | grep '\\.js$' | xargs eslint --", | |||
| "lint": "node ./scripts/tasks/eslint.js", | |||
| "postinstall": "node node_modules/fbjs-scripts/node/check-dev-engines.js package.json", | |||
| "test": "jest", | |||
| "test": "jest && bundlesize", | |||
There was a problem hiding this comment.
I don't think this will work. jest won't perform the build. We have a separate task that's used for CI.
Somewhere here is probably the right place for it.
package.json
Outdated
| }, | ||
| "bundlesize": [ | ||
| { | ||
| "path": "dist/react.min.js", |
There was a problem hiding this comment.
Did you verify this? I don't think a file with this name gets generated on master.
You probably want to add build/dist/react.production.min.js and build/dist/react-dom.production.min.js here.
|
@gaearon Oh I was just trying out circleCI, I'm far from done here 😄 Tagged the PR with |
|
@gaearon It's done. A circle CI integration with build status is coming soon too. I'll raise another PR then. |
|
Can we roll everything into one PR instead of splitting? |
Sure, you'll have to wait for a few days though |
|
What do you think about size-limit. It was created for same purpose. But it was created 2 day early :D. Size Limit has 3 differences:
|
|
@gaearon I decided to merge with bundlesize and send all Size Limit benefits in PR siddharthkp/bundlesize#26 |
|
@gaearon hi 😄 |
|
Sorry I forgot about this. I appreciate the effort but we'll probably go with #11865 instead. Since it lets us do other checks as well, and display more info. |
|
No worries! |
What:
Basically this in the terminal and CI logs:
Why:
This will help in keeping the library tiny.
How:
bundlesizeinnpm testThere is a tiny bit of configuration needed:
The config sits inside
package.json, the threshold is set to 10kB right now.Sadly,
bundlesizedoes not build status for circleCI yet, I'll work on it and submit another PR which gives you this: