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

Add a modernizr check for WebAssembly support #27776

Merged
merged 1 commit into from
Jul 18, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/vector/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,10 @@ function checkBrowserFeatures(): boolean {
// and older Firefox has the former but not the latter, so we add our own.
window.Modernizr.addTest("intlsegmenter", () => typeof window.Intl?.Segmenter === "function");

// Basic test for WebAssembly support. We could also try instantiating a simple module,
// although this would start to make (more) assumptions about how rust-crypto loads its wasm.
window.Modernizr.addTest("wasm", () => typeof WebAssembly === "object" && typeof WebAssembly.Module === "function");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this is sufficient, I believe rust crypto needs the modern API for loading it also. e.g. https://developer.mozilla.org/en-US/docs/WebAssembly/JavaScript_interface/instantiateStreaming_static rather than https://developer.mozilla.org/en-US/docs/WebAssembly/JavaScript_interface/instantiate_static (FF 58 vs FF 52)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

So it actually doesn't, because it seems the lowest-common-denominator way was the only reliable way of loading it portably, so it actually uses WebAssembly.instantiate() in practice. We could certainly try loading a simple module using that: that would cover CSP issues too, but we'd be assuming more details of how the library works, which could change without us knowing so we could end up saying it won't work even when it actually would.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What @dbkr says is true: we wanted to package the wasm blob in a way that it could be loaded by applications using the js-sdk without a whole lot of custom packaging. Doing that, whilst allowing EW to stream it, seemed approximately impossible at the time, mostly because everything in the JS ecosystem is awful. Hence, we use WebAssembly.instantiate rather than WebAssembly.instantiateStreaming.

(It may be worth revisiting that situation now that we have Webpack 5 and the JS-SDK is built as ES modules, but I digress.)

What we do do is use WebAssembly.instantiate rather than calling the WebAssembly.Module constructor, which loads the wasm synchronously. But both methods seems to have landed in the same version of FF, so I don't think it makes any difference.


const featureList = Object.keys(window.Modernizr) as Array<keyof ModernizrStatic>;

let featureComplete = true;
Expand Down Expand Up @@ -240,6 +244,10 @@ async function start(): Promise<void> {
}

start().catch((err) => {
// If we get here, things have gone terribly wrong and we cannot load the app javascript at all.
// Show a different, very simple iframed-static error page. Or actually, one of two different ones
// depending on whether the browser is supported (ie. we think we should be able to load but
// failed) or unsupported (where we tried anyway and, lo and behold, we failed).
logger.error(err);
// show the static error in an iframe to not lose any context / console data
// with some basic styling to make the iframe full page
Expand Down
Loading