search

expressjs / session

Simple session middleware for Express
MIT License
6.21k stars 973 forks source link

First-class Typescript support #723

Open dougwilson opened 4 years ago

dougwilson commented 4 years ago

Traditionally for the express ecosystem, typescript definitions have been created and maintained by great contributors to the DefinatelyTyped project. Types would be pulled in under a scope @types, separate from the module itself (@types/express-session, for instance).

These types are of various quality and many of the express community (like myself) is not familiar with (or use) Typescript, so don't know about it / would be able to identify an issue there.

@HoldYourWaffle graciously has worked on fixing up the type definitions for this module and created PR #656 to add the definitions to the module package directly.

This brings up languishing sticking points for the longevity of this, of course, that I would like to discuss to understand as the package maintainer.

Preface: Since the types would become part of the package, a given version with some new function signature would need a corresponding definition update. Failure to do so would mean a follow up release would need to me made (after eventually someone in typescript community tries to use it and then realizes, opens an issue, etc.). Under the @types a new version of the definitions would need to be published to add a new signature, which seems natural if the project doesn't keep them up.

That brings my thoughts always to the following: How can we prevent this "fatigue" between the maintainer and the typescript users?

Is there some kind of automation that could be added to make a PR fail if type definitions were not updated correctly? Or maybe someone who can commit for at least some kind of long-haul, like at least a year, to review PRs / changes as they come in and maybe even provide what the corresponding type change is?

I'm not sure, of course, about the above being disconnected from typescript.

(note @HoldYourWaffle , this entire thread is a meta discussion, unrelated to actually landing your PR into this module, but related to it's protentional future removal if problem arise and there is no solution in place).

HoldYourWaffle commented 4 years ago

As I already said in #656, keeping the (fixed) typings in DT would probably be the best solution for now. I still prefer integrating the typings into the module eventually, but this shouldn't be done at the expense of rushing v2.

About the "synchronization" problem: as far as I know it's not possible to automatically test the accuracy of type definitions. However, I'd be happy to check all incoming PR's for typing changes.

mastermatt commented 4 years ago

As an option: as of TS 3.7, it is possible to generate declaration files from JSDoc annotations. It's not perfect, but pretty good. There is still a potential for drift as changes are made, but I've found it easier to keep the doc strings up to date in vanilla JS vs a secondary .d.ts file. It also means any contributor, whether TS savvy or not, can be comfortable including the change with the code change.

dougwilson commented 4 years ago

That is awesome! I went down that road a log time ago and didn't find the tooling. But it exists now and I think that is a great potential avenue vs maintaining a separate file. Yep, they can drift, but with jsdocs front and center it should hopefully be harder than a separate file.

mastermatt commented 4 years ago

I'll do a POC PR on this repo so we can compare the output to the work done by @HoldYourWaffle.