-
-
Notifications
You must be signed in to change notification settings - Fork 5.8k
Improve @babel/parser error types #17531
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
base: main
Are you sure you want to change the base?
Conversation
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/60629 |
|
commit: |
87068ba to
93dd477
Compare
|
Rebase? :) |
93dd477 to
aa1136a
Compare
|
I realized an issue, errors that were misnamed in Babel 7 are now correctly named in types. |
Gulpfile.mjs
Outdated
| SetAccessorCannotHaveRestParameter: "SetAccesorCannotHaveRestParameter", | ||
| SetAccessorCannotHaveReturnType: "SetAccesorCannotHaveReturnType", | ||
| }; | ||
| for (const k of Object.keys(oldReasonCodes)) { |
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.
Object.entries()?
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.
Unfortunately Node.js 6 does not support Object.entries.
Gulpfile.mjs
Outdated
|
|
||
| if (output.includes("babel-parser.d.ts")) { | ||
| let code = fs.readFileSync(output, "utf-8"); | ||
| const oldReasonCodes = { |
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.
Are these only being included for Babel 7?
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.
Oh, it should be!
Gulpfile.mjs
Outdated
| }); | ||
|
|
||
| if ( | ||
| !bool(process.env.BABEL_8_BREAKING) && |
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.
Sorry to ask again, but could you rebase so that we can remove these checks?
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.
Of course!
c192e8c to
c07da00
Compare
c07da00 to
27979ae
Compare
|
What do you think about manually maintaining error messages instead of using a TypeScript type system? For example, This could significantly reduce the size of the |
|
It seems like it would save us about 2kB (the space taken by all the Maybe we can explore generating it at build time. |
|
I tried compressing it into an array, but TypeScript complained that the type was too complex, so I had to compress it into an object. |
Fixes #1, Fixes #2This PR is based on #17521, only the last commit is new.