Skip to content

FIX: Typing error with ClsStore interface #21

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Conversation

ruslanguns
Copy link

@ruslanguns ruslanguns commented Feb 27, 2022

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

This commit is going to close the Papooch#20.
@Papooch
Copy link
Owner

Papooch commented Feb 27, 2022

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
Copy link
Author

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
Copy link
Owner

Papooch commented Feb 27, 2022

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

declare module 'nestjs-cls' {
    interface ClsStore {
       demoData: string
    }
}

@ruslanguns
Copy link
Author

Without the fix:
image

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

@Papooch
Copy link
Owner

Papooch commented Feb 27, 2022

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
Copy link
Author

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
Copy link
Owner

Papooch commented Feb 27, 2022

@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

  1. 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.

@Papooch Papooch closed this Feb 27, 2022
@ruslanguns ruslanguns deleted the ruslanguns/fix-typing-error-with-ClsStore-interface branch February 28, 2022 10:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Getting typing error with ClsStore interface
2 participants