-
Notifications
You must be signed in to change notification settings - Fork 127
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
fix: improve ES6 bundling and support #1525
Conversation
This was missed in the ES6 switcheroo We then will also add IE11 for a specific bundle here https://github.com/PostHog/posthog-js/blob/4ac2e5dcaed08240b7ecddc5310aefed0ab9660a/rollup.config.js#L25-L28
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Size Change: +16.3 kB (+0.53%) Total Size: 3.13 MB
ℹ️ View Unchanged
|
oh, very interesting that makes tests fail 🤯 |
rollup.config.js
Outdated
// Explicitly included so we transform 1 ** 2 to Math.pow(1, 2) for ES6 compatability | ||
'@babel/plugin-transform-exponentiation-operator', |
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 think the browsers to support navigator.webdriver was tricking babel into thinking some ES2017 features were ok
including this forces babel to transform this
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.
Low context approval from me. I guess the only questions is if docs need to be updated or not
"safari > 12" | ||
], | ||
"es5": [ | ||
"defaults", |
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 was curious as to what defaults mean: https://github.com/browserslist/browserslist?tab=readme-ov-file#full-list
Can we put an old version of ios_saf here? I would guess that it's not needed given how much support IE11 would need, but it'd be better to be explicit. TBH I think we could copy the whole list above, given that these are ORd together, so there's no harm in adding versions.
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.
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 was curious as to what defaults
"defaults"
is just short-hand for the values we used to explicitly set.
I think we could copy the whole list above, given that these are ORd together, so there's no harm in adding versions.
i've been fiddling with this to get CI to pass 🙈
i don't understand why IE 11 is freezing in testcafe now so have been changing values around 🫠
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 agree the explicit list is better than "defaults" just hitting it with a hammer to try and understand what's happening :))
this failing IE 11 test is super annoying -> #1542 |
we can be much more explicit about what we support
and test that the bundle is ES6 only
which will extend browser support by >10% of global audience