nestjsx / nestjs-config

Config module for nestjs using dotenv :key:
MIT License
698 stars 44 forks source link

Add or document support for Webpack HMR #91

Open dhaspden opened 5 years ago

dhaspden commented 5 years ago

Issue type:

nestjs-config version

1.3.21

@nestjs/common+core or other package versions

Excepted behavior

Actual behavior or outcome (for issue)

Library worked great for me on a different project. I want to use this library with a new project, but this time I am trying to use the Webpack HMR capabilities to make the app reload a lot faster. Since Nest ships with this out-of-the-box would it be nice to have some kind of recipe for supporting this flow?

I'm not familiar with the internals of this library so not sure if it's even possible to be honest. It would be really handy to be able to use this library with HMR though!

Replication/Example

ConfigModule.resolveRootPath(__dirname).load('config/**/!(*.d).{ts,js}')

Works using yarn start with ts-node. Doesn't work with yarn start:hmr. The module initializes but no configuration is being read. Likely because all of the configuration is being appended into the server.js and is no longer in a config folder.

bashleigh commented 5 years ago

Hmm this is interesting. Personally I'm always using containers so I just mount an env file and run everything there. I've never really used hmr or webpack package commands.

Since writing the above paragraph I install a new nest v6.1.1 application, I checked package.json for start:hmr and webpack and I think it's since been removed? In an older version hmr looked to be using a dist/service file/folder? Are you still using this functionality with v6? If so do you have an example I can look at and see why it doesn't load configs?

Thanks! :)

dhaspden commented 5 years ago

Okay so I played around with this a bit more. Apparently I have an old version of the cli installed and at some point in the last few months Nest removed Webpack from it's typescript-starter repo you get when you run the nest new command.

I wouldn't call this a bug per-se since Nest seems to have abandoned using the HMR (I personally thought it was pretty neat compared to using ts-node.) That being said if you want to investigate anyway all you really need to do is clone version 5 of the starter here. Once that's cloned up just bootstrap an MVP nestjs-configsetup and you'll immediately see that it doesn't work with Webpack.

So not sure the internals of this library but it seems like since everything gets compiled into a dist/server.js as opposed to all of the .ts files being compiled and copied over separately. The problem then probably stems from the fact that since we dynamically require the .ts files Webpack probably doesn't pick them up and bundle them into the server.js.

I tried messing around with it to compile and copy the config folder over but didn't have any luck. It's definitely an interesting problem though because I personally thought the HMR was pretty useful.

Hopefully that gives you some detailed info, like I said it's probably not something you need to worry about but I think it would be cool if it worked :)

bashleigh commented 5 years ago

oh I see! So it's more about the file type than the .env file. I was presuming webpack loaded it's own dotenv package and was loading from a different dir or something like that but it sounds like this is more file type related.

I will try and investigate. My time is rather limited at the moment though as I'm working 2 jobs and still got to release V2! If I can find some time to investigate this in the next month or 2 I will. I'll try and get V2 published first though! After that I'll defo have a look into it!

OLDIN commented 5 years ago

Your library is excellent. And I do not want to give it up. But this library has a problem with the Webpack HMR capabilities. When can we expect a bug fix?

marktran commented 5 years ago

I looked into this and it doesn't appear to be possible to use Webpack HMR with this package. nestjs-config tries to load config files at runtime after Webpack has transpiled and bundled everything into a single file.

I ended up rolling my own ConfigService based on https://docs.nestjs.com/techniques/configuration so everything would work with HMR.

OLDIN commented 5 years ago

@marktran, can you share your configuration file?

marktran commented 5 years ago

@OLDIN

import path from 'path';
import { Module, Global } from '@nestjs/common';
import { ConfigService } from './config.service';

import { ROOT_PATH } from '../constants';
const NODE_ENV = process.env.NODE_ENV || 'development';

@Global()
@Module({
  providers: [
    {
      provide: ConfigService,
      useValue: new ConfigService(
        path.resolve(ROOT_PATH, '..', `.env.${NODE_ENV}`),
      ),
    },
  ],
  exports: [ConfigService],
})
export class ConfigModule {}
import _ from 'lodash';
import dotenv from 'dotenv';
import fs from 'fs';
import joi from '@hapi/joi';

import { RedisModuleOptions } from '../redis/redis.interface';
import { TypeOrmModuleOptions } from '@nestjs/typeorm';

import { databaseOptions } from './database.config';
import { redisOptions } from './redis.config';
import { graphQLOptions } from './graphql.config';
import { GqlModuleOptions } from '@nestjs/graphql';

export interface EnvConfig {
  [key: string]: string;
}

export class ConfigService {
  private readonly envConfig: EnvConfig;

  constructor(filePath: string) {
    let config: EnvConfig;

    if (process.env.DATABASE_URL) {
      config = process.env;
    } else {
      config = dotenv.parse(fs.readFileSync(filePath));
    }

    this.envConfig = this.validateInput(config);
  }

  public get(name: string): string {
    return this.envConfig[name];
  }

  getDatabaseOptions(): TypeOrmModuleOptions {
    return databaseOptions(
      this.envConfig.NODE_ENV,
      this.envConfig.DATABASE_URL,
      this.envConfig.TYPEORM_DROP_SCHEMA,
      this.envConfig.TYPEORM_LOGGING,
    );
  }

  getRootDomain(): string {
    const parts = this.get('HOSTNAME').split('.');
    return _.takeRight(parts, 2).join('.');
  }

