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

HasuraModule with forRootAsync is not working #148

Closed
rigobcastro opened this issue May 13, 2020 · 5 comments · Fixed by #149
Closed

HasuraModule with forRootAsync is not working #148

rigobcastro opened this issue May 13, 2020 · 5 comments · Fixed by #149
Assignees
Labels
bug Something isn't working

Comments

@rigobcastro
Copy link

rigobcastro commented May 13, 2020

When I import the HasuraModule with forRoot it works

     HasuraModule.forRoot(HasuraModule, {
         secretFactory: 'myadminsecretkey',
        secretHeader: 'x-hasura-admin-secret',
    }),

But when I import the module with forRootAsync it doesn't work

    HasuraModule.forRootAsync(HasuraModule, {
      inject: [ConfigService],
      imports: [ConfigModule],
      useFactory: async (configService: ConfigService) => ({
        secretFactory: configService.get<string>('GRAPHQL_API_AUTH_TOKEN'),
        secretHeader: configService.get<string>('GRAPHQL_API_AUTH_HEADER'),
      }),
    }),

I think it's something that I'm missing because no errors appears on the console just the RouteExplorer doesn't map the routes.

Thanks for your support.

@WonderPanda
Copy link
Collaborator

Hey @rigobcastro thanks for reporting this. I can confirm that the route isn't being mapped when using forRootAsync and will look into this.

As a side note, much do you leverage the ConfigService in your current application? I'm working on a new package that prevents the need to use forRootAsync in these situations because the equivalent of a ConfigService will already be bound and globally available before NestJS DI system.

This enables previously impossible patterns like dynamically configuring decorators as well.

@WonderPanda
Copy link
Collaborator

I found the issue. I'll have a new package version published later today just need to update some test cases so this is caught in the future

@WonderPanda
Copy link
Collaborator

Hey @rigobcastro thanks for reporting this. It should be fixed as of @golevelup/nestjs-hasura@0.3.3

@rigobcastro
Copy link
Author

Thanks @WonderPanda for your fix.

I use the configService to storage secret values via environment files trying to comply with methodology https://12factor.net/config. Your idea sounds great let me know if you want a tester.

@WonderPanda
Copy link
Collaborator

@rigobcastro If you're still interested in testing the new config service checkout https://github.com/golevelup/ts-ecosystem/tree/master/libs/profiguration

It provides a number of benefits over the NestJS Configuration module that'll help you achieve 12factor:

  • intellisense/type safety out of the box (you don't need to do get<string>(key) because we already know it's a string)
  • exists outside the DI system so you can use it anywhere (even in decorators!)
  • automatically coerces values to the correct datatype (no more needing to do weird conversions for boolean strings)

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

Successfully merging a pull request may close this issue.

2 participants