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

Fixup env.support for ChromeAndroid #8447

Merged
merged 1 commit into from Sep 6, 2022
Merged

Conversation

mischnic
Copy link
Member

@mischnic mischnic commented Sep 6, 2022

Closes #8444

Browser support data for mobile browsers is a sham anyway (e.g. https://caniuse.com/arrow-functions), but with this adding a query with ChromeAndroid to browserslist at least doesn't break env.support("arrow-functions") anymore. And I'm not even sure what the correct version for that added entry is, so I just used the conservative 50 from android. Now every feature has an entry for and_chr

That problem manifested here:

buildFunctionExpression(args: Array<string>, expr: string): string {
return this.bundle.env.supports('arrow-functions', true)
? `(${args.join(', ')}) => ${expr}`
: `function (${args.join(', ')}) { return ${expr}; }`;
}

MDN actually has historical data: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Functions/Arrow_functions#browser_compatibility

  • Makes me wonder if we should behave like Browserslist mobileToDesktop setting?

@parcel-benchmark
Copy link

parcel-benchmark commented Sep 6, 2022

Benchmark Results

Kitchen Sink ✅

Timings

Description Time Difference
Cold 1.88s +76.00ms
Cached 416.00ms -16.00ms

Cold Bundles

Bundle Size Difference Time Difference
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 309.00ms +25.00ms ⚠️
dist/legacy/parcel.7cdb0fad.webp 102.94kb +0.00b 311.00ms +26.00ms ⚠️
dist/modern/parcel.7cdb0fad.webp 102.94kb +0.00b 310.00ms +25.00ms ⚠️
dist/legacy/index.2c76ad23.js 1.66kb +0.00b 520.00ms +43.00ms ⚠️
dist/legacy/index.8aaa89c9.js 1.20kb +0.00b 520.00ms +43.00ms ⚠️
dist/modern/index.6be20f01.js 1.13kb +0.00b 520.00ms +43.00ms ⚠️
dist/legacy/index.html 826.00b +0.00b 659.00ms +57.00ms ⚠️
dist/modern/index.html 749.00b +0.00b 658.00ms +57.00ms ⚠️
dist/legacy/index.b8ae99ba.css 94.00b +0.00b 323.00ms +30.00ms ⚠️
dist/modern/index.31cedca9.css 94.00b +0.00b 322.00ms +30.00ms ⚠️

Cached Bundles

No bundle changes detected.

React HackerNews 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

AtlasKit Editor 🚨

Timings

Description Time Difference
Cold FAILED -0.00ms
Cached FAILED -0.00ms

Cold Bundles

No bundles found, this is probably a failed build...

Cached Bundles

No bundles found, this is probably a failed build...

Three.js ✅

Timings

Description Time Difference
Cold 8.26s +6.00ms
Cached 346.00ms +7.00ms

Cold Bundles

No bundle changes detected.

Cached Bundles

No bundle changes detected.

Click here to view a detailed benchmark overview.

@mischnic mischnic merged commit 55140b6 into v2 Sep 6, 2022
@mischnic mischnic deleted the env-arrow-support-android branch September 8, 2022 11:58
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.

AndroidChrome in browserslist breaks env.supports checks
3 participants