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

Transition away from Flow to TypeScript #1110

Open
4 of 55 tasks
segersniels opened this issue May 31, 2023 · 12 comments
Open
4 of 55 tasks

Transition away from Flow to TypeScript #1110

segersniels opened this issue May 31, 2023 · 12 comments

Comments

@segersniels
Copy link
Collaborator

segersniels commented May 31, 2023

Discussion

Mostly just to open a discussion.

I personally feel like TypeScript has been winning the race against Flow (and has won) for the last couple of years. So might be worth the effort to convert everything to TS and then compile/bundle the code into JS with something like tsup. Should improve development speed and give more control over types.

Validations

To do

  • src/constants/flags.js
  • src/constants/commit.js
  • src/constants/configuration.js
  • src/utils/getDefaultCommitContent.js
  • src/utils/filterGitmojis.js
  • src/utils/getAbsoluteHooksPath.js
  • src/utils/emojisCache.js
  • src/utils/getEmojis.js
  • src/utils/printEmojis.js
  • src/utils/configurationVault/getConfiguration.js
  • src/utils/configurationVault/index.js
  • src/utils/findGitmojiCommand.js
  • src/utils/isHookCreated.js
  • src/utils/buildFetchOptions.js
  • src/commands/commit/prompts.js
  • src/commands/commit/withClient/index.js
  • src/commands/commit/index.js
  • src/commands/commit/withHook/index.js
  • src/commands/commit/guard.js
  • src/commands/update/index.js
  • src/commands/config/prompts.js
  • src/commands/config/index.js
  • src/commands/config/guard.js
  • src/commands/index.js
  • src/commands/search/index.js
  • src/commands/list/index.js
  • src/commands/hook/remove/index.js
  • src/commands/hook/hook.js
  • src/commands/hook/index.js
  • src/commands/hook/create/index.js
  • test/utils/emojisCache.spec.js
  • test/utils/isHookCreated.spec.js
  • test/utils/printEmojis.spec.js
  • test/utils/getEmojis.spec.js
  • test/utils/findGitmojiCommand.spec.js
  • test/utils/configurationVault/vault.spec.js
  • test/utils/configurationVault/getConfiguration.spec.js
  • test/utils/configurationVault/defaults.spec.js
  • test/utils/filterGitmojis.spec.js
  • test/utils/buildFetchOptions.spec.js
  • test/utils/getAbsoluteHooksPath.spec.js
  • test/utils/stubs.js
  • test/utils/getDefaultCommitContent.spec.js
  • test/commands/commit.spec.js
  • test/commands/update.spec.js
  • test/commands/search.spec.js
  • test/commands/hook.spec.js
  • test/commands/list.spec.js
  • test/commands/config.spec.js
  • test/commands/stubs.js
  • test/commands/commands.spec.js
  • test/setupTests.js
@carloscuesta
Copy link
Owner

carloscuesta commented May 31, 2023

Hey!

This was one of the issues I wanted to open these days, a complete go from my side 🚀
I believe we can improve the codebase a lot by switching from Flow to TS ❤️

@carloscuesta
Copy link
Owner

Is there an easy way we can make the migration incrementally? 🤔

@segersniels
Copy link
Collaborator Author

Is there an easy way we can make the migration incrementally? 🤔

We could set a rule that if you touch a file, you convert it to TS which will then gradually convert the codebase. Which is how I've done it in the past for large codebases. But might as well just do everything in one go since the codebase is not that big.

I had a go at it yesterday and shouldn't be too much work since it's purely types that change, the underlying javascript doesn't really change much.

@carloscuesta
Copy link
Owner

Is there an easy way we can make the migration incrementally? 🤔

We could set a rule that if you touch a file, you convert it to TS which will then gradually convert the codebase. Which is how I've done it in the past for large codebases. But might as well just do everything in one go since the codebase is not that big.

I had a go at it yesterday and shouldn't be too much work since it's purely types that change, the underlying javascript doesn't really change much.

Sure but we would have to support both TS and JS Flow under the same build system, It's possible given that we can allow using JS within the tsconfig file and then transpile everything with tsup

Not sure if we will have conflicts by having "Flow types" on JS files when bundling with tsup

If that's the case then I would go for doing one PR with the setup for TS introducing the new bundling tool and then organise each part of the codebase so we can work together on a command/feature basis

@carloscuesta
Copy link
Owner

@segersniels Great work!! I'll add the list of files that we need to migrate to your original message in a checklist mode!

@Lauro235
Copy link

Lauro235 commented Oct 4, 2023

This is a cool project! Is this commit still up-to-date? I'd love to contribute 👍

@carloscuesta
Copy link
Owner

Hey @Lauro235 yes, it's up to date feel free to migrate and send PRs for the non TS files we have in the project

@shravan20
Copy link

@carloscuesta: I would love to contribute to this project for migration. Kindly do let me know if I can pick this up for migration.

@carloscuesta
Copy link
Owner

Hey @shravan20 feel free to go ahead ❤️

@mikelinhas
Copy link
Contributor

Hi @segersniels! I gave this a go, but I don't think it's possible to migrate gradually, file by file.
It kind of feels like an all or nothing task.

Question. Do you know if there is an easy way to do it piece by piece?

@shravan20
Copy link

@mikelinhas: I am moving it in minor versions, specific domain of logic by logic.

Adding the first PR: #1277

@segersniels
Copy link
Collaborator Author

segersniels commented Mar 21, 2024

@mikelinhas I made some improvements in #1279 to the TS setup which should allow to properly change file by file without interfering with Flow too much.

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

No branches or pull requests

5 participants