Refactor generated builder names in @babel/types #11582
Conversation
| } | ||
| }); | ||
|
|
||
| Object.keys(definitions.DEPRECATED_KEYS).forEach(type => { | ||
| const newType = definitions.DEPRECATED_KEYS[type]; | ||
| output += `export function ${type}(...args: Array<any>): Object { | ||
| console.trace("The node type ${type} has been renamed to ${newType}"); | ||
| return ${type}("${type}", ...args); | ||
| return builder("${type}", ...args); |
There was a problem hiding this comment.
here was a bug, which would result in code like this:
export function RestProperty(...args: Array<any>): Object {
console.trace("The node type RestProperty has been renamed to RestElement");
return RestProperty("RestProperty", ...args);
}
which will cause infinite recursion when using such deprecated builder…
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/25400/ |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 7e81925:
|
zxbodya
changed the title
zxbodya
force-pushed
the
lowercase-builder-names
branch
from
37251aa
to
2f1cefc
Compare
zxbodya
force-pushed
the
lowercase-builder-names
branch
from
2f1cefc
to
72c9d17
Compare
| export { Super as super }; | ||
| export function TaggedTemplateExpression(...args: Array<any>): Object { | ||
| export { _super as Super }; | ||
| export { _super as super }; |
| // This is needed for backwards compatibility. | ||
| // It should be removed in the next major version. |
There was a problem hiding this comment.
Nit: could you remove this comment for now? We haven't discussed about it as a team yet.
Since uppercase builders have been around for so long, maybe in Babel 8 we will "soft-remove" them (i.e. not document them, and not represent them in .d.ts files) and add a deprecation warning.
There was a problem hiding this comment.
good point, updated the PR
nicolo-ribaudo
added
pkg: types
PR: Bug Fix 🐛
github-actions
bot
added
the
outdated
This is related to migration to typescript #11578
The issue is that if exported builder name starts from upper case character - it conflicts with a type name.
This PRs refactors generation of builders, so by default, they are named starting with a lowercase character, and for backward compatibility re-exported starting with upper-case (currently it is opposite). This will make it easier to separate backward compatibility exports from the default, during migration to typescript. And would make it easier to be removed in the major version.
Also, I updated all the places where the builder's name starting with the upper-case was used(this is to be needed after migrating to TypeScript).