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

[Question] load config files from multiple sources #42

Closed
johannesschobel opened this issue Nov 6, 2018 · 27 comments
Closed

[Question] load config files from multiple sources #42

johannesschobel opened this issue Nov 6, 2018 · 27 comments
Assignees
Labels
question Further information is requested

Comments

@johannesschobel
Copy link
Contributor

Dear all,

thank you very much for this nice package. I am quite new to TypeScript - therefore, this might be a stupid question. Anyway, I am going for this 😆

Consider the following scenario (folder structure):

src
   app
      modules
         cat
            config
               cat-config.ts
               customconfig.ts
               whatever.ts
         dog
            config
               dog-config.ts
         snake
            config
               snake-config.ts
   config
      app.ts
      database.ts
      filesystem.ts

Basically, i would like to load all config files which are scattered around my application. Basic config files, like the app.ts or database.ts are stored in the src/app/config folder, whereas "domain specific" config files are located within their own module folder (e.g., the cat module has its configuration stored in src/app/modules/cat/config/cat-config.ts

Upon start of the application, i would like to get and merge all config files together, so i can use config.get('cat-config.deeply.nested.value'); to access specific values.

How can i manage to solve this?

All the best

@bashleigh
Copy link
Collaborator

bashleigh commented Nov 6, 2018

Yea sure there should be 2 different methods you can do this. The first being merge. https://github.com/nestjs-community/nestjs-config#mergeglob-string-options-dotenvoptions-promise

@Module({})
export class CatModule implements NestModule {

    constructor(@InjectConfig() private readonly config) {}

    async configure(consumer: MiddlewareConsumer) {
        await this.config.merge(path.join(__dirname, '**/*-config.{ts,js}'));
    }
}

You should be able to do this for each module in cat, dog and snake.

However if you're not using modules in these directories (I recommend you do as it makes life easier) you can modify your glob:

@Module({
  imports: [ConfigModule.load(path.resolve(__dirname, '**/**/*-config.ts'))],
})
export class AppModule  {
}

@bashleigh
Copy link
Collaborator

bashleigh commented Nov 6, 2018

Or again; another method I just found in tests directory https://github.com/nestjs-community/nestjs-config/blob/master/src/__tests__/config.module.spec.ts#L11 you can just omit the config in your load string and use dog.config.parameters.in.file.

@bashleigh bashleigh self-assigned this Nov 6, 2018
@johannesschobel
Copy link
Contributor Author

Hey @bashleigh , thanks a lot for your very fast response on this issue..
However, i have a hard time understanding your snippet..

Do i still need to call the ConfigService.load() in the AppModule in order to load the "common" configs? AND then load the other config files like so:

async configure(consumer: MiddlewareConsumer) {
        await this.config.merge(path.join(__dirname, '**/*-config.{ts,js}'));
    }

and if so - wouldn't it be better to actually just have one dedicated ConfigLoaderService that does this once upon starting the application? Because actually, all my modules are structured the same way (e.g., all config files are located within src/app/modules/**/config/*.{ts,js} by default).

Again, sorry for the stupid question, but i am quite new to TypeScript and NestJS

@bashleigh
Copy link
Collaborator

Sorry I was a bit busy yesterday so gave a very minimal explanation.

I think the simplest solution you can choose for your scenario is to omit the 'config' string from your glob. Like so

import {ConfigModule} from 'nestjs-config';
import {Module} from '@nestjs/common';
import * as path from 'path';

@Module({
    imports: [ConfigModule.load(path.resolve(__dirname, '**/*-config.{ts,js}'))],
}) 
export AppModule {}

This will load every file that ends in '-config.ts' or '-config.js'. from the current working directory.

Another way (and I'll answer your second comment there) is to import the ConfigModule into your submodules.

wouldn't it be better to actually just have one dedicated ConfigLoaderService that does this once upon starting the application?

The ConfigService itself within the container is one instant. There shouldn't be more than one instance of ConfigService in your application. The load method is also static so all configs will be available everywhere provided they're loaded before you use them!

Do i still need to call the ConfigService.load() in the AppModule in order to load the "common" configs?

Yes! And just import ConfigModule into the submodules without calling load.

But to further your first question. Yes and it's possible by changing the glob path to search all sub directories instead of just a /config directory. Doing so means you won't need to call merge from your sub module.

I hope this helps!

@johannesschobel
Copy link
Contributor Author

Dear @bashleigh ,

thank you very much for your answer.. Today I had some spare time and was able to try some things out.. This is my current state - maybe you can give me some feedback about this approach in general?!

My thought process is to have the overall AppModule that is the starting point of my application. However, this module imports the CoreModule, which is responsible for the basic setup of my application.

@Module({
  imports: [CoreModule],
  controllers: [AppController],
  providers: [AppService],
})
export class AppModule {}

The CoreModule, should - amongst others - load the config files from all different locations of the application. Please keep in mind, that the config files are stored in

  • /src/app/config --> basic configuration files
  • /src/app/modules/**/config --> configuration files for this particular module

The CoreModule looks like this:

@Module({
  imports: [
    ConfigModule.load(path.join(PathHelper.configPath(), '*.{ts,js}')), // load the "basic" configuration files!
  ],
  controllers: [],
  providers: [ContainerService],
  exports: [],
})
export class CoreModule {
  constructor() {
    ContainerService.loadContainers();
  }
}

The described import here loads the "basic" configuration files from /src/config/*. My thought-process was, to handle loading the other configuration files in the ContainerService.loadContainers() method.

As i was not really able to "call" the respective ContainerService, i thought about adding it to the constructor() method of the CoreModule (as the core should "bootstrap" the other modules, this sounds quite good to me!).

Respective function looks like this:

@Injectable()
export class ContainerService {

  private readonly config;

  constructor(config: ConfigService) {
    this.config = config;
  }

  public static async loadContainers() {
    // get all available containers
    const containers = await ContainerHelper.getContainerNames();

    for (const container of containers) {
      // build an absolute path to this specified container ( = module)
      const containerPath = PathHelper.containerPath(container);

      // now call the loaders for each container
      await ConfigLoader.loadContainerConfiguration(containerPath);
    }
  }
}

And the loadContainerConfiguration(containerPath) method would then call something like config.merge(containerPath);

Unfortunately, this did not really work - mostly, because i do not know how to particularly call the ContainerService properly. Currently, i am calling it in a static way - and i do not know if this is correct. Further, when calling it static, i am not able to access the this.config member, which contains the injected ConfigService from your package - and, therefore, i am not able to merge() the configs..

Maybe you can guide me to the proper direction on how to solve this?
What do you think - in general - about this approach? Or is this complete bull"$%")§! 😆

Thank you very much for your time and effort!
All the best

@bashleigh
Copy link
Collaborator

Ok so! I made a repository for the easiest example. I wanted to add the a merge example but I'm struggling for time. Check out the repo! https://github.com/bashleigh/multi-config-example/blob/master/src/config.spec.ts
In the config.spec.ts file you can see I'm using .get('cat-config.lives') to get the loaded config from any file named *-config.

Merge would be used incredibly similar except is required to be called in the ModuleInit method like so

import { Module } from '@nestjs/common';
import { AppController } from './app.controller';
import { AppService } from './app.service';
import {ConfigModule} from 'nestjs-config';
import * as path from 'path';

@Module({
  imports: [ConfigModule))],
  controllers: [AppController],
  providers: [AppService]
})
export class SnakeModule implements OnModuleInit {
  constructor(config: ConfigService) {}

  async onModuleInit() {
    await this.config.merge(path.resolve(__dirname, 'config/*.{ts,js}'));
}

Then your configs should be stored by the file name.

Hope this helps! If I find some time this week I'll add the merge method to the repo as a separate file so you can compare the difference :)

And I'm sorry but your example confused me a little 😂 seems a little over complicated :p I'm being rushed out of the office now... (typical) I'll be back in a bit to finish my response!

@johannesschobel
Copy link
Contributor Author

Hi @bashleigh ,

thank you very much for your time and effort. I have put together a solution as well - i will share some code later, as i need to rush to my lectures, haha..

your idea with the await onModuleInit() is nice - i didn't knew about this.. I will post my solution after lunch!
All the best

@johannesschobel
Copy link
Contributor Author

johannesschobel commented Nov 13, 2018

i just tried your await onModuleInit() approach and this really (!) looks nice..

However, i was wondering...
In this case, i would need to add this code to all of my modules:

   constructor(public configService: ConfigService) {}

  async onModuleInit() {
    await this.configService.merge(path.resolve(__dirname, 'config', '*.{ts,js}'));
  }

So i was wondering, if i could write a BaseModule class that provides these methods and just do something similar like this:

export abstract class BaseModule implements OnModuleInit { 
   constructor(public configService: ConfigService) {}

  async onModuleInit() {
    await this.configService.merge(path.resolve(__dirname, 'config', '*.{ts,js}'));
  }
}

export class SnakeModule extends BaseModule {}

this does not work, because the BaseModule does not know the ConfigModule (and, therefore, has no access to the ConfigService to be injected)..

@johannesschobel
Copy link
Contributor Author

Anyway.. here is my current solution (that works):

AppModule

@Module({
  imports: [
    ContainerLoaderModule,
    // ... 
  ],
  controllers: [AppController],
  providers: [AppService],
})
export class AppModule implements NestModule {
  // ...
}

ContainerLoaderModule

@Module({
  imports: [
    ConfigModule.load(path.join(PathHelper.configPath(), '*.{ts,js}')), // this loads the /src/config folder
  ],
  controllers: [],
  providers: [ConfigLoaderService],
  exports: [],
})
export class ContainerLoaderModule {}

ConfigLoaderService

@Injectable()
export class ConfigLoaderService {

  private static readonly filePattern = '*.{ts,js}';
  private readonly configService: ConfigService;

  constructor(configService: ConfigService) {
    this.configService = configService;
  }

  async loadContainerConfiguration(container: string) {
    const containerConfigPath = path.join(PathHelper.containerPath(container), 'config', ConfigLoaderService.filePattern);
    await this.configService.merge(containerConfigPath);
  }
}

I am certainly aware, that this is not the most elegant way of dealing with this issue.. but it was the best one i could come up for now ;)

Thought i could share this here..
All the best

@bashleigh
Copy link
Collaborator

strange that the abstract method doesn't work because it can't find the ConfigService? I'll pay around with it when I get home. And see what's going on with it

@bashleigh bashleigh added the question Further information is requested label Nov 13, 2018
@johannesschobel
Copy link
Contributor Author

johannesschobel commented Nov 13, 2018

gimme a second.. i think i managed to get it to work :D

Update
yeah, it works.. will post my solution in a few minutes ;)

@johannesschobel
Copy link
Contributor Author

johannesschobel commented Nov 13, 2018

Ok, my solution (based on the main idea from @bashleigh ) is like this:

CatModule

@Module({
  imports: [],
  controllers: [],
  providers: [CatService],
})
export class CatModule extends BaseModule {
  // this returns the path to the folder where this module is stored
  getModulePath(): string {
    return path.join(PathHelper.appPath(), 'custom', 'path', 'to', 'modules', 'cat');
  }
}

BaseModule

@Module({
  imports: [ConfigModule],
  controllers: [],
  providers: [],
})
export abstract class BaseModule implements OnModuleInit {
  // inject the ConfigService from the ConfigModule package
  protected constructor(protected configService: ConfigService) {}

  async onModuleInit() {
    await this.configService.merge(path.resolve(this.getModulePath(), 'config', '*.{ts,js}'));
  }

  // define this method here as abstract so every module **must** implement it 
  abstract getModulePath(): string;
}

What do you think of this approach here?!
You were a really (!!!) really big help in this issue! Thank you very much for the time and effort put into this issue and helping me out! 👍

All the best

@bashleigh
Copy link
Collaborator

bashleigh commented Nov 13, 2018

yea I like that :) I think that's a good approach! You could even use a property instead of a function. Forgive me if I'm wrong but I think this'll work.

export abstract class BaseModule implements OnModuleInit {
  // inject the ConfigService from the ConfigModule package

  protected globPath = path.resolve(__dirname, 'config', '*.{ts,js}');

  protected constructor(protected configService: ConfigService) {}

  async onModuleInit() {
    await this.configService.merge(this.globPath);
  }
}

export class CatModule extends BaseModule {
  protected globPath = path.resolve(__dirname, 'config-override', '*.{ts,js}'));
}

This way you have a default ;) But yea looks pretty good :) and was no problem at all! Sorry if I've been a bit slow at replying! Let me know if there's anything else I can help you with or if you'd like to join our slack channel :)

@johannesschobel
Copy link
Contributor Author

Hey there,

you were a really big help - thanks for pointing the thing with the onModuleInit() out - i did not know that method (and interface!) - really cool!

I think, i like the method more than the property approach, because this actually forces you to implement the method. But i will think about it during the next days when toying around with this approach a bit more!

I really really like this package - great work and very easy to use; once you know how to deal with such "non-standard" use-cases like i described! haha 😆

All the best!

PS: Maybe you can send me a link to your slack channel?

@bashleigh
Copy link
Collaborator

try this: https://join.slack.com/t/nestjscommunity/shared_invite/enQtNDgwMzI0OTg1MzM1LWExNTg4YWY5Mjg0YmVhYzgxYzY2MWMwOTkyMDMyYmUyOTc0NzdmNjk5ZTQ2MmI4Nzk3MDc5OGUyNzI4NmJmYTI

It expires is 30 days apparently?

No problem :) Glad I could help. Going to close the issue now.

@eugenchio
Copy link

@bashleigh , Hi and first of all thank you for this great package and such an amazing support! I'm trying to extend a bit the solution suggested in this issue, having the configuration files named identically (e.g. modules-config.ts), so finally I can access a merged configuration of all modules with e.g. this.config.get('modules-config'). But it seems, that if there are few files with the same name - one overrides another. Is it somehow possible to merge configuration from different files under a specific key with this package?

@johannesschobel
Copy link
Contributor Author

@eugenchio , hey there, ...
AFAIK, this is not possible.. the package uses the filename as the "first" dot-part of the name of the config key.. For example, if your file is structured as follows:

// database.ts
export default = {
   host: 'localhost',
   port: 12345,
   // additional config values
};

you would access the config values like follows:

const dbHost = configService.get('database.host'); // 'localhost'

If you have multiple files with the same name, the values are overwritten. The order, thereby, is defined by the order the files are imported.. This may be not deterministic because files may be read async..

I would go for different file-names ;)

@eugenchio
Copy link

@johannesschobel , thanks for the reply. Yeah, the way to access the configs is clear. Different file names would work if I wouldn't suppose to be independent of the exact sub-modules which I'm collecting the configs from. Will think on a different solution. Will post it there if come to something "elegant" in case if someone googles the same question.

@bashleigh
Copy link
Collaborator

bashleigh commented Jan 28, 2019

@eugenchio There is now a 'modifyConfigName' options that you can pass in.

 @Module({
  imports: [
    ConfigModule.load(path.resolve(__dirname, '*-config.ts'), {
        modifyConfigName: name => name.replace('-config', ''),
    }),
  ],
})

I've noticed this isn't actually in the readme 👎 so I'll add an issue for that now

@johannesschobel
Copy link
Contributor Author

I mean, you could also do it like this:

// users/module-config.ts
export default = {
   user: {
      foo: 'bar',
      whatever: true,
      anotherKey: 1234,
   }
}

and

// projects/module-config.ts
export default = {
   project: {
      foo: true,
      whatever: 'blabla',
      anotherKey: 1000,
   }
}

and access the data like this: configService.get('module-config.project.foo');

What do you think about this "solution"?

@johannesschobel
Copy link
Contributor Author

@bashleigh oh wow, that is actually quite nice! ;) good to know!

@bashleigh
Copy link
Collaborator

The modifyConfigName method also should resolve your renaming/redeclaring of the config https://github.com/nestjs-community/nestjs-config/blob/master/src/module/config.service.ts#L264-L268

@eugenchio
Copy link

@bashleigh , thanks a lot for the info. Will try that immediately!
@johannesschobel , That's not really what I'm trying to fulfill. The idea behind my question is to have one module as a "collector" or "manager" of modules of a specific "type", which will provide the list of the modules, which are registered in the app. Let's say, it is payment-types-manager. So, I want to register payment-types in the app and create a config for each payment type module. Then ask payment-types-manager for the list of configuration of all payment-types. But I don't want to put the manager into dependency on any specific payment type, but to keep it abstract. If you're interested to discuss it further I can create a small code example if my explanation is not fully clear :)

@eugenchio
Copy link

@bashleigh , why not to use "merge()" instead of flat assignment? https://github.com/nestjs-community/nestjs-config/blob/master/src/module/config.service.ts#L268

@bashleigh
Copy link
Collaborator

@eugenchio hmmm suppose we could. I was thinking of making the configs immutable but I'm not sure others would agree; especially since someone added a 'set' method

@eugenchio
Copy link

@bashleigh Hope to get this feature someday :)

@bashleigh
Copy link
Collaborator

bashleigh commented Jan 28, 2019

I've opened #57 for the merge/immutable. I've also opened a PR for modifyConfigName method to be added to the readme https://github.com/nestjs-community/nestjs-config/tree/feature/modify-name-readme#multi-modular-config-usage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

3 participants