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

☂️TypeScript migration tracking issue #7807

Closed
SimenB opened this issue Feb 5, 2019 · 70 comments
Closed

☂️TypeScript migration tracking issue #7807

SimenB opened this issue Feb 5, 2019 · 70 comments

Comments

@SimenB
Copy link
Member

SimenB commented Feb 5, 2019

This issue is meant to track Jest's internal migration to TypeScript.

Process

We would love to get help from the community here. I've compiled a list of all the packages in this repo below - feel free to claim any package not claimed and submit a PR migrating it.

List

Package Name Task Owner PR Status Notes
@jest/core @SimenB #7998 MERGED
@jest/reporters @loryman #7994 MERGED
@jest/transform @SimenB #7918 MERGED
babel-jest @SimenB #7862 MERGED
babel-plugin-jest-hoist @deneuv34 #7898 MERGED
babel-preset-jest - - - Not transpiled
diff-sequences @loryman #7820 MERGED
eslint-config-fb-strict - - - Not transpiled
expect @natealcedo #7919 MERGED
jest @SimenB #8024 MERGED
jest-changed-files @loryman #7827 MERGED
jest-circus @doniyor2109 #7916 MERGED
jest-cli @SimenB #8024 MERGED
jest-config @SimenB #7944 MERGED
jest-diff @SimenB #7824 MERGED
jest-docblock @SimenB #7836 MERGED
jest-each @mattphillips #8007 MERGED
jest-environment-jsdom @lirbank #8003 MERGED
jest-environment-node @lirbank #7985 MERGED
jest-get-type @SimenB #7818 MERGED
jest-haste-map @jeysal #7854 MERGED
jest-jasmine2 @doniyor2109 #7970 MERGED
jest-leak-detector @r3nya #7825 MERGED
jest-matcher-utils @SimenB #7835 MERGED
jest-message-util @SimenB #7834 MERGED
jest-mock @thymikee #7847 MERGED
jest-phabricator @r3nya #7965 MERGED
jest-regex-util @loryman #7822 MERGED
jest-repl @natealcedo #8000 MERGED
jest-resolve @SimenB #7871 MERGED
jest-resolve-dependencies @jeysal #7922 MERGED
jest-runner @natealcedo #7968 MERGED
jest-runtime @SimenB #7964 MERGED
jest-serializer @thymikee #7841 MERGED
jest-snapshot @doniyor2109 #7899 MERGED
jest-util @SimenB #7844 MERGED
jest-validate @SimenB #7991 MERGED
jest-watcher @SimenB #7843 MERGED
jest-worker @SimenB #7853 MERGED
pretty-format @SimenB #7809 MERGED

After all packages are migrated, we can start to migrate our integration tests. Depending on how this migration goes, we can track that in this issue as well, or we can track it separately later.

How

Order of packages to migrate

One thing to note is that because this repo is a monorepo, we have dependencies between packages. So we have to migrate leaf packages (without dependencies) first, then walk up the dependency tree until everything is migrated. Which means jest-cli will be the last package migrated.

You can use yarn to figure out which packages depend on which internally: yarn --silent workspaces info. This will output a JSON object of all packages in the workspace. An example looks like this:

{
  "babel-jest": {
    "location": "packages/babel-jest",
    "workspaceDependencies": ["babel-preset-jest"],
    "mismatchedWorkspaceDependencies": []
  }
}

The interesting part here is workspaceDependencies. If that array is empty, that's a perfect package to start migrating. If it is not empty, you'll want to make sure that every package listed in the array has been migrated already.

Steps

  1. Claim a package in this issue
  2. Copy tsconfig.json from an already migrated package
    1. If the package you're migrating have dependencies on another package in this repo, use references
  3. If a type file exists in types/ in the root of the repo for the package, move that into the package's src directory as a file named types.ts
  4. Add "types": "build/index.d.ts" to package.json below main
  5. Rename all files with js extension to ts (or tsx if they have jsx in them), fixing type errors as you go
  6. Make sure tests and lint (including flow) pass
  7. Ensure that the JS after compilation is essentially the same as before migrating*
  8. Open up a PR

To build, you can run yarn build or yarn watch in the root of the repository.

You can look at my PR for pretty-format for a look at how a migration might look.

You can use flow-to-typescript to help in migration. However, since the syntax between Flow and Typescript is so similar, I personally only used it for the type definition files in types - for normal source files it was easier to rename the file to ts(x) and fix the syntax errors that my IDE showed me.

*) Do this by comparing git diffs before and after migration (also please include this diff in the PR after opening it)

  1. run yarn build on master
  2. git add -f packages/MY_PACKAGE/build*
  3. git commit -m 'before migration'
  4. run yarn build on your branch with the migration
  5. rm packages/MY_PACKAGE/build*/**/*.ts packages/MY_PACKAGE/build*/**/*.map
  6. git add -f packages/MY_PACKAGE/build*
  7. git commit -m 'after migration'
  8. git diff master packages/MY_PACKAGE/build*
  9. On macOS (there probably exists similar tools on Linux and Windows), this can be copied and included in the PR git --no-pager diff master packages/MY_PACKAGE/build* | pbcopy. Stick that in a code fence with diff syntax in the PR.

Things to look out for during migration

The config doesn't allow implicit any

Out current setup with flow allows this - just add an explicit any (or a stricter type if you're able) in those cases. If possible, please use unknown instead of any.

The module exports CommonJS

Convert require to import Use exports = to replace modules.exports - this allows TypeScript to understand the export. We include a babel-plugin which transpiles this into module.exports for the distributed code (and tests).

Potential gotchas

Probably more, but I'll write down the ones I know of

  • Jest currently imports quite a lot of types from types/ in the root of this repo, allowing us to use types across packages without dependencies on them (typically modules will depend on types/Config which allow them to have ProjectConfig as arguments). Since we'll be distributing the types, we need those dependencies to be explicit.
    • A solution in this concrete case is probably to create a separate packages that has everything jest-config has today except for the default configs (so it can drop babel-jest, the test environments etc)
  • Similarly, types/TestResult is used by jest-jasmine2, jest-circus and jest-cli (for reporters). We need to figure out how to share that type effectively between packages.

Another idea on how to solve "type sharing" is to use a separate jest-types project that's just types that are shared between modules.

Ideas here are very much welcome!

EDIT: As of #7834, I've created a @jest/types package


PS: This will not affect anyone using Jest (unless you use the modules exported by Jest directly, which will become typed). This is strictly an internal refactor. We will not be considering adding types to be a breaking change.

PPS: It is currently a non-goal to distribute TS types for using Jest - that is excellently handled in @types/jest already. At some point we might want to distribute that with jest as well - but that's a separate issue.

PPPS: This issue is not for discussions about the merits of the migration in and of itself - that feedback can be posted in the RFC. However, if you have experience migrating, building, testing or distributing a module written in TS, feel free to chime in with learnings from that process in this issue.

@lorenzorapetti
Copy link
Contributor

I would love to contribute on this! Can i take diff-sequences for now? I've already ported it in my branch and all the tests for the module are passing.

@SimenB
Copy link
Member Author

SimenB commented Feb 6, 2019

Yeah, go for it!

@r3nya
Copy link
Contributor

r3nya commented Feb 6, 2019

Hey @SimenB!
I'd be happy to help. Can you recommend a package to start with? :)

@SimenB
Copy link
Member Author

SimenB commented Feb 6, 2019

That's awesome @r3nya!

I'd say the easiest are babel-plugin-jest-hoist, jest-leak-detector and jest-phabricator.

Others that should be pretty straightforward are jest-changed-files, jest-diff, jest-docblock, jest-message-util, jest-regex-util and jest-serializer.

@lorenzorapetti
Copy link
Contributor

Yeah, go for it!

Thank you! I could take jest-changed-files and jest-regex-util as well.

@SimenB
Copy link
Member Author

SimenB commented Feb 6, 2019

🎉 I've updated the OP

@thymikee thymikee changed the title TypeScript migration tracking issue ☂️TypeScript migration tracking issue Feb 6, 2019
@SimenB SimenB pinned this issue Feb 7, 2019
@r3nya
Copy link
Contributor

r3nya commented Feb 7, 2019

@SimenB let me do a jest-leak-detector and jest-phabricator for now. ;)

@thymikee
Copy link
Collaborator

thymikee commented Feb 7, 2019

@r3nya go for it! :)

@lorenzorapetti
Copy link
Contributor

lorenzorapetti commented Feb 7, 2019

So, jest-changed-files imports from types/Config and types/ChangedFiles which are imported by several packages as well. What do you think is the right way to go from here?

@SimenB
Copy link
Member Author

SimenB commented Feb 7, 2019

Path is just string, so that should be fine to replace. You can to type Path = string to keep the diff down 🙂

types/ChangedFiles can be copied and moved into jest-changed-files (similar to what I did with types/PrettyFormat)

@lorenzorapetti
Copy link
Contributor

Got it, thanks! I've created the PR :)

@MoKhajavi75
Copy link

I'll take jest-repl 😁

@SimenB
Copy link
Member Author

SimenB commented Feb 8, 2019

@MohamadKh75 thanks! However, that depends on jest-runtime, jest-config and jest-validate, all of which have to be migrated first. You can try jest-validate?

@mattphillips
Copy link
Contributor

Happy to migrate jest-each over 😄

@SimenB
Copy link
Member Author

SimenB commented Feb 8, 2019

Woo, do it 😀

@MoKhajavi75
Copy link

@MohamadKh75 thanks! However, that depends on jest-runtime, jest-config and jest-validate, all of which have to be migrated first. You can try jest-validate?

Sure! then i start jest-validate

@SimenB
Copy link
Member Author

SimenB commented Feb 9, 2019

awesome!

@jeysal
Copy link
Contributor

jeysal commented Feb 10, 2019

I'll try my luck with haste map 😄

@lirbank
Copy link
Contributor

lirbank commented Feb 11, 2019

I'd be happy to do jest-environment-node and jest-environment-jsdom.

@SimenB
Copy link
Member Author

SimenB commented Feb 11, 2019

Awesome, thanks! OP updated 🙂

@deneuv34
Copy link
Contributor

Hi, can I do the first try for babel-plugin-jest-hoist?

@SimenB
Copy link
Member Author

SimenB commented Feb 11, 2019

For sure!

@SimenB
Copy link
Member Author

SimenB commented Feb 25, 2019

Woah, we're almost done! Awesome 😀 If you want, you can start converting the e2e tests? Just the ones in this directory, not the tests themselves: https://github.com/facebook/jest/tree/master/e2e/__tests__
It essentially involves converting Utils to TypeScript I think, after which the tests themselves should just be renaming the file

I'll ping the ones that have claimed packages but not opened up a PR yet, might be more available.


@mattphillips @lirbank @MohamadKh75 hey, one more ping. If you don't have time to migrate that's totally fine, but if so it'd be nice if someone else could pick up the package you've claimed 🙂

@natealcedo
Copy link

natealcedo commented Feb 25, 2019

The link you provided returns a 404. But you're referring to the contents of the __tests__ directory I presume?

If that's the case I'll gladly take it up 👍

@SimenB
Copy link
Member Author

SimenB commented Feb 25, 2019

Yes! Not sure why GH messes up the link, what I pasted is https://github.com/facebook/jest/tree/master/e2e/__tests__. Seems like a bug 🤷‍♂️

@natealcedo
Copy link

Am I going to need to have a tsconfig.json in the __tests__ directory?

@SimenB
Copy link
Member Author

SimenB commented Feb 25, 2019

I don't know. I don't think so (we won't be generating typings files here)? Try without 🙂

@natealcedo
Copy link

Sure thing. 💯

@doniyor2109
Copy link
Contributor

@SimenB There is also jest-repl left

@SimenB
Copy link
Member Author

SimenB commented Feb 25, 2019

It has dependencies that have not been migrated. But I guess we can just do ts-ignore on those imports for now, I think we've got a good enough of a setup to support that now. It should take like 5 minutes, though 🙂

@lirbank
Copy link
Contributor

lirbank commented Feb 25, 2019

@SimenB Sorry for the lag, got sidetracked and was out all week. I'll get it done today!

@SimenB
Copy link
Member Author

SimenB commented Feb 25, 2019

Awesome. I've added a @jest/environment package which is supposed to export an interface to be implemented by the environments. I haven't tested it though, so it might not work properly. Feel free to open an early PR if it gives you difficulties

@MoKhajavi75
Copy link

Woah, we're almost done! Awesome If you want, you can start converting the e2e tests? Just the ones in this directory, not the tests themselves: https://github.com/facebook/jest/tree/master/e2e/__tests__
It essentially involves converting Utils to TypeScript I think, after which the tests themselves should just be renaming the file

I'll ping the ones that have claimed packages but not opened up a PR yet, might be more available.

@mattphillips @lirbank @MohamadKh75 hey, one more ping. If you don't have time to migrate that's totally fine, but if so it'd be nice if someone else could pick up the package you've claimed

I'm so sorry... i just had some unexpected problems... maybe i can help in other sections... 😞

@SimenB
Copy link
Member Author

SimenB commented Feb 26, 2019

No problem @MohamadKh75, thanks for reaching out! 🙂 You can do jest-repl if you wanna do a package? We're at the point where we don't have to wait for dependencies. If you don't have time, that's no problem at all, of course - we're all doing this in our spare time

@natealcedo
Copy link

Hey can I take up jest-repl since @MohamadKh75 isn't available to pick up the task? :)

@SimenB
Copy link
Member Author

SimenB commented Feb 27, 2019

Sure, go for it 🙂

@SimenB
Copy link
Member Author

SimenB commented Mar 2, 2019

Hey @loryman, I picked up jest-cli since you're busy with @jest/reporters 🙂

@lorenzorapetti
Copy link
Contributor

Hey @loryman, I picked up jest-cli since you're busy with @jest/reporters 🙂

Sure, go ahead!

@SimenB
Copy link
Member Author

SimenB commented Mar 5, 2019

Holy shit, we're done! 🎉

Thank you so much to everyone who helped out. I did not think we'd be able to do this in just 4 weeks.

Jest is now officially a TS repo.

image

$ cloc packages --vcs git --exclude-dir __tests__,__mocks__
     462 text files.
     408 unique files.
      76 files ignored.

github.com/AlDanial/cloc v 1.80  T=0.45 s (896.0 files/s, 98149.4 lines/s)
-------------------------------------------------------------------------------
Language                     files          blank        comment           code
-------------------------------------------------------------------------------
TypeScript                     286           4581           4052          29135
JSON                            82              0              0           2106
Markdown                        19            571              0           1697
JavaScript                      16            217            213           1497
PHP                              1             22             14            150
-------------------------------------------------------------------------------
SUM:                           404           5391           4279          34585
-------------------------------------------------------------------------------

Next steps are as follows:

  • Release an alpha to verify we didn't break anything (done as jest@24.2.0-alpha.0)
  • Setup a replacement for @types/jest
  • Convert more of the unit tests to TS
  • Setup tests to verify our types (especially around inferring the type of jest spies)

I'll be opening issues for these soon(ish).

Thank you again to everybody who chipped in!

@SimenB SimenB closed this as completed Mar 5, 2019
@SimenB SimenB unpinned this issue Mar 5, 2019
@SimenB
Copy link
Member Author

SimenB commented Mar 5, 2019

Alpha released! Please do yarn add -D jest@beta or npm i -D jest@beta (currently jest@24.2.0-alpha.0) and complain if it explodes horribly 🙂

@github-actions
Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
Please note this issue tracker is not a help forum. We recommend using StackOverflow or our discord channel for questions.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 12, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests