seanpmaxwell / overnight

TypeScript decorators for the ExpressJS Server.
MIT License
878 stars 40 forks source link

Loading the dotenv variables doesn't seem to work (probably doing it wrong?) #17

Closed sebastiangug closed 5 years ago

sebastiangug commented 5 years ago

I'm trying to load my env variables with dotenv, -- just converting a working api project to Overnightjs;

However I keep getting a bunch of errors with clientID's missing and whatnot due to the dotenv file not being processed.

So far I've tried it:

None of them have worked so far so I'm a bit confused where its supposed to go.

Thanks & great package, I think once I finish the transfer I'll never be able to go back to writing typescript+express without overnight. I've tried nest 2-3 times but gave up due to how many things needed to be changed/how complex it was. The cons have outweighed the pros but with overnight most things are quite straightforward and the benefits are plenty, especially at scale!

seanpmaxwell commented 5 years ago

Loading the environment variables at the start of start.ts should work. There is one quirk to dotenv though. It's path is relative to wherever 'node' is called from. So if you are starting node (or ts-node) at the root of your app with 'npm start' or whatever, that path to your environment files that's passed to dotenv need to be in relation to the root. Example:

import * as dotenv from 'dotenv';
import Server from './Server';

const nodeEnv = process.argv[2] || 'development';

const result2 = dotenv.config({path: `./env/${nodeEnv}.env`}); <--- ./env/ is at the root of the project
if (result2.error) {
    throw result2.error;
}

const server = new Server();
server.start(Number(process.env.PORT || 3000));

I'm actually working on npm library right now which can do all this automatically https://github.com/seanpmaxwell/express-generator-typescript

sebastiangug commented 5 years ago

Thanks for this! I kept at it and tried to eliminate as many possibilities and it turns out the environmental variables DO get loaded but because of the way I'm trying to initialize passport -- the passport 'middleware' throws out some errors, Now I think this is the part I'm doing wrong:

.......
  this.app.use(bodyParser.urlencoded({ extended: true }));
        // Authenticating passport
        this.app.use(passport.initialize());
        // ACCESS TOKEN STRATEGY
        console.log(this.opts1);
        passport.use(
            'jwt-1',
            new passportjwt.Strategy(
                this.opts1,
                (
                    jwt_payload: passportjwt.JwtFromRequestFunction,
                    done: passportjwt.VerifiedCallback
                ) => {
                    return done(null, jwt_payload);
                }
            )
        );
....

So this is happening in the constructor block -- and it throws out errors regarding any of the keys, if I disable the 1'st strategy (comment it out) it will throw error on the key in the 2n'd strategy, or in the clientID of the google strategy.

But if I disable them all, everything works and I can see all env variables on the console and I can even connect to the DB via an environment variable, so the env variables doesn't seem to be the issue

EDIT: the result of the console.log is the right opts configuration except It's showing 'undefined' when it comes to the environment variable (jwt key) which is weird :-? as this is a project that was already working and I hadn't touched the env variables at all

sebastiangug commented 5 years ago

Ok sorry for double-posting but I just realized what's happening after more investigation. It seems that any place I declared a const or something static, the process.env.something will resolve as 'undefined' even if it's not using them.

If in the constructor after I configure dotenv I start filling in variables with env.variables again -- it will work.

So I have to configure dotenv somewhere where it's the very first thing running;

Which means even if I'm configuring dotenv before I'm calling server.start() -- it won't work, I have to configure dotenv before the import statement.

If I import & configure that on the first two lines everything seems to work and it's all peachy. Thanks & great work once again!

seanpmaxwell commented 5 years ago

Hmm that's strange. Configuring dotenv works for me just fine after importing Server. I just make sure to call 'new Server()' after configuring dotenv.

sebastiangug commented 5 years ago

I had the same code going on -- it seems that variables are not loaded/calculated for stuff that's not affected by the constructor.

Might also be some issues with my overall project configuration :-?? but in the end it works and for production purposes it doesn't really matter as env variables are configured manually

seanpmaxwell commented 5 years ago

Hey man you were right. I just had a similar issue trying to load an environment variable from a nested controller. And that's where the problem is. Because nested controllers are initialized in the @Children() decorator as soon as the parent controller's script is called, not as soon as the server's constructor is called. And that's why all the environment variables have to be loaded before the server script is imported. Thanks for posting the issue. I'll add a note to the readme about this. Here's an updated start script:

// Setup the environment variables
import * as dotenv from 'dotenv';

const nodeEnv = process.argv[2] || 'development';
const envFilePath = `./env/${nodeEnv}.env`;

const result2 = dotenv.config({path: envFilePath});
if (result2.error) {
    throw result2.error;
}

// Start the server, server must be imported
// after loading the environment variables
import Server from './Server';
const isDevMode = (nodeEnv === 'development');

const server = new Server(isDevMode);
server.start(Number(process.env.PORT || 3000));