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

Add or document support for Webpack HMR #91

Open
2 of 4 tasks
dhaspden opened this issue Apr 13, 2019 · 12 comments
Open
2 of 4 tasks

Add or document support for Webpack HMR #91

dhaspden opened this issue Apr 13, 2019 · 12 comments
Labels
bug Something isn't working

Comments

@dhaspden
Copy link

dhaspden commented Apr 13, 2019

Issue type:

  • question
  • bug report
  • feature request
  • documentation issue

nestjs-config version

1.3.21

@nestjs/common+core or other package versions

  • @nestjs/common: 6.1.0
  • @nestjs/core: 6.1.0

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 bashleigh added the bug Something isn't working label Apr 15, 2019
@bashleigh
Copy link
Collaborator

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
Copy link
Author

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
Copy link
Collaborator

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
Copy link

OLDIN commented Jun 8, 2019

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
Copy link

marktran commented Jun 8, 2019

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
Copy link

OLDIN commented Jun 8, 2019

@marktran, can you share your configuration file?

@marktran
Copy link

marktran commented Jun 8, 2019

@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
Copy link
Collaborator

bashleigh commented Jun 17, 2019

@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
Copy link

@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
Copy link

blue-cp commented Jul 1, 2019

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

@lsurma
Copy link

lsurma commented Aug 25, 2019

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
Copy link

@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

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

No branches or pull requests

7 participants