feat: migrate to typescript

chaijs / chai

BDD / TDD assertion framework for node.js and the browser that can be paired with any testing framework.

https://chaijs.github.io

MIT License

8.11k stars 694 forks source link

feat: migrate to typescript #1567

Open 43081j opened 8 months ago

43081j commented 8 months ago

This is an initial attempt at migrating the codebase to typescript.

The tests and build should pass but we can go further and do more with the types.

WIP until we do that.

cc @keithamus @koddsson

TODO

[ ] strengthen a lot of the types with inference etc
[ ] remove as many casts as possible
[ ] add some $ExpectType tests (there's an eslint plugin for this now)
[ ] run a formatter
[ ] add a linter

Some of these should probably be in their own PR.

For reviewers

Some things to review thoroughly:

the various overloads of assertion functions
- we do a lot of overloading which is super awkward in a strongly typed world, so these need a very close look
the addition of files in the package manifest
- make sure we pack the right files

koddsson commented 8 months ago

WIP until we do that.

I'm gonna convert this to a draft while it's a WIP. Hope that's OK :)

SmashingQuasar commented 6 months ago

Hey @43081j !

I love Chai and I have been using it for years. This PR is extremely interesting and migrating to TypeScript would do a world of good to this library. If you want, I'm ready to help you with this so we can lift Chai to the next level. Just let me know so we can sync' on this and work together to finish it up. :)

43081j commented 6 months ago

before we can really split the work up to contributors etc, i think i really need a thorough review from @koddsson or @keithamus to make sure we're on the right track and in agreement

there's a couple of awkward patterns lying around that i want to clean up before this can ever land:

passing a createDefaultValue around feels awkward, but its better than a circular import. i feel like there must be a third solution though with a bit of a refactor
logic has changed in some places, usually in functions with overloads. not sure how we want to deal with that in terms of semver
there's a bunch of casts i think we can get rid of with some thought

SmashingQuasar commented 6 months ago

I've browsed the code base quite a bit and I think there is some work to be done after migrating to TypeScript. There are some code sections that look really difficult to understand and maintain. Especially those functions created within functions. There are also some wild var at different places and other things that look like they could be improved.

It's most likely an heirloom of some older sections of the code base which is to be expected. All of this to say that it would be easier to have a first base that is using TypeScript, event approximately, and then refactor fractions of the code base in different PRs.

My point is: I don't think it's too big of a deal if you have some things that aren't totally perfect, we can work on this afterwards. :)

I'm not a maintainer, not even a contributor yet, so it's just the opinion of one random dev'. ;)

43081j commented 5 days ago

FYI i've caught this up and manually diffed against the current JS sources (since git won't do that for everything in this case)

everything seems fine so I think we should just ship a next tag or something and get people to try it out for a while

I considered the prev comment too but doing a typescript conversion a step at a time will be pretty difficult since everything is so intermingled