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

NODE_CONFIG_DIR not working with gulp / ES6 (doc-patch welcome) #246

Open
xmlking opened this issue Sep 12, 2015 · 14 comments
Open

NODE_CONFIG_DIR not working with gulp / ES6 (doc-patch welcome) #246

xmlking opened this issue Sep 12, 2015 · 14 comments

Comments

@xmlking
Copy link

xmlking commented Sep 12, 2015

I am setting NODE_CONFIG_DIR in the beginning of my gulpfile.babel.js file, but it seams executed after import files are loaded. please advice any workaround.

it would nice if we can pass settings like:

let  config = requires('config')({NODE_CONFIG_DIR:'gulp/config'})
process.env.NODE_CONFIG_DIR = 'gulp/config';
console.log(process.env.NODE_CONFIG_DIR);

//import config from 'config';
let  config = requires('config');
console.log('NODE_CONFIG_DIR: ' + config.util.getEnv('NODE_CONFIG_DIR'));

import './gulp/server.js';
import './gulp/typescript.js';
import './gulp/sass.js';
import './gulp/images.js';
import './gulp/offline.js';
import './gulp/build.js';
import './gulp/gpdeploy.js';

Error:

WARNING: No configurations found in configuration directory:
WARNING: /Developer/Work/ng-starter-kit/config
WARNING: See https://www.npmjs.org/package/config for more information.
/Developer/Work/ng-starter-kit/node_modules/config/lib/config.js:177
    throw new Error('Configuration property "' + property + '" is not defined');
    ^

Error: Configuration property "apiProxy" is not defined
    at [object Object].Config.get (/Developer/Work/ng-starter-kit/node_modules/config/lib/config.js:177:11)
    at Object.<anonymous> (/Developer/Work/ng-starter-kit/gulp/server.js:14:31)
    at Module._compile (module.js:434:26)
    at normalLoader (/Developer/Work/ng-starter-kit/node_modules/babel-core/lib/api/register/node.js:199:5)
    at Object.require.extensions.(anonymous function) [as .js] (/Developer/Work/ng-starter-kit/node_modules/babel-core/lib/api/register/node.js:216:7)
@markstos
Copy link
Collaborator

If you believe you've found a bug in node-config, please add a test to the test suite which illustrates it.

I'm guessing something about transpilation of ES6 to ES5 is the issue here. For example, the use of 'requires may mean something different than singular require keyword in ES5.

If you have test which illustrates a bug in node-config, please re-open this issue.

@lorenwest
Copy link
Collaborator

Something is wrong with your error output. You explicitly say

process.env.NODE_CONFIG_DIR = 'gulp/config';
console.log(process.env.NODE_CONFIG_DIR);

But your output doesn't show that log statement, which is a clue. I suspect there is another requires('config') earlier in the execution, because based on this line:

WARNING: /Developer/Work/ng-starter-kit/config

it shows that process.env.NODE_CONFIG_DIR hasn't been set to 'gulp/config'. I have a hard time imagining this as a bug in ES5 node-config.

Try setting the NODE_CONFIG_DIR environment variable as the FIRST LINE in your executable, or in your environment before running node.js.

Or, consider putting configurations in the default place. If you're thinking about distributing your app, that's where people will be looking for it.

@lorenwest
Copy link
Collaborator

As to the suggestion to add arguments to node-config:

let  config = requires('config')({NODE_CONFIG_DIR:'gulp/config'})

Unfortunately if we added the ability to specify overrides, they would have to be consistently applied in every module that used let config = requires('config'), including sub-modules that know nothing about the application from which they're running.

@xmlking
Copy link
Author

xmlking commented Sep 12, 2015

thanks @lorenwest and @markstos for looking into this issue.
@markstos sorry requires was a typo, but problem remains same.

looks like ES6 module imports are hoisted (meaning all the dependent modules will be loaded before running any code.). spec

we can see this with simple example.
working example ( running : babel-node test.js)

process.env.NODE_CONFIG_DIR = 'gulp/config';
console.log(process.env.NODE_CONFIG_DIR);

//import config from 'config';
let  config = require('config');
console.log('NODE_CONFIG_DIR: ' + config.util.getEnv('NODE_CONFIG_DIR'));

output

gulp/config
NODE_CONFIG_DIR: gulp/config

failing example

process.env.NODE_CONFIG_DIR = 'gulp/config';
console.log(process.env.NODE_CONFIG_DIR);

import config from 'config';
//let  config = require('config');
console.log('NODE_CONFIG_DIR: ' + config.util.getEnv('NODE_CONFIG_DIR'));

output

WARNING: No configurations found in configuration directory:
WARNING: /Developer/Work/ng-starter-kit/config
WARNING: See https://www.npmjs.org/package/config for more information.
gulp/config
NODE_CONFIG_DIR: /Developer/Work/ng-starter-kit/config

As developers are moving to ES6 , this will limit the usability of this module with ES6.

it would be nice if we re open this issue and let the community aware of this limitation.

@lorenwest
Copy link
Collaborator

Thank you for discovering this. It's an interesting difference with ES6, and I'm sure it's going to be the source of many headaches during the transition.

It sounds like if you want to use environment variables in node-config, you actually have to use environment variables.

Another option may be to store your environment variable declarations in a module, and to load that module before loading node-config.

Or you could store your configurations in the default location, and circumvent this issue all together.

@xmlking
Copy link
Author

xmlking commented Sep 15, 2015

I am defined those environment variables in globals.js and referring it in the beginning of the main program as you mentioned. For those who are interested in this solution , here is a reference project: https://github.com/xmlking/gulp-ng-recipes
I am using node-config in two of my recent projects and happy with it :)

@lorenwest
Copy link
Collaborator

Glad to hear you've gotten it working!

@markstos
Copy link
Collaborator

I suspect we'll see more of the global.js design pattern with ES6. I see it mentioned here as well. Seems like a clear enough solution.

@markstos
Copy link
Collaborator

I'm re-opening this issue as a reminder that our README or a wiki page should be updated to document ES6 support gotchas and syntax.

I'm also curious to test if we can still force a reload with ES6-- currently you can delete config from the module cache, change a environment variable and then require it again to have it load a different configuration based on the change environment.

This doesn't normally come up, but we use the pattern internally in the node-config test suite, and I also use it in a private test suite to run through all the configurations we have and reality-check that they can be loaded.

... continuing to think out load: That's another pattern that might be helpful to provide an example for: A sample test that can be added to an external test suite that tests all the config files. Perhaps we could even provide a method to help with that lilke "test all configs". Providing a testing function may make most sense in a sister module like test-config rather than an util function.

@markstos markstos reopened this Sep 15, 2015
@markstos markstos changed the title NODE_CONFIG_DIR not working with gulp / ES6 NODE_CONFIG_DIR not working with gulp / ES6 (doc-patch welcome) Nov 2, 2016
@benedyktdryl
Copy link
Contributor

@markstos what kind of ES6 gotchas we should list here? I can prepare PR if you can provide at least basic list of things needs to be explained (as you probably know node-config better).

@Subterrane
Copy link

This hack worked for me.

in main program:

import { NODE_CONFIG_DIR, SUPPRESS_NO_CONFIG_WARNING } from "./env.js";
import config from "config";

in env.js:

process.env.NODE_CONFIG_DIR = process.cwd();
process.env.SUPPRESS_NO_CONFIG_WARNING = "true";

export const NODE_CONFIG_DIR = process.env.NODE_CONFIG_DIR;
export const SUPPRESS_NO_CONFIG_WARNING = process.env.SUPPRESS_NO_CONFIG_WARNING;

You don't need to do anything with the imported values in the main program, but this makes sure the environment variables get set before config is imported.

@lorenwest
Copy link
Collaborator

That's an excellent solution - thanks

@mountaindude
Copy link

Indeed a neat solution.

Works well in plain Node (I am using Node 20.9.0 right now), but fails when used together with Jest, i.e. for test cases.

Exactly same code that works when doing node index.js fails when used with jest, npm test index.test.js.
From what little I understand it seems that Jest manipulates the execution order.. so that the import of "config" is done before the import of "env.js".

I am still digging through this, figuring there has got to be a way to work around this.
Wanted to share findings so far in case someone else also struggle with this.

Sample code follow.

env.js

