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

Migrating to Typescript #1665

Open
szyn opened this issue Oct 21, 2021 · 7 comments
Open

Migrating to Typescript #1665

szyn opened this issue Oct 21, 2021 · 7 comments

Comments

@szyn
Copy link
Member

szyn commented Oct 21, 2021

See also #1641 (comment) for details.

@seiyab
Copy link
Contributor

seiyab commented Dec 7, 2021

Can I work on it?

@szyn
Copy link
Member Author

szyn commented Dec 15, 2021

Sure, no problem at all. I appreciate your help as usual 👍

@seiyab
Copy link
Contributor

seiyab commented Dec 18, 2021

@szyn
Which plan is better do you think?

  1. Gradually migrate
    • steps
      1. Make all the codes valid typescript syntax without resolving type errors and lint errors
      2. Gradually collect type errors and lint errors by many small PRs.
      3. When all the type errors and lint errors are resolved, add type check step and lint step into CI.
    • pros
      • can avoid conflicts
      • each PRs will be small and easier to review
      • other contributors can help migration
      • some progress will be commited even if I stop commit for some reason
  2. Migrate in single PR
    • steps
      1. Make syntax, type, format and coding style valid at one PR and check them on CI
    • pros
      • can avoid committing half done codes
      • you will review PR only 1 time
      • can avoid half done migration even if I stop migration for some reason

@szyn
Copy link
Member Author

szyn commented Dec 31, 2021

Thank you, I feel that 2 is simple and better for me. What do you think?

@seiyab
Copy link
Contributor

seiyab commented Jan 7, 2022

I think each of them is good.
I'll try latter one.

@seiyab
Copy link
Contributor

seiyab commented Jan 10, 2022

@szyn
I found that fixing all the typing error in reliable way is hard.
It is similar problem to newer flow that reports too many type errors.
Many type annotations are unclear.

So I will add some any, that is unclear type and some ts-ignore, that make type checker skip codes.
Do you agree it?

This approach is based on original approach 2, but it also has some flavor of original approach 1.

  • No type error will be reported by type checker after the single PR.
  • So we can run type checker on CI.
  • However, a part of type annotations (any) don't make sense, and even be ignored sometimes.
    • So, After migration, I recommend that request change for PRs that adds more any or ts-ignore to make type annotations more reilable.
    • I'm going to additional PRs that will resolve some part of anys and ts-ignores after migration.

@szyn
Copy link
Member Author

szyn commented Jan 19, 2022

So I will add some any, that is unclear type and some ts-ignore, that make type checker skip codes.
Do you agree it?

Yes, I agree with it. And also it looks good to me for the approach you suggested. Thank you for letting me know 👍

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

No branches or pull requests

2 participants