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

fix: 修复现代浏览器不支持catch optional #10052

Closed

Conversation

ykant
Copy link

@ykant ykant commented Sep 9, 2022

Description

支持module,但是不支持 catch optional的低版本浏览器会导致程序不能正常运行

在浏览器判断处新增catch optional场景

Additional context


What is the purpose of this pull request?

  • Bug fix
  • New Feature
  • Documentation update
  • Other

Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Read the Pull Request Guidelines and follow the Commit Convention.
  • Check that there isn't already a PR that solves the problem the same way to avoid creating a duplicate.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@ykant ykant changed the title 修复现代浏览器不支持catch optional fix: 修复现代浏览器不支持catch optional Sep 9, 2022
@sapphi-red
Copy link
Member

I don't think this is needed. Optional catch binding is supported by all default target browsers.

export const ESBUILD_MODULES_TARGET = [
'es2020', // support import.meta.url
'edge88',
'firefox78',
'chrome87',
'safari13' // transpile nullish coalescing
]

https://caniuse.com/mdn-javascript_statements_try_catch_optional_catch_binding

@ykant
Copy link
Author

ykant commented Sep 9, 2022

I fixed the problem in my project just a few hours ago(Android 9 Webview), but my solution is ugly.
`
<script>
window.__vite_is_modern_browser_ugly = false;
try {
eval('try {} catch {}');
window.__vite_is_modern_browser_ugly = true;
} catch (e) {}
window.addEventListener('load', function() {
if (window.__vite_is_modern_browser_ugly) return;
var e = document.getElementById('vite-legacy-polyfill'),
n = document.createElement('script');
(n.src = e.src),
(n.onload = function () {
System.import(document.getElementById('vite-legacy-entry').getAttribute('data-src'));
}),
document.body.appendChild(n);
})
</script>

`

@sapphi-red sapphi-red added p4-important Violate documented behavior or significantly improves performance (priority) plugin: legacy labels Sep 9, 2022
@sapphi-red
Copy link
Member

sapphi-red commented Sep 9, 2022

OK. I now understand the problem.
Our default target browsers are not the same with the browsers supporting ESM + dynamic import + import.meta.
This is different from what we document.

Current default target browsers (A): es2020, edge88, firefox78, chrome87, safari13
browsers supporting ESM + dynamic import + import.meta (B): es2020, edge79, firefox67, chrome64, safari11.1

So features supported in A but not in B will be used in modern chunks and will break users using B.
Optional catch binding is supported from Chrome 66 and that is one of the affected features.

It seems the difference comes from #2976.

@sapphi-red
Copy link
Member

I think there's three options here.

  1. Change build.target to B
  2. Change build.target to B by plugin-legacy
  3. Check every feature supported between A and B, and detect whether it is supported in detectModernBrowserCode

@ykant
Copy link
Author

ykant commented Sep 10, 2022

Thank you for the answer :)

@yoyo837
Copy link
Contributor

yoyo837 commented Nov 28, 2023

I think there's three options here.

  1. Change build.target to B
  2. Change build.target to B by plugin-legacy
  3. Check every feature supported between A and B, and detect whether it is supported in detectModernBrowserCode

So, in current, option 2 is already used as the fix, right?

@sapphi-red
Copy link
Member

Yes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
p4-important Violate documented behavior or significantly improves performance (priority) plugin: legacy
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants