chore(types): auto-generate types for metro-file-map#1611
chore(types): auto-generate types for metro-file-map#1611nickhudkins wants to merge 5 commits intofacebook:mainfrom
Conversation
meta-cla
bot
added
the
CLA Signed
facebook-github-bot
added
the
Shared with Meta
nickhudkins
force-pushed
the
types/metro-file-map
branch
2 times, most recently
from
8ee7e05 to
2d6edcb
Compare
|
There are a few failures here, some are easily fixable, others will take some finagling. 1.) Option 1: Use
Option 2: Some Naive Tooling!
Note I have a local script that performs these pre-processing steps to be able to successfully generate TS at least for Option 3: Make flow-api-translator "Just Work" |
nickhudkins
left a comment
nickhudkins
left a comment
There was a problem hiding this comment.
Mostly dropping comments for myself so I don't lose track, but also for anyone reading this, perhaps my commentary is helpful.
nickhudkins
force-pushed
the
types/metro-file-map
branch
3 times, most recently
from
1243041 to
5060eb0
Compare
nickhudkins
force-pushed
the
types/metro-file-map
branch
from
5060eb0 to
252837b
Compare
nickhudkins
force-pushed
the
types/metro-file-map
branch
from
252837b to
ae7a830
Compare
|
|
||
| beforeEach(() => { | ||
| jest.resetModules(); | ||
| // Re-require the worker module to reset its internal state. |
There was a problem hiding this comment.
what? get rid of you
| }>; | ||
|
|
||
| type V8Serializable = interface {}; | ||
| type V8Serializable = unknown; |
There was a problem hiding this comment.
Need @robhogan input on how we want to treat interface {}, Since the type itself is not exported, and expects a generic, unknown does seem "fine", though I assume there should be some constraint on this type.
|
|
||
| export type WorkerSetupArgs = $ReadOnly<{}>; | ||
| export interface WorkerSetupArgs { | ||
| __future__?: false; |
There was a problem hiding this comment.
this is another one of those "doesn't translate to TS well" (empty object types), at the moment tossed in a field, but realistically there is probably a better type all together?
|
|
||
| const NOT_A_DOT = '(?<!\\.\\s*)'; | ||
| const CAPTURE_STRING_LITERAL = (pos /*: number */) => | ||
| const CAPTURE_STRING_LITERAL = (pos: number) => |
There was a problem hiding this comment.
I believe this is a file that needs to be CommonJS, though interestingly making this change did not cause test failures beyond the shape of the exports changing from using export default instead of module.exports
| ]); | ||
|
|
||
| module.exports = extensions; | ||
| export default extensions; |
There was a problem hiding this comment.
similar to other files that were CommonJS, these might end up with errors that aren't reproduced within this repo.
| '**/node_modules/**', | ||
| 'packages/metro-babel-register/**', | ||
| 'packages/*/build/**', | ||
| 'packages/metro-file-map/src/worker.js', |
There was a problem hiding this comment.
Needs to remain CommonJS?
Node version + vm.runScript usage in tests makes this tricky. We can omit it from generation for now, use the escape hatch from my other PR and hand define some types until we can actually convert it to ESM
3 participants
Summary
As I began working to hand-fix some types, the formidable @robhogan pointed me in the direction of
generateTypeScriptDefinitions.js, which has been used here to generate types formetro-file-mapChangelog:
[Fix]: Auto generate types for
metro-file-mapTest plan
Automatically tested in CI, also peformed a good-ol eyeball check.