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.

Sign up for GitHub

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

Jump to bottom

module: add ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX #56610

Merged
merged 4 commits into from
Jan 17, 2025
Merged

module: add ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX #56610

merged 4 commits into from
Jan 17, 2025
+93 −38

Conversation

marco-ippolito
Copy link
Member

This new error allows user to know if its unsupported syntax like enums in strip-types or just general invalid syntax.

@nodejs-github-bot
Copy link
Collaborator

Review requested:

  • @nodejs/loaders
  • @nodejs/typescript

@nodejs-github-bot nodejs-github-bot added errors Issues and PRs related to JavaScript errors originated in Node.js core. module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. labels Jan 15, 2025
@codecov Codecov

This comment was marked as outdated.

Sign in to view
@marco-ippolito
Copy link
Member Author

idk why the coverage complains, the behavioral change is tested 😫

@marco-ippolito marco-ippolito added author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue Add this label to land a pull request using GitHub Actions. and removed commit-queue Add this label to land a pull request using GitHub Actions. labels Jan 15, 2025
@marco-ippolito
Copy link
Member Author

marco-ippolito commented Jan 15, 2025 •
edited
Loading

Removed some dead code. Since we splitted compilation from execution, there is no need to check if what is thrown is not an error or if error properties have been overwritten.

@marco-ippolito marco-ippolito force-pushed the update-error-to-syntax-error branch from f300ebb to f67762a Compare January 15, 2025 14:35
@marco-ippolito marco-ippolito added the request-ci Add this label to start a Jenkins CI on a PR. label Jan 15, 2025
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Jan 15, 2025
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/64523/

@marco-ippolito marco-ippolito added commit-queue Add this label to land a pull request using GitHub Actions. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. labels Jan 15, 2025
@aduh95 aduh95 added semver-minor PRs that contain new features and should be released in the next minor version. request-ci Add this label to start a Jenkins CI on a PR. and removed request-ci Add this label to start a Jenkins CI on a PR. labels Jan 15, 2025
aduh95
aduh95 reviewed Jan 15, 2025
View reviewed changes
doc/api/errors.md Outdated Show resolved Hide resolved
@marco-ippolito @aduh95
Update doc/api/errors.md
b15ee51 
Co-authored-by: Antoine du Hamel <[email protected]>
@marco-ippolito marco-ippolito force-pushed the update-error-to-syntax-error branch from a3e87d4 to b15ee51 Compare January 15, 2025 19:03
@nodejs-github-bot
Copy link
Collaborator

CI: https://ci.nodejs.org/job/node-test-pull-request/64526/

robpalme
robpalme approved these changes Jan 16, 2025
View reviewed changes
doc/api/errors.md
Comment on lines +3148 to +3150
The provided TypeScript syntax is unsupported.
This could happen when using TypeScript syntax that requires
transformation with [type-stripping][].
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should we be more explicit and immediately suggest to the user that a potential solution is to run using --experimental-transform-types?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems like a more forward-looking suggestion would be to suggest they use the TS 5.8 option (once it's out) so they avoid using that syntax in the first place (or perhaps both suggestions)

Copy link
Member Author

@marco-ippolito marco-ippolito Jan 17, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could add it in the error message itself?
I will followup with a PR after swc updates since I have to refactor error messages anyways

@legendecas legendecas mentioned this pull request Jan 17, 2025
Closed
@nodejs-github-bot nodejs-github-bot removed the commit-queue Add this label to land a pull request using GitHub Actions. label Jan 17, 2025
@nodejs-github-bot nodejs-github-bot merged commit cee63dc into nodejs:main Jan 17, 2025
53 checks passed
@nodejs-github-bot
Copy link
Collaborator

Landed in cee63dc

aduh95 pushed a commit that referenced this pull request Jan 27, 2025
@marco-ippolito @aduh95
module: add ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX
e669297 
PR-URL: #56610
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Ethan Arrowood <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
@github-actions github-actions bot mentioned this pull request Jan 28, 2025
Merged
nodejs-github-bot added a commit that referenced this pull request Jan 28, 2025
@nodejs-github-bot
2025-01-29, Version 23.7.0 (Current)
e296b09 
Notable changes:

crypto:
  * update root certificates to NSS 3.107 (Node.js GitHub Bot) #56566
fs:
  * (SEMVER-MINOR) allow `exclude` option in globs to accept glob patterns (Daeyeon Jeong) #56489
module:
  * (SEMVER-MINOR) add ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX (Marco Ippolito) #56610
sqlite:
  * (SEMVER-MINOR) support TypedArray and DataView in `StatementSync` (Alex Yang) #56385
src:
  * (SEMVER-MINOR) add --disable-sigusr1 to prevent signal i/o thread (Rafael Gonzaga) #56441
src,worker:
  * (SEMVER-MINOR) add isInternalWorker (Carlos Espa) #56469
test_runner:
  * (SEMVER-MINOR) add TestContext.prototype.waitFor() (Colin Ihrig) #56595
  * (SEMVER-MINOR) add t.assert.fileSnapshot() (Colin Ihrig) #56459
  * (SEMVER-MINOR) add assert.register() API (Colin Ihrig) #56434

PR-URL: #56800
aduh95 pushed a commit that referenced this pull request Jan 30, 2025
@marco-ippolito @aduh95
module: add ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX
9c5c3b3 
PR-URL: #56610
Reviewed-By: James M Snell <[email protected]>
Reviewed-By: Ruben Bridgewater <[email protected]>
Reviewed-By: Geoffrey Booth <[email protected]>
Reviewed-By: Ethan Arrowood <[email protected]>
Reviewed-By: Chengzhong Wu <[email protected]>
aduh95 added a commit that referenced this pull request Jan 30, 2025
@aduh95
2025-01-30, Version 23.7.0 (Current)
73b4c04 
Notable changes:

crypto:
  * update root certificates to NSS 3.107 (Node.js GitHub Bot) #56566
fs:
  * (SEMVER-MINOR) allow `exclude` option in globs to accept glob patterns (Daeyeon Jeong) #56489
module:
  * (SEMVER-MINOR) add ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX (Marco Ippolito) #56610
sqlite:
  * (SEMVER-MINOR) support TypedArray and DataView in `StatementSync` (Alex Yang) #56385
src:
  * (SEMVER-MINOR) add --disable-sigusr1 to prevent signal i/o thread (Rafael Gonzaga) #56441
src,worker:
  * (SEMVER-MINOR) add isInternalWorker (Carlos Espa) #56469
test_runner:
  * (SEMVER-MINOR) add TestContext.prototype.waitFor() (Colin Ihrig) #56595
  * (SEMVER-MINOR) add t.assert.fileSnapshot() (Colin Ihrig) #56459
  * (SEMVER-MINOR) add assert.register() API (Colin Ihrig) #56434

PR-URL: #56800
aduh95 added a commit that referenced this pull request Jan 30, 2025
@aduh95
2025-01-30, Version 23.7.0 (Current)
c2c7f08 
Notable changes:

crypto:
  * update root certificates to NSS 3.107 (Node.js GitHub Bot) #56566
fs:
  * (SEMVER-MINOR) allow `exclude` option in globs to accept glob patterns (Daeyeon Jeong) #56489
module:
  * (SEMVER-MINOR) add ERR_UNSUPPORTED_TYPESCRIPT_SYNTAX (Marco Ippolito) #56610
sqlite:
  * (SEMVER-MINOR) support TypedArray and DataView in `StatementSync` (Alex Yang) #56385
src:
  * (SEMVER-MINOR) add --disable-sigusr1 to prevent signal i/o thread (Rafael Gonzaga) #56441
src,worker:
  * (SEMVER-MINOR) add isInternalWorker (Carlos Espa) #56469
test_runner:
  * (SEMVER-MINOR) add TestContext.prototype.waitFor() (Colin Ihrig) #56595
  * (SEMVER-MINOR) add t.assert.fileSnapshot() (Colin Ihrig) #56459
  * (SEMVER-MINOR) add assert.register() API (Colin Ihrig) #56434

PR-URL: #56800
This was referenced Jan 30, 2025
Closed
Closed
Closed
Closed
Closed
Closed
Closed
Merged
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@aduh95 aduh95 aduh95 left review comments

@ljharb ljharb ljharb left review comments

@Ethan-Arrowood Ethan-Arrowood Ethan-Arrowood approved these changes

@BridgeAR BridgeAR BridgeAR approved these changes

@legendecas legendecas legendecas approved these changes

@robpalme robpalme robpalme approved these changes

@GeoffreyBooth GeoffreyBooth GeoffreyBooth approved these changes

@jasnell jasnell jasnell approved these changes

Assignees
No one assigned
Labels
author ready PRs that have at least one approval, no pending requests for changes, and a CI started. commit-queue-squash Add this label to instruct the Commit Queue to squash all the PR commits into the first one. errors Issues and PRs related to JavaScript errors originated in Node.js core. module Issues and PRs related to the module subsystem. needs-ci PRs that need a full CI run. process Issues and PRs related to the process subsystem. semver-minor PRs that contain new features and should be released in the next minor version.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
10 participants
@marco-ippolito @nodejs-github-bot @ljharb @jasnell @GeoffreyBooth @robpalme @legendecas @BridgeAR @aduh95 @Ethan-Arrowood