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.15k stars 698 forks source link

Add types #1557

Closed koddsson closed 10 months ago

43081j commented 11 months ago

Just FYI so we don't duplicate work again: I also did this in a branch but threw it away to work on a typescript migration instead.

To be clear, don't start migrating to typescript, otherwise this will be the third time we have done the same work in parallel 😂

As a precursor to that, this would be a good one to merge.

43081j commented 11 months ago

Couple of things I did different that may be worth considering:

koddsson commented 11 months ago

I feel like I've underestimated the amount of work we'd need to do to get types to work :D

Couple of things I did different that may be worth considering:

  • change all occurrences of String to string
  • use typescript JSDoc notation for optional params (e.g. @param {string=} paramName)
  • a few methods that mutate flags and stuff should really still consume an object and cast internally, since flags and friends are internally just stuck onto whatever you give it afaict

Yeah I think we should enforce this with a ESLint plugin.

43081j commented 11 months ago

I feel like I've underestimated the amount of work we'd need to do to get types to work :D

it may be possible to just strongly type the exposed API and cast internally where needed for now.

if you start fixing up all the type errors etc, you're verging on converting it to typescript already. that is exactly what lead me to it - i was fixing up enough types that i realised i may as well be writing typescript... to avoid doing the same effort twice (once in jsdoc, once in type annotations).

im on a different laptop to the one with my typescript branch for the next few days, but once im back on there i'll push what i have

koddsson commented 10 months ago

Closing this in favor of @43081j PR: https://github.com/chaijs/chai/pull/1567