const configDir = '/home/goran/code/butler/src/config/';
console.log(`b1 ${configDir}`);

process.env.NODE_CONFIG_DIR = configDir;
process.env.NODE_ENV = 'production';
process.env.SUPPRESS_NO_CONFIG_WARNING = 'false';

export const { NODE_CONFIG_DIR } = process.env;
export const { SUPPRESS_NO_CONFIG_WARNING } = process.env;
export const { NODE_ENV } = process.env;

index.js

/* eslint-disable import/order */
import { NODE_CONFIG_DIR, SUPPRESS_NO_CONFIG_WARNING, NODE_ENV } from './env.js';
import config from 'config';

console.log(`a1 ${NODE_ENV}`);
console.log(`a2 ${NODE_CONFIG_DIR}`);
console.log(`a3 ${SUPPRESS_NO_CONFIG_WARNING}`);

const logLevel = config.get('Butler.logLevel');
console.log(`Log level: ${logLevel}`);

index.test.js

/* eslint-disable import/order */
import { NODE_CONFIG_DIR, SUPPRESS_NO_CONFIG_WARNING, NODE_ENV } from './env.js';
import config from 'config';

console.log(`a1 ${NODE_ENV}`);
console.log(`a2 ${NODE_CONFIG_DIR}`);
console.log(`a3 ${SUPPRESS_NO_CONFIG_WARNING}`);

const logLevel = config.get('Butler.logLevel');
console.log(`Log level: ${logLevel}`);

Executing index.js:

> node index.js 
b1 /home/goran/code/butler/src/config/
a1 production
a2 /home/goran/code/butler/src/config/
a3 false
Log level: info

Executing test case index.test.js indicate that the import of "config" was done before the import of "env.js" (??):.
Btw, the project's package.json file lives in /home/goran/code/butler, so that first warning below suggest that config is looking for its input file in the default location, rather than in the location specified in env.js.

> npm test index.test.js

> butler@9.4.0 test
> node --experimental-vm-modules node_modules/jest/bin/jest.js index.test.js

  console.error
    WARNING: No configurations found in configuration directory:/home/goran/code/butler/config

      at Object.<anonymous> (node_modules/config/lib/config.js:1519:11)

  console.error
    WARNING: To disable this warning set SUPPRESS_NO_CONFIG_WARNING in the environment.

      at Object.<anonymous> (node_modules/config/lib/config.js:1520:11)

  console.log
    b1 /home/goran/code/butler/src/config/

      at src/test/routes/env.js:2:9

  console.log
    a1 production

      at src/test/routes/index.test.js:5:9

  console.log
    a2 /home/goran/code/butler/src/config/

      at src/test/routes/index.test.js:6:9

  console.log
    a3 false

      at src/test/routes/index.test.js:7:9

 FAIL  src/test/routes/index.test.js
  ● Test suite failed to run

    Configuration property "Butler.logLevel" is not defined

       7 | console.log(`a3 ${SUPPRESS_NO_CONFIG_WARNING}`);
       8 |
    >  9 | const logLevel = config.get('Butler.logLevel');
         |                         ^
      10 | console.log(`Log level: ${logLevel}`);
      11 |

      at Config.Object.<anonymous>.Config.get (node_modules/config/lib/config.js:179:11)
      at src/test/routes/index.test.js:9:25

Test Suites: 1 failed, 1 total
Tests:       0 total
Snapshots:   0 total
Time:        0.334 s
Ran all test suites matching /index.test.js/i.
(node:66713) ExperimentalWarning: VM Modules is an experimental feature and might change at any time
(Use `node --trace-warnings ...` to show where the warning was created)

@mountaindude
Copy link

Found a working solution for the scenario of using Jest with config, reading the config data from a custom file/directory.

The previous post does it all, except that the env.js file should be specified in Jest's setupFiles array.
That makes it execute before the test script, setting the env variables that config will look at to determine which file to load settings from.

Can be done in a jest.config.js file, or in the package.json file:

    "scripts": {
        "test": "node --experimental-vm-modules node_modules/jest/bin/jest.js",
    },
    "jest": {
        "transform": {},
        "setupFiles": ["./src/test/env.js"]
    },

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants