joarwilk / flowgen

Generate flowtype definition files from TypeScript
Other
658 stars 78 forks source link

Handle `import(…)` types for non-builtin modules, too #177

Closed gnprice closed 2 years ago

gnprice commented 2 years ago

We've been handling TypeScript's import(…) feature by rewriting it to Flow's $Exports utility type.

However, it turns out that $Exports only works when the module it refers to was defined in a libdef, as a builtin. It doesn't work if the module was defined in a .js.flow file, or a normal .js file. That's OK for import('react') or import('http'); but when a TypeScript library uses import(…) to refer to its own modules, this means we generate broken output when trying to turn its .d.ts files into .js.flow files.

Happily, we can completely resolve the issue by rewriting import(…) into a reference to a normal import * as … from … declaration.


That limitation of $Exports sadly isn't in its Flow documentation: https://flow.org/en/docs/types/utilities/#toc-exports

But it's explained in this commit message which introduced the limitation, in Flow v0.123.0: https://github.com/facebook/flow/commit/3182f731a0 and briefly mentioned in the changelog: https://github.com/facebook/flow/blob/main/Changelog.md#01230

(From that commit message, it sounds like $Exports already didn't completely work in this case, but I'm not sure exactly how.)

gnprice commented 2 years ago

Thanks for the review!

I also realized that I'd used the wrong email address in the Git author metadata; just pushed a revision that fixes that and changes nothing else.

orta commented 2 years ago

👍🏻

orta commented 2 years ago

Shipped (with the others) in 1.19.0

gnprice commented 2 years ago

Thanks!