  getRedisOptions(): RedisModuleOptions {
    return redisOptions(this.envConfig.NODE_ENV, this.envConfig.REDIS_URL);
  }

  getGraphQLOptions(): GqlModuleOptions {
    return graphQLOptions(
      this.envConfig.NODE_ENV,
      this.envConfig.FRONTEND_HOSTNAME,
    );
  }

  private validateInput(envConfig: EnvConfig): EnvConfig {
    const envVarsSchema: joi.ObjectSchema = joi.object({
      AIRTABLE_API_KEY: joi.string().required(),
      AIRTABLE_SALES_CRM_BASE_ID: joi.string().required(),
      DATABASE_URL: joi.string().required(),
      FRONTEND_HOSTNAME: joi.string().required(),
      GOOGLE_AUTH_CLIENT_ID: joi.string().required(),
      GOOGLE_AUTH_CLIENT_SECRET: joi.string().required(),
      HOSTNAME: joi.string().required(),
      NODE_ENV: joi
        .string()
        .valid(['development', 'test', 'production'])
        .default('development'),
      PORT: joi.number().default(5000),
      README_JWT_SECRET: joi.string().required(),
      REDIS_URL: joi.string().required(),
      SENTRY_DSN: joi.string().allow(''),
      SENTRY_ENV: joi.string().allow(''),
      SESSION_SECRET: joi.string().required(),
      TYPEORM_DROP_SCHEMA: joi.boolean().default(false),
      TYPEORM_LOGGING: joi.boolean().default(false),
    });

    const { error, value: validatedEnvConfig } = joi.validate(
      envConfig,
      envVarsSchema,
      { stripUnknown: true },
    );

    if (error) {
      throw new Error(`Config validation error: ${error.message}`);
    }

    return validatedEnvConfig;
  }
}
bashleigh commented 5 years ago

@marktran @OLDIN what version of nest are you using? Just curious to know if you're using v6+ as HMR has been removed apparently? (not sure if that's still the case). Looking to add validation into nestjs-config V2 but I don't want to only use a specific package like joi as someone else might want to use something different. How would you like to use the validation? Could implement decorators but not sure if that's a good idea, would be nice or whatever.

As for the loading of configs though, yea I only built this for prod envs really, at the time. It would be possible to implement HMR support, because the ConfigService builds this statically it might be an idea to rebuild at some point. I think the issue is that the nestjs-config package, like you said, loads the files at runtime and therefore doesn't change it's dependants. I'll have a think about the best way to combat this. Currently in V2 I have implemented everything for async providers. This obviously doesn't fix the issue as these are still created at runtime and not rebuilt.

I myself haven't used HMR, I'll try and find some time to play with it and try out the config package with it and see if I can find an easy "for now" solution.

further to validation, it's also possible in V2 to use interfaces and classes for your config.

import {TypeOrmOptions} from 'typeorm';

export default class DatabaseConfig implements TypeOrmOptions {
    public type = 'mysql';
    public host: string = process.env.TYPEORM_HOST;
    public username: string = process.env.TYPEORM_USERNAME;
    public password: string = process.env.TYPEORM_PASSWORD;
    public database: string = process.env.TYPEORM_DATABASE;
    public logging: boolean = process.env.TYPEORM_LOGGING === 'true';
    public sync: boolean = process.env.TYPEORM_SYNCHRONIZE === 'true';
    public entities: string[] = process.env.TYPEORM_ENTITIES.split(',');
    public port: number = parseInt(process.env.TYPEORM_PORT);
};
marktran commented 5 years ago

@bashleigh I'm on the latest version of Nest. Not sure what HMR integration looked like in previous versions but it must have been pretty minimal (see: https://docs.nestjs.com/techniques/hot-reload).

blue-cp commented 5 years ago

Having exact same issue here. It is unable to find config. Using version nestjs-config 1.4.0

lsurma commented 5 years ago

Hi,

I think this issue is related to dynamic path which are used to load config files.

ConfigModule.load(path.resolve(__dirname, 'config', '**/!(*.d).{ts,js}')), Because of that, webpack doesn't know that file in config dir are required, and those are not merged/copied into webpack bundles in "dist" dir, which later on is used for hot reloading etc.

Possible way for overcome this is use of absolute path to config dir, for e.g. ConfigModule.load(path.resolve("/var/www/project-dir/src/config/**/!(*.d).{ts,js}"))

Other way (im not sure, not tested) is to use webpack copy plugin, to copy config dir into correct location in "dist"

Hope it helps you a little bit, @dhaspden @bashleigh @OLDIN @marktran @abhishekdubey1

HardlyMirage commented 5 years ago

@Isurm

Hi,

I think this issue is related to dynamic path which are used to load config files.

ConfigModule.load(path.resolve(__dirname, 'config', '**/!(*.d).{ts,js}')), Because of that, webpack doesn't know that file in config dir are required, and those are not merged/copied into webpack bundles in "dist" dir, which later on is used for hot reloading etc.

Possible way for overcome this is use of absolute path to config dir, for e.g. ConfigModule.load(path.resolve("/var/www/project-dir/src/config/**/!(*.d).{ts,js}"))

Other way (im not sure, not tested) is to use webpack copy plugin, to copy config dir into correct location in "dist"

Hope it helps you a little bit, @dhaspden @bashleigh @OLDIN @marktran @abhishekdubey1

No, the issue is that webpack compresses the entire project into a single js file so you no longer have config/*.js files to load into ConfigModule