-
Notifications
You must be signed in to change notification settings - Fork 695
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
Require an ok status for a response #1060
Conversation
Once there's a more formal specification we should deduplicate this. |
This changes both WebAssembly.compile and WebAssembly.instantiate to be both more clear about response requirements and require an ok status. Fixes #1039.
fe7a092
to
1ea453f
Compare
@@ -60,15 +60,18 @@ as described in the [`WebAssembly.Module` constructor](#webassemblymodule-constr | |||
On success, the `Promise` is [fulfilled](http://tc39.github.io/ecma262/#sec-fulfillpromise) | |||
with the resulting `WebAssembly.Module` object. On failure, the `Promise` is | |||
[rejected](http://tc39.github.io/ecma262/#sec-rejectpromise) with a | |||
`WebAssembly.CompileError`. | |||
`WebAssembly.CompileError` or `TypeError`, depending on the type of failure. |
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.
Nit: should we say "depending on the type of failure, as it will be clarified further below"
... or something to that extent?
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 went with what we had for instantiate, but happy to change both in this manner if others agree. Eventually we need to redo both these descriptions in a more formal matter once you all figure out how you want to define your objects (see #1048).
Anyone else have any feedback? |
This changes both WebAssembly.compile and WebAssembly.instantiate to be
both more clear about response requirements and require an ok status.
Fixes #1039.