-
-
Notifications
You must be signed in to change notification settings - Fork 408
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
[Merged by Bors] - Generic JsResult<R>
in context.throw_
methods
#1734
Conversation
Test262 conformance changesNon-VM implementation
VM implementation
|
Codecov Report
@@ Coverage Diff @@
## main #1734 +/- ##
==========================================
+ Coverage 53.33% 53.38% +0.04%
==========================================
Files 200 200
Lines 16983 16967 -16
==========================================
- Hits 9058 9057 -1
+ Misses 7925 7910 -15
Continue to review full report at Codecov.
|
672c7f2
to
5fcf232
Compare
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.
Thanks! For a second there I thought we already had this, but indeed, this extra generic parameter makes it much more useful, since it can basically be used everywhere.
I'm approving the PR, even if there seems to be something strange with the test suite. Could you check that? Maybe the sub-module needs updating.
Yes, it was out of date, I updated it and committed it :) |
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.
bors r+
Previously when we had the `context.throw_` methods (like `context.thtrow_type_error()`) they were limited as to where we could call them, e.i. a function that returned `JsResult<JsValue>`. So we had to call the `context.construct_` methods with an explicit `Err()` enum wrap to throw in functions that returned non-jsvalues (which happens a lot). Now, with this PR the throw methods have a generic `JsResult<R>` return that can return in any `JsResult<T>` returning function. Which cleans the API and makes the user experience a bit better. ```rust return Err(context.construct_type_error("...")); // to return context.throw_type_error("..."); ```
Pull request successfully merged into main. Build succeeded: |
JsResult<R>
in context.throw_
methodsJsResult<R>
in context.throw_
methods
Previously when we had the
context.throw_
methods (likecontext.thtrow_type_error()
) they were limited as to where we could call them, e.i. a function that returnedJsResult<JsValue>
. So we had to call thecontext.construct_
methods with an explicitErr()
enum wrap to throw in functions that returned non-jsvalues (which happens a lot).Now, with this PR the throw methods have a generic
JsResult<R>
return that can return in anyJsResult<T>
returning function. Which cleans the API and makes the user experience a bit better.