Enhances #354 for stricter env method

Author: webketje

lib/index.js

@@ -8,6 +8,10 @@ const { rm, isString, isBoolean, isObject, isNumber, isUndefined, match } = requ
 const utf8 = require('is-utf8')
 const Ware = require('ware')
 
+const symbol = {
+  env: Symbol('env')
+}
+
 /**
  * Metalsmith representation of the files in `metalsmith.source()`.
  * The keys represent the file paths and the values are {@link File} objects
@@ -97,7 +101,7 @@ function Metalsmith(directory) {
   this.metadata({})
   this.source('src')
   this.destination('build')
-  this.env({})
+  this[symbol.env] = Object.create(null)
   this.concurrency(Infinity)
   this.clean(true)
   this.frontmatter(true)
@@ -297,7 +301,7 @@ Metalsmith.prototype.match = function (patterns, input, options) {
 }
 
 /**
- * Get or set one or multiple metalsmith environment variables.
+ * Get or set one or multiple metalsmith environment variables. Metalsmith env vars are case-insensitive.
  * @param {string|Object} [vars] name of the environment variable, or an object with `{ name: 'value' }` pairs
  * @param {string|number|boolean} [value] value of the environment variable
  * @example
@@ -317,29 +321,21 @@ Metalsmith.prototype.match = function (patterns, input, options) {
  * })
  */
 Metalsmith.prototype.env = function (vars, value) {
-  if (!arguments.length) return this._env
-
-  if (arguments.length === 1) {
-    assert(
-      isString(vars) || isObject(vars),
-      'You must pass an environment variable name, or an object with name:value pairs'
-    )
-    if (isString(vars)) {
-      return this._env[vars]
+  if (isString(vars)) {
+    if (arguments.length === 1) {
+      return this[symbol.env][vars.toUpperCase()]
     }
-    if (isObject(vars)) {
-      this._env = Object.assign(this._env || Object.create(null), vars)
+    if (!(isFunction(value) || isObject(value))) {
+      this[symbol.env][vars.toUpperCase()] = value
       return this
     }
+    throw new TypeError('Environment variable values can only be primitive: Number, Boolean, String or null')
   }
-  if (arguments.length === 2) {
-    assert(
-      isString(vars) && (isString(value) || isNumber(value) || isBoolean(value)),
-      'Environment variable values can only be primitives, no objects or functions'
-    )
-    this._env[vars] = value
+  if (isObject(vars)) {
+    Object.entries(vars).forEach(([key, value]) => this.env(key, value))
     return this
   }
+  if (isUndefined(vars)) return Object.assign(Object.create(null), this[symbol.env])
 }
 
 /**

test/index.js

@@ -153,13 +153,30 @@ describe('Metalsmith', function () {
       assert(Object.keys(m.env()).length === 0)
     })
 
+    it('should not be accessible directly from the Metalsmith instance', function () {
+      const m = Metalsmith('test/tmp')
+      assert(!Object.keys(m).includes('env') && !Object.keys(m).includes('_env'))
+    })
+
     it('should allow getting & chainable setting single environment variables', function () {
       const m = Metalsmith('test/tmp').env('DEBUG', true).env('ENV', 'development')
 
       assert(m.env('DEBUG'))
       assert(m.env('ENV') === 'development')
     })
 
+    it('should throw on non-primitive values', function () {
+      assert.throws(() => Metalsmith('test/tmp').env('DEBUG', {}))
+      assert.throws(() => Metalsmith('test/tmp').env('DEBUG', () => {}))
+    })
+
+    it('should treat a lowercase and uppercase variable as the same', function () {
+      const m = Metalsmith('test/tmp')
+      m.env('debug', true)
+      m.env('DEBUG', false)
+      assert(m.env('debug') === m.env('DEBUG'))
+    })
+
     it('should allow setting a batch of environment variables', function () {
       const m = Metalsmith('test/tmp').env({ DEBUG: true, ENV: 'development' })
 
@@ -169,8 +186,11 @@ describe('Metalsmith', function () {
     it('should provide out of the box support for process.env', function () {
       const npmvar = process.env.npm_config_shell
       const m = Metalsmith('test/tmp').env(process.env)
-
-      assert.deepStrictEqual(m.env(), Object.assign(Object.create(null), process.env))
+      const uppercased = Object.entries(process.env).reduce((vars, [name, value]) => {
+        vars[name.toUpperCase()] = value
+        return vars
+      }, {})
+      assert.deepStrictEqual(m.env(), Object.assign(Object.create(null), uppercased))
 
       m.env('npm_config_shell', '/bin/not-really')
       assert.strictEqual(process.env.npm_config_shell, npmvar)