-
Notifications
You must be signed in to change notification settings - Fork 3.4k
-sEXPORT_ES6 and multithreading too old browsers. #25408
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
-sEXPORT_ES6 and multithreading too old browsers. #25408
Conversation
…ith too old target browser versions.
Naively adding So changed this PR to establish ES6 Module Workers as a feature instead. |
#endif | ||
#if MIN_SAFARI_VERSION < 150000 | ||
#error new Worker() supports ECMAScript module only starting from Safari 15. Pass -sMIN_SAFARI_VERSION=150000 to target -sEXPORT_ES6 with -pthread. See https://caniuse.com/mdn-api_worker_worker_ecmascript_modules | ||
#endif |
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.
It is even possible to hit these error now that the feature is added? i.e. -sEXPORT_ES6
+ -pthreads
will now force the correct versions or fail, right?
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 pondering the same, so wanted to add these in to verify that won't be the case. Doesn't hurt to have them?
Or I could remove them - but seems like they're safe to have, they could trigger on possible refactors down the road.
Without this change it might appear that this is where the check/enforcement is happening, but its actually done by the feature_matrix.
Abort build if attempting to target -sEXPORT_ES6 and multithreading with too old target browser versions.Add ES6 Module Workers as feature in the feature matrix.