Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Experimental] Typescriptify #455

Open
wants to merge 5 commits into
base: main
Choose a base branch
from
Open

Conversation

taneba
Copy link

@taneba taneba commented Jun 2, 2022

related: #448

This is an experimental PR for TypeScript migration.
I used flow-to-typescript-codemod by stripe. Though it says "This project is provided as-is and is not actively maintained" in its readme, it's quite fresh atm so I think it makes sense to migrate to typescript before it gets outdated.

As I'm not very familiar with final-form's whole codebase, probably there might be a few mistakes and it's not completely done (especially build part). Additionally, we need to make some decision on things (e.g. drop flow type support or not). But I hope this can be a good starting point for discussion.

@erikras

What I've done

  • ran codemod with yarn typescriptify convert --path ../final-form/**/*.js --write --delete
  • manually changed types.js.flow to types.ts
  • changed couple of files that doesn't need to be .ts to .js
    • rollup.config.js
    • package.script.js
  • fixed type errors manually.

@rosskevin
Copy link
Contributor

That's an interesting starter, assuming @erikras is interested in moving to typescript (which I would be in favor). But, did you compare the generated output to the types that are included and working e.g. https://github.com/final-form/final-form/blob/main/src/index.d.ts?

Is this a goal or not @erikras? #448 seems to be unanswered. Even though it is short on rationale, perhaps that's not necessary at this point (sorry, I've been on hiatus for about two years).

@taneba
Copy link
Author

taneba commented Jun 15, 2022

@rosskevin
Thank you for giving your thoughts!
I have not compare index.d.ts yet and I even didn't generate it. One issue here is that the original type declaration file is not generated but it's hand-written, so we need to decide how to provide declaration file.

Sorry for opening PR before enough discussion, but I think there's no point to keep codebase flow-typed in terms of ease of contribution. And since flow-to-typescript-codemode is really thorough one, it'd be nice to migrate while it is fresh (note that it's already archived and not supposed to actively be maintained in the future, meaning that it's probably gonna be broken after next flow/typescript update).

What's your thoughts? @erikras

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants