chore(configuration): Remove environment variable substitution (#1942)

BREAKING CHANGE: Falls back to node-config instead of adding additional
functionality like path replacements and automatic environment variable insertion.

Author: daffl

packages/configuration/package.json

@@ -34,7 +34,7 @@
     "prepublish": "npm run compile",
     "compile": "shx rm -rf lib/ && tsc",
     "test": "npm run compile && npm run mocha",
-    "mocha": "mocha --config ../../.mocharc.ts.json --recursive test/**.test.ts test/**/*.test.ts"
+    "mocha": "NODE_CONFIG_DIR=./test/config mocha --config ../../.mocharc.ts.json --recursive test/**.test.ts test/**/*.test.ts"
   },
   "semistandard": {
     "env": [

packages/configuration/src/index.ts

@@ -1,62 +1,24 @@
 import { Application } from '@feathersjs/feathers';
 import Debug from 'debug';
-import path from 'path';
 import config from 'config';
 
 const debug = Debug('@feathersjs/configuration');
-const separator = path.sep;
 
 export default function init () {
   return (app?: Application) => {
-    const convert = (current: any) => {
-      const result: { [key: string]: any } = Array.isArray(current) ? [] : {};
-
-      Object.keys(current).forEach(name => {
-        let value = current[name];
-
-        if (typeof value === 'object' && value !== null) {
-          value = convert(value);
-        }
-
-        if (typeof value === 'string') {
-          if (value.indexOf('\\') === 0) {
-            value = value.replace('\\', '');
-          } else {
-            if (process.env[value]) {
-              value = process.env[value];
-            }
-            if (value.indexOf('.') === 0 || value.indexOf('..') === 0) {
-              // Make relative paths absolute
-              value = path.resolve(
-                path.join(config.util.getEnv('NODE_CONFIG_DIR')),
-                value.replace(/\//g, separator)
-              );
-            }
-          }
-        }
-
-        result[name] = value;
-      });
-
-      return result;
-    };
-
-    const env = config.util.getEnv('NODE_ENV');
-    const conf = convert(config);
-
     if (!app) {
-      return conf;
+      return config;
     }
 
-    debug(`Initializing configuration for ${env} environment`);
+    debug(`Initializing configuration for ${config.util.getEnv('NODE_ENV')} environment`);
 
-    Object.keys(conf).forEach(name => {
-      const value = conf[name];
+    Object.keys(config).forEach(name => {
+      const value = (config as any)[name];
       debug(`Setting ${name} configuration value to`, value);
       app!.set(name, value);
     });
 
-    return conf;
+    return config;
   };
 }
 

packages/configuration/test/config/custom-environment-variables.json

@@ -1,11 +1,5 @@
 {
   "port": 3030,
-  "environment": "NODE_ENV",
-  "path": "../something",
-  "pathFromEnv": "PATH_ENV",
-  "unescaped": "\\NODE_ENV",
-  "from": "default",
-  "deeply": { "nested": { "env": "NODE_ENV" } },
   "array": ["one", "two", "three"],
   "deep": { "base": false },
   "nullish": null

packages/configuration/test/config/default.json

@@ -1,91 +1,27 @@
-import assert from 'assert';
-import { join } from 'path';
+import { strict as assert } from 'assert';
 import feathers, { Application } from '@feathersjs/feathers';
+import plugin from '../src';
 
 describe('@feathersjs/configuration', () => {
-  const originalEnv: { [key: string]: any } = {};
-  let app: Application;
-  let plugin: any;
+  const app: Application = feathers().configure(plugin());
 
-  before(() => {
-    originalEnv.NODE_ENV = process.env.NODE_ENV;
-    originalEnv.NODE_CONFIG_DIR = process.env.NODE_CONFIG_DIR;
-
-    process.env.NODE_ENV = 'testing';
-    process.env.NODE_CONFIG_DIR = join(__dirname, 'config');
-    process.env.PATH_ENV = '../something';
-
-    plugin = require('../lib');
-    app = feathers().configure(plugin());
+  it('exports default', () => {
+    assert.ok(typeof require('../lib') === 'function');
   });
 
-  after(() => {
-    process.env.NODE_ENV = originalEnv.NODE_ENV;
-    process.env.NODE_CONFIG_DIR = originalEnv.NODE_CONFIG_DIR;
+  it('initialized app with default.json', () => {
+    assert.equal(app.get('port'), 3030);
+    assert.deepEqual(app.get('array'), [
+      'one', 'two', 'three'
+    ]);
+    assert.deepEqual(app.get('deep'), { base: false });
+    assert.deepEqual(app.get('nullish'), null);
   });
 
-  it('exports default', () =>
-    assert.strictEqual(plugin, plugin.default)
-  );
-
-  it('initialized app with default data', () =>
-    assert.strictEqual(app.get('port'), 3030)
-  );
-
-  it('initialized with <env>', () =>
-    assert.strictEqual(app.get('from'), 'testing')
-  );
-
-  it('initialized with <env> derived data module', () =>
-    assert.strictEqual(app.get('derived'), 'Hello World')
-  );
-
-  it('initialized property with environment variable', () =>
-    assert.strictEqual(app.get('environment'), 'testing')
-  );
-
-  it('initialized property with environment variable from <env>', () =>
-    assert.strictEqual(app.get('testEnvironment'), 'testing')
-  );
-
-  it('initialized property with derived environment variable from <env> module', () =>
-    assert.strictEqual(app.get('derivedEnvironment'), 'testing')
-  );
-
-  it('uses an escape character', () =>
-    assert.strictEqual(app.get('unescaped'), 'NODE_ENV')
-  );
-
-  it('normalizes relative path names', () =>
-    assert.strictEqual(app.get('path'), join(__dirname, 'something'))
-  );
-
-  it('normalizes relative path names from environment variable', () =>
-    assert.strictEqual(app.get('pathFromEnv'), join(__dirname, 'something'))
-  );
-
-  it('converts environment variables recursively', () =>
-    assert.strictEqual(app.get('deeply').nested.env, 'testing')
-  );
-
-  it('converts arrays as actual arrays', () =>
-    assert.ok(Array.isArray(app.get('array')))
-  );
-
   it('works when called directly', () => {
     const fn = plugin();
+    const conf = fn() as any;
 
-    assert.strictEqual(fn().port, 3030);
-  });
-
-  it('deep merges properties', () =>
-    assert.deepStrictEqual(app.get('deep'), {
-      base: false,
-      merge: true
-    })
-  );
-
-  it('supports null value', () => {
-    assert.strictEqual(app.get('nullish'), null);
+    assert.strictEqual(conf.port, 3030);
   });
 });