Skip to content
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

ts second args dont working #956

Open
2 of 4 tasks
productdevbook opened this issue Jun 2, 2022 · 8 comments
Open
2 of 4 tasks

ts second args dont working #956

productdevbook opened this issue Jun 2, 2022 · 8 comments
Labels
bug Something isn't working

Comments

@productdevbook
Copy link

productdevbook commented Jun 2, 2022

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

image

Minimum reproduction code

https://github.com/productdevbook/nestjs_bug

Steps to reproduce

  1. yarn install
  2. go to vscode app.service.ts

image

Expected behavior

jwt.private etc typescript see. But dont see

Package version

2.1.0

NestJS version

8.4.6

Node.js version

v16.15.0

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

No response

@productdevbook productdevbook added the bug Something isn't working label Jun 2, 2022
@micalevisk
Copy link
Member

micalevisk commented Jun 2, 2022

indeed. The type inference from ConfigService doesn't work with the latest version of typescript (v4.7.2), but it does in v4.6.3. Here's the demo of such:

demo.mp4

I can't tell if it's a limitation or a bug tho

@kamilmysliwiec
Copy link
Member

So it seems that the feature itself works as expected but the auto-complete is no longer useful, at least that's what I reproduced

@micalevisk
Copy link
Member

micalevisk commented Jun 3, 2022

yeah

I read the whole https://devblogs.microsoft.com/typescript/announcing-typescript-4-7 but I still don't understand why that didn't work anymore. Maybe it's related with Stricter Checks with Template String Expressions

@micalevisk
Copy link
Member

I found another type utility for the dot notation type inference here

It does work but I didn't manage to make it work with ConfigService yet

image

@baconcheese113
Copy link

Thanks for all your work on this @micalevisk

Is there any chance of making infer: true the default value? Using dot-notation through a string value with autocomplete is a neat feature, but I wonder if it's worth the months of tech debt on this. It would be nice to simplify the interface to just

constructor(
    private readonly configService: ConfigService<AppConfig>,
) {
    const database = configService.get('database');
}

And still have database be typed correctly as an object with nested and port

@micalevisk
Copy link
Member

@baconcheese113 it would introduce a breaking change. But yeah, we could change that in the next major release. I don't know why it is not the default behavior, tbh

@baconcheese113
Copy link

Oh additionally, we already validate the runtime value of our config object when registering the ConfigModule.

So it would be nice if configService.get would narrow the type based off the generic parameter instead of assuming all values can be undefined if not called with getOrThrow, maybe through some config we can provide to the ConfigModule?

@micalevisk
Copy link
Member

micalevisk commented Jul 21, 2023

@baconcheese113 that's not feasible in TS, if I understood you correctly

Please use our Discord server to discuss about it or other requests if you want to.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants