Skip to content

Conversation

@anmonteiro
Copy link
Member

@anmonteiro anmonteiro commented Jan 29, 2024

fixes #1042

@EduardoRFS
Copy link
Collaborator

Ideally we could generate something like this and the MelangeError class would build the object. This would both make the code more readable and decrease bundle size.

throw new Caml_js_exceptions.MelangeError("Assert_failure", [
  "jscomp/test/fun_pattern_match.ml",
  41,
  9
]);

@anmonteiro
Copy link
Member Author

We can make that change, but I'm not too concerned about bundle size, to be honest.

I suspect results after gzipping will be negligible.

@EduardoRFS
Copy link
Collaborator

EduardoRFS commented Jan 30, 2024

@anmonteiro my main issue with bundling time is parsing time, the network overhead is likely negligible but starting time is always nice to be improved and code reuse also makes engines heat up.

@anmonteiro
Copy link
Member Author

anmonteiro commented Jan 30, 2024

the only thing you're saving is the extra MEL_EXN_ID string, because the exception payload arguments would still need to be passed in, e.g. exception Foo of (int * string * string) would still generate a payload of:

{
  _1: 42,
  _2: 'string1',
  _3: 'string2'
}

We can avoid generating an extra { cause: .. } wrapping though.

Base automatically changed from anmonteiro/raise-js-error to main January 30, 2024 21:12
@anmonteiro anmonteiro force-pushed the anmonteiro/use-melangeerror branch from 3844191 to 8ba7775 Compare January 30, 2024 21:25
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

Successfully merging this pull request may close these issues.

Throw MelangeError, a custom Error instance

3 participants