joarwilk / flowgen

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

Migrate to TypeScript #115

Closed zxbodya closed 4 years ago

zxbodya commented 4 years ago

This might look a bit weird, considering this is one of the most important tools in the Flow community…

But, I believe, considering, a large part of flowgen is actually based on TypeScript - this makes a lot of sense.

Some background: did this mostly to make it easier to explore flowgen codebase for me, but I think this might be helpful to move for project in general.

Changes in PR:

orta commented 4 years ago

I don't mind this personally, I feel like we'd need buy-in from @goodmind (who is probably the most active contributor and user for flowgen) for this to get merged though 👍

joarwilk commented 4 years ago

This PR made me chuckle. Looks good, but I dont contribute anymore so I agree with Orta, lets wait for more input.

orta commented 4 years ago

Alright, I'll give it one more week and I'll merge 👍

meandmax commented 4 years ago

@goodmind any thoughts?

@zxbodya I would like to add support to translate utility types like Pick and Omit to its target object type, since they are not available in flow. I would like to base this on your fork since it is much easier to reason about this in TS ... :) any thoughts or tips?

zxbodya commented 4 years ago

I would like to add support to translate utility types like Pick and Omit to its target object type, since they are not available in flow. I would like to base this on your fork since it is much easier to reason about this in TS ... :) any thoughts or tips?

have looked into exactly the same issue recently :)

in general case - it does not look possible to implement this utilities in Flow(at least I have not figured yet how to do it) - but it is possible to make something which, I would consider "good enough" - check helerTypes.ts here: https://github.com/zxbodya/flowts/commit/cafce1912b1526e90e8306e9be06c0b71ec7ae32#diff-754fea4ffe52a2096bdf2d7b852ad6de

also, there are special cases where it can be handled in better way, instead of having helper utility - see here: https://github.com/zxbodya/flowts/commit/ee982fb56fabd8e1dd842333d3fc1c3f339f656e

I did it outside of flowgen - in the experimental utility, which is based on babel ast transformation (just like flowts)… it is not yet finished: for practical use, currently - flowgen is better.

as an option, if you are looking for quick way to fix utility types in generated code - it should be easy to extract utility types related logic, from repo I linked, into separate babel codemod and just run it after flowgen on generated code

meandmax commented 4 years ago

Thank you so much @zxbodya that is really helpful indeed. I was wondering if it is possible to completely replace these utility types with the result of them like:

type A = {
  a: number,
  b: string,
  c: string 
};

type Picked = Pick<A, 'a', 'b'>;

to:

type Picked = {
  a: number,
  b: string,
};

What do you think about this idea? Btw. thank you for translating flowgen in TS 👍

zxbodya commented 4 years ago

Theoretically yes - I think, it should be possible to generate the result of applying Pick/Omit on the type.

But, I would suggest the approach should be different from what I did, and probably more complicated. Just babel ast - might not be enough to make it properly… Maybe using TypeScript compiler directly can help here (flowgen is probably better place to try doing it).

The only issue, I am thinking about - it might result in a lot of generated code, in some cases, which might be harder to read when using the definitions…

But generally, I think, this is a good idea - would be really interesting to see how it will work comparing to generic util types.

orta commented 4 years ago

OK, looks like goodmind has been pretty inactive all year - so let's not be blocked there.

@zxbodya - if you get this mergeable, I'll merge it 👍🏻

zxbodya commented 4 years ago

merged the changes from master, updated dependencies except typescript - which looks to require additional changes(so better to have it separate from this)

orta commented 4 years ago

Cool, works for me 👍🏻