Migrate Babel from Flow to TypeScript (except Babel parser)#11578
Merged
nicolo-ribaudo merged 69 commits intobabel:mainfrom Nov 25, 2021
Merged
Migrate Babel from Flow to TypeScript (except Babel parser)#11578nicolo-ribaudo merged 69 commits intobabel:mainfrom
nicolo-ribaudo merged 69 commits intobabel:mainfrom
Conversation
This was referenced May 18, 2020
Merged
Collaborator
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/49954/ |
|
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 bc07a4d:
|
8d7f7be to
af41cdc
Compare
9e2a988 to
36da578
Compare
packages/babel-register/src/index.ts
Outdated
| Object.assign(exports, node); | ||
|
|
||
| // make TypeScript happy, by making this file a "module" | ||
| export default register; |
Member
There was a problem hiding this comment.
This is an observable change, because it changes the source type (and thus the compiled output) from "script" to "module".
I'll revert this file to .js; it will go away in Babel 8 anyway.
nicolo-ribaudo
approved these changes
Nov 25, 2021
Member
nicolo-ribaudo
left a comment
There was a problem hiding this comment.
Sorry that this took so long! There are still many places where we need to improve our types, bug this is a good step.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Still work in progress, but most of the migration already done:
flowts:)Currently, the whole project passes the type check, also existing tests should be working.
Commits are grouped by package and sorted in topological order (before fixing some package - all of its dependencies should be already fixed)
I tried to avoid unnecessary changes, and so - for example, did not use strict mode anywhere(I think it would be good to add in some places, but - later).
Remaining things, I am planing before marking this as ready for review:
Some background:
I working on a tool automating the Flow to TypeScript migration, which is already in a quite working state.
It is built using Babel, and partially because of that - I want to improve TypeScript typings for Babel.
I think, the best way to so is to convert the whole project to TypeScript, which I believe also has other benefits:
Related meeting notes: https://github.com/babel/notes/blob/master/2020/05-07.md#investigate-using-typescript-in-our-code
Migration summary:
The most complicated/problematic was babel/parser - there are a lot of issues in flow types that were highlighted after the migration(basically, as often happens with flow… - some issues were ignored) in particular, there are a lot of issues in node type definitions:
weird changes (EDIT: we don't import
package.jsonanymore)There is a tricky problem about importing
../package.jsonin TypeScript.Generally TypeScript can support importing json files, but it also wants all imported files to be included as sources.
So if to include only
src-../package.jsonwould be allowed.If to include
package.jsoninto sources - when building, structure of all the sources (not onlysrcas needed) would be replicated.To solve this, I did a bit weird trick:
Where needed - I added
src/package.js, with following content:Which is not type-checked, and can be imported instead of
package.json.Suggestions for better options are welcome.
common fixes:
remove unnecessary typecasts(most common
as Array<any>)add missing optional argument annotations (most often boolean arguments which are optional, are not marked as such)
add
as constin some places to get more specific types (typically for "enum-like" constants)Object.freeze({})->Object.freeze({} as const){…}->{…} as constremove some
anys(btw, in flowFunctionandObject- are aliases toany), in many cases typescript can infer better typeremove some typecasts to
unknown(mixedin flow) or replace it withanywhere appropriatewhere needed replace
$FlowIgnorewith@ts-ignorefixing usages of not existing types (some types used in flow code not were imported, or imported from somewhere they do not exist)
replace
voidwithundefindedin some places (in tsvoidmean no value and should be used only as return type)fixing optional argument annotations (in typescript
undefnedtype, does not mean that argument is optional, and so it would require argument which however might be undefined)change type annotation for some functions to be type guards, for example:
in some cases - added type refinement, before the condition, for example:
if (t.isStringLiteral(node) && node.value === "use strict")instead of justnode.value === "use strict"(which technically will work, but would make TS not happy)add missing type parameter(most common
ArraytoArray<any>, but also the same forSet,MapandWeekMaptypes)fix incorrect
DOMtype usages:Locationwitht.SourceLocationNodewitht.Nodeadd missing fields in some types(for example
recordAndTupleSyntaxType,jsescOptionandjsonCompatibleStringsinFormatinbabel-generator)declared instance fields in class declarations (often they were assigned somewhere in class methods or constructor, but not declared)
replace some assertions like
path.type === "JSXFragment"with appropriate utility callspath.isJSXFragment()remove unused call arguments or properties on options object where in some places, where function does not have that argument or does not use the property
@ts-expect-errorwhen doing something not type-safe, like assigning.typeon AST node@ts-expect-errorwhen gettingtagExpr.valuefromLiteral(because it is not present onNullLiteral)import and use types from
babel-typesandbabel-travesein some plugins, helpers (not everywhere, I would suggest this can be one of the next steps)replace
template->template.statementwhere appropriate to have simpler type-castsuse
!!valuein some places where value should be converted tobooleanadditional type annotations in some places(tried to minimize this, also to consider as one of the next steps after landing this)
typecasts like
path.get("tag.object") as NodePathwhere correct type cannot be inferredtype declaration for visitor state in some helpers/plugins (as possible next step - to do more of this)
use
Visitortype frombabel-traversefor visitor declaration in plugins (allows improving type coverage for individual visitors without manually typing each property in declaration)in some places,
t.Nodevalidators were used onNodePath(which evetually worked, but is not correct in general), example fix:t.isImportDeclaration(stmt)->stmt.isImportDeclaration()notable changes:
.tsfilesjsfile for deprecated exports, to avoid exporting types for them)use
abstact class(instead of hack with flow type comments)refactor parser plugins to be typed
specify exact argument list in methods overridden in parser plugins, and in calls to original implementation - to have exactly the same method signature(which is now type-checked). for example:
refactoring in node type definitions
&in flow types, which often looked not correct)babel-parser move included type definitions to source code
add/fix some node types
thisfor generators assigned toPrinter.prototypethistype annotation for mixins in NodePath, export types for fields added by mixinstype SourceMap = typeof import("convert-source-map").SourceMap;withanyinpackages/babel-core/src/transformation/file/generate.tsdeclareutilityGoing to periodically rebase this on current master, while finishing work on.
However, there is not likely to be a big change here, so comments and suggestions are welcome.