-
Notifications
You must be signed in to change notification settings - Fork 453
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
Throw Js Erros constructor EvalError
, RangeError
, etc
#6931
base: master
Are you sure you want to change the base?
Conversation
This change will break my libs and applications |
Your libs use |
EvalError
, RangeError
, etc
Sorry, I've taken a look at your PR, and it indeed improves things compared to the current v12 alpha release. But it doesn't solve the root of the problem, and for example, the Js.Exn.raiseError still have a regressed behaviour compared to v11. I'm working on the PR #6933, which should automatically fix the issue you're trying to solve in the PR. This is why I think it's not needed. |
What is the regression compared to v11? |
Previously it would throw a The main usecase for the |
And if we are talking about the issue #6929 |
After #6611 was merged it is no longer possible to throw errors like
EvalError
. The throw statement now isthow new Error(exn)
.Example: The
Js.Exn.raiseEvalError
function:https://github.com/rescript-lang/rescript-compiler/blob/0b1443f967808a41c3ad309002eca964f0b6465f/jscomp/others/js_exn.res#L48
compile to:
https://github.com/rescript-lang/rescript-compiler/blob/0b1443f967808a41c3ad309002eca964f0b6465f/lib/es6/js_exn.js#L10-L14
This PR add a function
throw
:Side question
Here a question arises whether we should support
EvalError
and others, or encourage ReScript exceptions. Recently documented the exceptions builtin (rescript-lang/rescript-lang.org#880)My idea is to use ReScript exceptions and not bindings to Js
Error
(i.e@new external make: string => t = "Error"
). We will still catch Js ErrorsRelated #6929