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

Polyfill for Promise might breaks wasm on edge #815

Closed
clouds56 opened this issue Apr 21, 2020 · 7 comments
Closed

Polyfill for Promise might breaks wasm on edge #815

clouds56 opened this issue Apr 21, 2020 · 7 comments

Comments

@clouds56
Copy link

We might fix for WebAssembly.instantiateStreaming.

(function() {
	var proxied = WebAssembly.instantiateStreaming;
	WebAssembly.instantiateStreaming = function(response, importObject) {
		return Promise.resolve(response).then(res => {
			return proxied(res, importObject);
		});
	};
})();

See also webpack/webpack#10560

@JakeChampion
Copy link
Contributor

That looks like a bug in WebAssembly.instantiateStreaming and not in the Promise polyfill. It looks like you need to polyfill WebAssembly.instantiateStreaming; in browsers which don't support it receiving a Promise.

@clouds56
Copy link
Author

But before core-js polyfills Promise, WebAssembly.instantiateStreaming of Edge do work with native Promise.
If core-js breaks something and refused to fix we should at least document it (something like known issues).
It takes lots of time for us to track down what's happening inside.

@JakeChampion
Copy link
Contributor

I found this comment about Edge and core-js Promise #579 (comment)

@clouds56
Copy link
Author

Thanks for related link.
IIRC, it is a comment that why we wrap fetch, but not mention the consequence of breaking built-in APIs.

I'm not a expert at js but I think it is definitely a bug in core-js: it fakes Promise but not be compatible with built-in APIs. What's worse it wraps fetch (another built-in API) and you could not get a native Promise in this case.
An idealistic solution should be make polyfilled Promise recognized as Promise in built-in API like WebAssembly.instantiateStreaming at least on Edge.

@slowcheetah
Copy link
Contributor

@clouds56 in case with strict type checking we can't do anything else like hack that you provide in first message. See #178 (comment) and the whole issue.

methyl added a commit to surferseo/intl-segmenter-polyfill that referenced this issue Jun 14, 2020
@zloirock
Copy link
Owner

zloirock commented May 5, 2021

At this moment, in case of polyfilled Promise, core-js patch native Promise implementation for make native Promise instances look like core-js Promise instances:

nativePromiseBasedAPI();                         // -> it's a native, not `core-js`, promise, but:
nativePromiseBasedAPI() instanceof Promise;      // -> true
nativePromiseBasedAPI().constructor === Promise; // -> true
nativePromiseBasedAPI().then(fn);                // -> it's a `core-js` promise
nativePromiseBasedAPI().catch(fn);               // -> it's a `core-js` promise
nativePromiseBasedAPI().finally(fn);             // -> it's a `core-js` promise

and that fixes most possible related problems.

But we can't patch all possible native promise-based APIs to make them return or accept (the case of this issue) core-js promises - however, most APIs should be generic and accept all thenables. Feel free to use the workarounds like you proposed.

@zloirock
Copy link
Owner

zloirock commented May 5, 2021

BTW your problem case most likely will work in the actual core-js version since native fetch should return patched native, not core-js, promise and WebAssembly.instantiateStreaming could accept it.

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

No branches or pull requests

4 participants