Papooch / nestjs-cls

A continuation-local storage (async context) module compatible with NestJS's dependency injection.
https://papooch.github.io/nestjs-cls/
MIT License
434 stars 28 forks source link

FIX: Typing error with ClsStore interface #21

Closed ruslanguns closed 2 years ago

ruslanguns commented 2 years ago

This PRis going to closes #20.

Seems like Records works much better for mixings. Can't explain the deep reason but it fixes the issue with lower version of Typescript v4.3.5

Papooch commented 2 years ago

Thanks for the PR. I'll check it later today, but did you verify that it's still possible to augment the ClsInterface as outlined in the docs?

ruslanguns commented 2 years ago

Thanks for the PR. I'll check it later today, but did you verify that it's still possible to augment the ClsInterface as outlined in the docs?

Yes, and everything works as expected to be:

In this image you can see the implementation of my simply service which applies the generics of my interface: image

And in this the cool strict typing for autocompletion still works: image

Papooch commented 2 years ago

@ruslanguns this should work as expected, but I was talking about interface augmentation using

declare module 'nestjs-cls' {
    interface ClsStore {
       demoData: string
    }
}
ruslanguns commented 2 years ago

Without the fix: image

And yes applying the fix with Records still makes possible to work with interface augmentation: image

Papooch commented 2 years ago

Awesome, though it's not exactly the intended use case - you shouldn't need to pass the interface (ClsStore) as a type parameter to ClsService when working with interface augmentation.

Anyway, I'll review it as soon as I get to the computer.

ruslanguns commented 2 years ago

That's makes lot's of sense, nothing should be changed... When I'm back home will test without passing it though the generics. Thanks mate

Papooch commented 2 years ago

@ruslanguns Alright, so I had some time to investigate and this is what I found: 1) I forgot to export ClsStore from the module, that's why you had to import it from dist/lib/.... I'm going to fix this ASAP. 2) If you declare ClsStore as type, then augmenting the interface is not possible (because type augmentation is not allowed)
image But that's fixable by instead delcaring it as follows:

export interface ClsStore extends Record<symbol, any> {};

But because typescript < 4.4 does not support symbol members in interfaces, it does not help and produces a confusing error image

3) The library itself is not buildable with typescript < 4.4 because it relies on new features of it.

As such, I'm sorry, but I'll have to reject this PR and declare that typescript >= 4.4 is required to use the typing feature in nestjs-cls > 2, otherwise, version 1.6.2 should be used.