Closed Ponjimon closed 4 years ago
Can you provide a minimum reproduction?
@jmcdo29 it's related to register .forFeature
and in the same module .forRoot
.
Try to move it to separate modules and import it to one common, it's only workaround, we need to address that issue.
Another way to make it work is workaround .forFeature
and inject provider by yourself (eg. I solve that way on tests because there I didn't have a good way to separate it to modules), but then if you would like to inject it to your module imo it will be a very bad decision because you would need to use instance of OgmaService
where you need to provide options etc. so use core features of library, for tests it works because there you don't need to provide instance just mock (as far as you wouldn't need logs in your tests).
{
...
providers: [
{
provide: createRequestScopedProviderToken(SomeService.name),
useValue: jest.fn()
}
]
}
(It's for request scoped providers, if you would like to use not request scoped use createProviderToken
instead)
It's probably related to dependency @golevelup/nestjs-modules
and not in @ogma
itself
I did that now, the error is gone, but there is no log at all. I added some console logs to ogma.interceptor.js
inside node_modules
and OgmaInterceptor
is not even being instanciated...
In version 0.3.0 and on, the interceptor is no longer auto attached. This is to allow for better customization of the request id generation.
From the README for @ogma/nestjs-module
Note: As of version 0.3.0 the
OgmaInterceptor
is not bound by default, it is still necessary to pass all of the expected configuration options due to the way that the dependencies are built. If you would like to bind the interceptor globally, you can still do so usingAPP_INTERCEPTOR
in a custom provider. The interceptor is not bound by default anymore to allow for more customization when it comes to the generation of request IDs.
So, the interceptor works now, but the main issue is still there.
I also made a codesandbox where you can reproduce it.
On the left navigation bar, go to the Server Control Panel
and click Restart Server
. In the terminal output you will then see the above error message.
https://codesandbox.io/s/nest-typescript-starter-forked-ofwpb?file=/src/app.module.ts
I also included the LoggerModule
I'm using in my project in this codesandbox, maybe I'm doing something wrong, but afaik, I'm just using it as per documentation so it should work?
It's logging in the AppService
@Ponjimon in the future, GitHub repositories are preferable to codesandbox reproductions.
I noticed in your package.json you had several @nestjs/
dependencies still on version five while others were up on version 7. These should always keep the same major version, just a heads up.
I was able to reproduce the bug, and have a workaround for the time being. I never really anticipated anyone to move the OgmaModule.forRoot/Async()
method out of the RootModule
, so this wasn't in my suite of tests. It's a problem with Nest's module resolution system and with how @golevelup/nestjs-modules
works with creating dynamic modules. For now, what can be done, in your LoggerModule
do something like this:
import { Module } from '@nestjs/common';
import { APP_FILTER, APP_INTERCEPTOR } from '@nestjs/core';
import { OgmaModule, OgmaModuleOptions } from '@ogma/nestjs-module';
import { ExpressParser } from '@ogma/platform-express';
import { GraphQLParser } from '@ogma/platform-graphql';
import { AppService } from 'app.service';
import { GqlHttpExceptionFilter } from './gql-http-exception.filter';
import { LoggingInterceptor } from './logging.interceptor';
export const OGMA_MODULE_OPTIONS: OgmaModuleOptions = {
service: {
color: Boolean(process.env.IS_OFFLINE),
},
interceptor: {
http: ExpressParser,
gql: GraphQLParser,
},
};
@Module({
imports: [
OgmaModule.forFeature(AppService),
OgmaModule.forRootAsync({
useFactory: (): OgmaModuleOptions | Promise<OgmaModuleOptions> =>
OGMA_MODULE_OPTIONS,
}),
],
providers: [
{ provide: APP_INTERCEPTOR, useClass: LoggingInterceptor },
{ provide: APP_FILTER, useClass: GqlHttpExceptionFilter },
],
exports: [OgmaModule]
})
export class LoggerModule {}
Creating the OgmaModule.forFeature()
in there and exporting it, then in your AppModule
just having imports: [LoggerModule]
I'll see what I can do to make this a better dev experience. I'm thinking I may just have to write the dynamic module myself and not use the @golevelup
package, but we'll see if there's anything else I can do first.
That's a bummer, having to do that inside the LoggerModule makes the whole purpose of that module obsolete because that creates a circular dependency ModuleThatUsesLogger <=> LoggerModule.
Thanks anyway, hopefully, this can be fixed in a future update.
The reason I did it that way was because I am using this LoggerModule in many Nest applications and not just one, so I wanted to reduce code (duplication).
To finally circle back around to this, I'm gonna work on phasing out the usage of OgmaCoreModule.Deferred
and just use a @Global()
decorator instead. I'll make sure to add this to the test suite and see if it fixes the issue there 😄 I'll try to have something up by the end of the weekend
Added to v0.4.0. This will most likely be the last pre v1 version. Thanks for the idea
Bug Report
I get the above error when running
^0.2.2
and above but not with^0.1.0
I cannot find any breaking change about this?
Current behavior
The app doesn't start because of that error.
Expected behavior
It should start.
Environment