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

WebAssembly.{compile|instantiate}Streaming: what if embedder doesn't implement it? #1085

Open
mtrofin opened this issue May 31, 2017 · 7 comments

Comments

@mtrofin
Copy link
Contributor

mtrofin commented May 31, 2017

In Chrome, we're implementing the 2 streaming APIs in v8, on the WebAssembly object (as per spec), and inject the actual implementation from Chrome. This is for layering reasons. I'm assuming we're not alone in taking this approach.

What do we want to do if the embedder "forgets" to inject a dependency? This scenario is unlikely, but may happen in non-web embeddings. A developer porting code to node.js, for example, may hit this situation.

Options:

  1. quietly return undefined (weird, no indication of what just happened)
  2. throw RangeError (we did that for browser limits for sync compilation, but it is weird because why "Range" in this case?)
  3. throw Error with a nice description
  4. behave as-if the API didn't exist (that'd be weird, because the API is testably present)
  5. crash (this is also weird, why such drastic measures when calling an API)

Personal preference: option 3.

Thoughts?

@mtrofin mtrofin changed the title WebAssembly.{compile|instantiate}Streaming: what if embedder doesn WebAssembly.{compile|instantiate}Streaming: what if embedder doesn't implement it? May 31, 2017
@domenic
Copy link
Member

domenic commented May 31, 2017

It seems like it'd be much better if the API didn't exist (and wasn't testably present). Maybe nobody wants to implement that though...

@mtrofin
Copy link
Contributor Author

mtrofin commented May 31, 2017

Actually... come to think about it, not terribly hard to implement, at least on the chrome/v8 side.

@mtrofin
Copy link
Contributor Author

mtrofin commented May 31, 2017

Assuming this is how we all go about it, would this need additional specification?

@domenic
Copy link
Member

domenic commented May 31, 2017

In my opinion the current spec supports my suggestion, but it could be made clearer by adding a sentence explicitly saying something like "these methods must not exist if they are not supported" or similar.

@rossberg
Copy link
Member

rossberg commented Jun 1, 2017

Not sure how this is fundamentally different from other properties that are added by other APIs. For example, the DOM adds plenty of properties to the global object. Ecma-262 explicitly allows adding properties. So I agree that (1) is the natural option (and what is currently implied).

Also, not returning undefined would break the standard feature detection patterns.

@domenic, where do you suggest to add this sentence? If it's in the Web API spec, then it won't have any binding consequences, since it wouldn't apply to an embedder that doesn't implement that spec. If it's in the basic JS API spec then it would basically remove the layering. (FWIW, the same question would apply to any of the other suggestions in the OP.)

@mtrofin
Copy link
Contributor Author

mtrofin commented Jun 1, 2017

@rossberg-chromium for clarity, option 1 was:

"instantiateStreaming" in WebAssembly === true
WebAssembly.instantiateStreaming(some_response) === undefined

I think what we're agreeing on, which wasn't on the original list, and was subsequently proposed by @domenic, is:

"instantiateStreaming" in WebAssembly === false

and, then, WebAssembly.instantiateStreaming throws ("instantiateStreaming not a function")

We're still on the same page, though, correct? (i.e. we want this latter option, not option "1")

@rossberg
Copy link
Member

rossberg commented Jun 1, 2017

@mtrofin, ah, okay, thanks for the clarification. And yes, I agree with the resolution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants