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

Add bundlesize #353

Merged
merged 3 commits into from
Jan 23, 2020
Merged

Add bundlesize #353

merged 3 commits into from
Jan 23, 2020

Conversation

ctavan
Copy link
Member

@ctavan ctavan commented Jan 20, 2020

This will let us prevent regressions in bundlesize.

I chose to monitor the bundlesize of webpack and rollup build because this is what ultimately matters to the user. If we break something in our library that would e.g. break tree shaking in one of these bundlers we would want to notice.

Closes #355

@ctavan
Copy link
Member Author

ctavan commented Jan 20, 2020

@broofa how do you feel about adding bundlesize to this repo (see the additional check in "show all checks")?

@ctavan ctavan mentioned this pull request Jan 20, 2020
10 tasks
@broofa
Copy link
Member

broofa commented Jan 21, 2020

If this is based on #352, can you change the base to prepare-for-alpha-release so it's easier to review, please? (That makes sense, right?)

@ctavan
Copy link
Member Author

ctavan commented Jan 21, 2020

Uhm yeah, sorry. I rebased now, should be easy to review.

@ctavan ctavan force-pushed the add-bundlesize branch 12 times, most recently from 2cd9d2d to e413cd1 Compare January 21, 2020 22:02
@ctavan
Copy link
Member Author

ctavan commented Jan 21, 2020

Sorry for the noise of the many force pushes. I was experiencing flaky browser tests here. It seems that sometimes this await just never returns and the test just stalls there until it hits the jest timeout. I have no clue why, it seems to happen across almost all browsers (haven't seen it in Chrome so far). I'm a bit clueless 🤷‍♂ . Best bet so far is to just re-run the browser tests through the GitHub actions UI until they don't time out.

Anyways, the bundlesize part in this PR is ready for review now.

This will let us prevent regressions in bundlesize.

I chose to monitor the bundlesize of webpack and rollup build because
this is what ultimately matters to the user. If we break something in
our library that would e.g. break tree shaking in one of these bundlers
we would want to notice.
Copy link
Member

@broofa broofa left a comment

Choose a reason for hiding this comment

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

See #355

@ctavan
Copy link
Member Author

ctavan commented Jan 22, 2020

@broofa thanks for the suggestion. You are right, it is better to check the individual versions!

However I also want to ensure that we don't break the treeshaking capabilities which is why I don't want to specify the source esmodule files as entrypoints to rollup and webpack, but instead use separate files that explicitly do:

import { v1 as uuidv1 } from 'uuid';

Fine for you?

(I picked your commit and added an additional change commit, would squash merge everything in here in the end)

@ctavan ctavan requested a review from broofa January 22, 2020 13:09
@ctavan ctavan merged commit 4acaea4 into next Jan 23, 2020
@ctavan ctavan deleted the add-bundlesize branch January 27, 2020 22:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants