ConfigModule.forRoot() changes ConfigService singleton even if ConfigModule.forRoot() is not imported

[ericmorand]

The simple fact of calling ConfigModule.forRoot() changes the ConfigService singleton, even if ConfigModule.forRoot() result (a DynamicModule) is not imported by any dependent module.

Consider the following entry point index.ts:

import {INestMicroservice, Module} from "@nestjs/common";
import {NestFactory} from "@nestjs/core";
import {MicroserviceOptions, Transport} from "@nestjs/microservices";
import {ConfigModule, ConfigService} from "@nestjs/config";
import {strictEqual} from "assert";

@Module({
    providers: [
        ConfigService,
        {
            provide: 'Bar',
            useFactory: (config: ConfigService) => {
                strictEqual(config.get('FOO'), 'BAR'); // should throw since ConfigService is not "dot env enabled"
            },
            inject: [
                ConfigService
            ]
        }
    ]
})
class MainModule {

}

/**
 * Never imported by any module but still mutate ConfigService singleton.
 */
const forRootConfigModule = ConfigModule.forRoot();

export const bootstrap = (port: number): Promise<INestMicroservice> => {
    return NestFactory.createMicroservice<MicroserviceOptions>(
        MainModule,
        {
            transport: Transport.TCP,
            options: {
                port
            }
        },
    );
}

bootstrap(8888);

And the following .env file:

FOO=BAR

Executing index.ts would not throw an error, even though it should since ConfigService has not been configured as "dot env enabled" by any module imported by MainModule or by MainModule itself.

I understand where it is coming from: the simple fact of executing forRoot() mutate ConfigService, but I think this is a non-expected behavior: since the module returned by forRoot() is never imported by MainModule, it should not have any side effect on MainModule scope.

In other words, my interpretation of the issue is that forRoot() pollutes the scope of MainModule in an unexpected way.

How to double-check the issue

Commenting the line const forRootConfigModule = ConfigModule.forRoot(); and executing the script throws an AssertionError, as expected.

What do you think?

[kamilmysliwiec]

Well, so this one is somewhat tricky. ConfigModule is one of these modules which actually introduces side-effects immediately after calling the forRoot() method (as you've noticed) and this is actually, sort of the expected behavior. Basically, forRoot() triggers loading the environment variables from .env files using dotenv package (and process.env which holds env vars isn't scoped to any specific application, it's global for a Node process). This must happen right away (we could theoretically delay it a bit but I'm not sure whether that would be valuable) because these vars are typically crucial for other modules (and so they must be present before any provider is initialized - for example, we can't put this logic in the ConfigModule class constructor because that would mean, in some cases, env vars could have been loaded after some other providers are already initialized).

What we could do here (to somehow address this issue), instead of triggering this logic right away inside the forRoot() method, we could add a new hook (at the @nestjs/core package level). This hook would be executed when module is indeed imported by any other module but before any of them is "processed". However, this would require changes in the core (as I've mentioned earlier) and would be useful only for this particular library (config package), which doesn't sound perfect.