joarwilk / flowgen

Generate flowtype definition files from TypeScript
Other
657 stars 87 forks source link

[draft] Upgrade TS to 4.5.x, fix some of the broken tests #192

Open karmeleon opened 1 year ago

karmeleon commented 1 year ago

Hi! I'm working with a codebase that heavily uses the import { type A } from '...'; syntax that is currently broken (see issue 185). So I'm trying to update the version of TS used in flowgen to latest, stopping at 4.5.x first for simplicity's sake.

It's quite a bit more involved than I expected. Actually fixing the import type syntax was trivial, but it turns out that sometime between TS 4.4 and 4.5 there was a bugfix for the getSymbolAtLocation() function that causes it to return symbols in several situations where previously it would simply return null. You can see this easily by selecting AlbumLabel on line 3 of this TS AST viewer and changing the TS version used to generate it under Options: under 4.4 the Symbol is shown as None in the right-hand column, but it's correctly identified in 4.5 and newer. I haven't been able to figure out exactly under what circumstances the behavior changed. I looked through the git history of the TS repo but couldn't find any commits that mentioned that function at all, and the only evidence I could find of it online were this old, unresolved bug report and this SO question.

Several parts of flowgen seem to implicitly rely on this buggy behavior, and will emit incorrect Flow defs when TS returns non-null. I've been able to guess my way into "fixing" one or two instances of this, but I'm not convinced what I did was actually correct. I think some parts of flowgen might require nontrivial rewrites to work with the new behavior, but I don't know enough about flowgen to say for sure. Does anyone have any suggestions as to what I can try next?