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

Adding napi_create_syntax_error #1099

Closed
idan-at opened this issue Nov 4, 2021 · 3 comments
Closed

Adding napi_create_syntax_error #1099

idan-at opened this issue Nov 4, 2021 · 3 comments

Comments

@idan-at
Copy link

idan-at commented Nov 4, 2021

Hi,

Some of the known js tooling (webpack and babel for example) are re-written in other languages (esbuild in go, swc and parcel in rust)

Because of that, throwing a syntax error from napi makes sense.
unfortunately, the API does not exist at this moment (although other native errors can be thrown, like type error and range error)

what do you think about adding it?

If it’s accepted, I’d be happy to contribute (if you can point me)

Thanks!

@NickNaso
Copy link
Member

NickNaso commented Nov 4, 2021

Hi @idan-at,
Syntax erros while detecting or parsing source code. See: MDN Syntax Error and ECMAScript.
What I unsterstood is that adding this type of error could be helpful to build tools like esbuild, babel or swc using implemented like a native addon and return a more exaustive error in case of incorrect syntax.
This is a changes that need to be made on the Node.js core. In the specific we need to add two new functions:

napi_status napi_throw_syntax_error(napi_env env, const char* code, const char* msg);
napi_status napi_create_syntax_error(napi_env env, napi_value code, napi_value msg, napi_value* result);

We can disccus this proposal in Node-API Team tomorrow. If you want to partecipate you can find all the info here: https://github.com/nodejs/abi-stable-node#meeting

@idan-at
Copy link
Author

idan-at commented Nov 5, 2021

@NickNaso thank you for the quick reply.
Unfortunately I won't be able to attend today. I'd appreciate if you can bring it up and share your decision here.

If you find the API useful I will be happy to contribute.

Thanks

@mhdawson
Copy link
Member

mhdawson commented Nov 5, 2021

We discussed in the Node-API team meeting today, and agreed it would make sense to add methods to throw/create the errors listed in the spec including syntax_error.

If you'd like to submit a PR to core to add these and add equivalents to node-addon-api once the core PR lands that would be great.

idan-at pushed a commit to idan-at/node that referenced this issue Nov 6, 2021
Add `napi_create_syntax_error` and `napi_throw_syntax_error`.

Fixes: nodejs/node-addon-api#1099
targos pushed a commit to nodejs/node that referenced this issue Nov 21, 2021
Add `napi_create_syntax_error` and `napi_throw_syntax_error`.

Fixes: nodejs/node-addon-api#1099

PR-URL: #40736
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
danielleadams pushed a commit to nodejs/node that referenced this issue Jan 30, 2022
Add `napi_create_syntax_error` and `napi_throw_syntax_error`.

Fixes: nodejs/node-addon-api#1099

PR-URL: #40736
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
danielleadams pushed a commit to nodejs/node that referenced this issue Feb 1, 2022
Add `napi_create_syntax_error` and `napi_throw_syntax_error`.

Fixes: nodejs/node-addon-api#1099

PR-URL: #40736
Reviewed-By: James M Snell <jasnell@gmail.com>
Reviewed-By: Michael Dawson <midawson@redhat.com>
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 a pull request may close this issue.

3 participants