-
Notifications
You must be signed in to change notification settings - Fork 361
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
chore: Bump (most) deps #1081
base: master
Are you sure you want to change the base?
chore: Bump (most) deps #1081
Conversation
Co-authored-by: Jason Miller <[email protected]>
🦋 Changeset detectedLatest commit: 68d47e0 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
Size Change: +1.46 kB (+5.7%) 🔍 Total Size: 27.1 kB
|
@@ -1,5 +1,6 @@ | |||
{ | |||
"compilerOptions": { | |||
"moduleResolution": "node", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
tslib
otherwise can't be found, oddly. This should be the default but maybe rtp2 is doing something odd.
- name: compressed-size-action | ||
uses: preactjs/compressed-size-action@v2 | ||
with: | ||
pattern: 'test/fixtures/**/dist/!(*.map)' | ||
build-script: 'test' | ||
repo-token: '${{ secrets.GITHUB_TOKEN }}' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test suite already covers this really, we don't need a separate readout for file size bumps in a PR comment.
Also this is not aligning w/ the 'Test' CI action, probably Node version differences that isn't worth trying to hack around
This is IMO very important to merge. I need to manualy override "rollup-plugin-typescript2" to latest (0.36.0) everywhere I use microbundle, else it can't compile typescript because it can't find the types:
I had the solution found a year or so ago, but forgot about it and did get cought by it again and wasted a few hours to find out how to solve it. Workaround: Add this to your
If you use npm workspaces make sure it is overwritten and not existing anymore in |
@developit If you have a few minutes, would appreciate a peek at this and potentially #1082 & #1083 -- we're starting to hit dep rot |
@@ -2320,26 +2319,26 @@ modern-generators | |||
|
|||
|
|||
Build \\"modern-generators\\" to dist: | |||
248 B: modern-generators.js.gz | |||
201 B: modern-generators.js.br | |||
2.57 kB: modern-generators.js.gz |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we ok with this massive increase?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey, long time no see! Hope all is well & good w/ you.
Mentioned this above in a resolved comment:
Oh nice,
7.18.0
ended up inlining regenerator as a normal helper, dropping the global: babel/babel#14538This is a sizable increase but IMO it's completely reasonable.
I still think this is reasonable, but admittedly, I was very much not a fan of Babel's previous behavior.
Do you disagree with this? I suppose we could work around it and revert to previous behavior if we really needed to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All good, how about you?
So if I'm understanding it correctly, the previous size was somewhat misleading because the runtime was needed?
But now that it's inlined it will be duplicated for each library built this way
Maybe @developit could give his opinion
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not bad, thanks!
You're right, yep. regeneratorRuntime
was a unique case in that instead of being inlined, as every other Babel helper is, it was consumed as a global with the expectation that the user makes it available. This tripped up a great many users over the years (Preact org issues, for reference, and you can find many hundred similar issues elsewhere) as it was a bit odd and had questionable DX.
Indeed though, like all Babel helpers this can end up duplicated and it's particularly bad if it is -- even gzip isn't going to save you there.
That being said, I'm not sure there's a whole heck of a lot that we can do, or nothing comes to mind at least. If a user is bundling their deps with Microbundle, and one of those deps has this helper inlined, there's still duplication whether we stick with the old global or duplicate it. In a perfect world every module would be distributing modern bundles with minimal transpiling/helpers, but... :/ Heck, even our "modern" is still ES2017, could probably do with a bump to 2019-2020?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am wondering why the modern output has regenerator in it at all, since ES2017 supports generators.
Maybe we need to configure Babel "assumptions"
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is perhaps a poorly named test case: modern-generators
is the package name, which forms the output file name. modern-generators.modern.js
is the modern output, and it's unchanged.
Caught me off guard once or twice too, could rename it to just generators
I suppose.
Supersedes #969