search

francescov1 / mongoose-tsgen

A plug-n-play Typescript generator for Mongoose.
102 stars 24 forks source link

Why are interfaces augmented into the mongoose module? #9

Closed HoldYourWaffle closed 3 years ago

HoldYourWaffle commented 3 years ago

I just stumbled on this project and I have to say it looks great! I'll probably start using it and might even put in some contributions if I ever find the time... I have some experience with TS AST reading/manipulation from similar projects, so perhaps I could be useful.

However, I noticed something odd while reading the README: why are generated interfaces augmented into the mongoose module? Is there a specific reason for this? It seems like a very odd choice to me...

Edit: possibly related to this, what is the intended use for the custom.d.ts file? What kind of "custom interfaces" would need to be defined there, and why do they have to be augmented into the mongoose module? Wouldn't it make more sense to leave this type of finnicking to the user instead of always generating a dedicated file for it?

HoldYourWaffle commented 3 years ago

I just discovered that in my specific (edge-case-y) situation this behavior is actually hurtful.

I'm using ts-node to ease the recompile-run loop during development. For performance reasons this module does not eagerly load type definitions, in other words: type definitions are not found unless the file containing them is explicitly referenced somewhere.

This doesn't cause issues in normal circumstances, since types are usually imported from where they're defined. However, with this module I'm importing from mongoose, while the definitions are in types/mongoose/index.d.ts. The latter is not picked up by ts-node, because it's never explicitly referenced anywhere (since I'm referring to mongoose instead).

I have found 3 ways to fix this issue:

  1. Use the --files option with ts-node. This is undesirable for performance reasons.
  2. Reference the definition file with a triple-slash-directive. Also undesirable in my opinion.
  3. Don't use augmentation, just export & import generated definitions normally. Since we're now explicitly referencing the file where the definitions actually are, it's picked up by ts-node and all errors melt as snow in spring.

Note that it's not my intention to change this module "because ts-node", if there's an actual reason for these augmentation-shenanigans I'll gladly fumble around with triple-slash-directives ;)

francescov1 commented 3 years ago

You're totally right! I'll be cleaning this up over the weekend, there's a couple requests to export these interfaces normally.

francescov1 commented 3 years ago

This is addressed in PR #16, need to still fix tests and README before merge, but feel free to check it out! (updates specified in description)

Also, would love to get some help working with Typescript AST, I'm relatively new to it but have found ts-morph makes it quite straight forward!

HoldYourWaffle commented 3 years ago

Great to hear! I'll take a look at the PR in a couple of minutes.

The things that really helped me when working with TS AST was ts-ast-viewer.com/# (made by the same dude as ts-morph). Actually seeing the tree makes a lot of things easier, combined with ts-morph things have been relatively painless for me... If you have anything specific you'd like help with I'd be happy to take a look 😄

francescov1 commented 3 years ago

Released in v5.1.0!