Add kind to TSModuleDeclaration#16732
Conversation
liuxingbaoyu
commented
Aug 10, 2024
| Q | A |
|---|---|
| Fixed Issues? | #16679 |
| Patch: Bug Fix? | |
| Major: Breaking Change? | |
| Minor: New Feature? | |
| Tests Added + Pass? | Yes |
| Documentation PR Link | |
| Any Dependency Changes? | |
| License | MIT |
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/58211 |
48eccee to
5ba2c05
Compare
nicolo-ribaudo
left a comment
There was a problem hiding this comment.
I would actually be curious here if ts-eslint is interested in not having this kind:
kind: "global"is just a repetition overglobal: truemodule X {}is being deprecated and people should instead usenamespace X {}, so this is similar to how forimport {} assert { ... }we still generate the same AST as forimport {} with { ... }
|
|
||
| if (!node.global) { | ||
| this.word(id.type === "Identifier" ? "namespace" : "module"); | ||
| this.word(kind ?? (id.type === "Identifier" ? "namespace" : "module")); |
There was a problem hiding this comment.
Even if we add the kind to the AST I would prefer to not do this change, since we currently always emit "good" code and there is no need to emit module (similarly to how we, for example, always emit semicolons even if they are absent in the input code).
There was a problem hiding this comment.
I prefer the new behavior, Babel prints as is and it's fine. The deprecation is handed over to TS and ts-eslint, which will be convenient for people to use Babel to make code modifications.
In addition, the current behavior is also strange, it only outputs some module as is,
They deprecated
Tools like Prettier need to distinguish this |
|
Back-linking for reference: typescript-eslint/typescript-eslint#6443 |
5ba2c05 to
126ec83
Compare
|
@liuxingbaoyu I wouldn't mark this as "spec compliance", as there is no spec |
I wonder whether we should drop |
Yes, I plan to open a PR to remove it in Babel 8! |
|
Could you rebase? |
126ec83 to
b4d7b6f
Compare