JS/Node.js .env loader

Question

I needed an environment values loader for a random node project, and instead of just using dotenv, decided to just make one myself and see how it'll turn out.

To make this meaningful, here's a rough description of the task requirements:

// make an .env file loader
// each line has the format key=value
// # starts a comment
// updates process.env and returns an object
// may only use native JS (ES5/6), no external dependencies

I've made two implementations:

Version 1:

module.exports = function() {
 var lines = require('fs').readFileSync('.env', 'utf8').split('\n');
 var map = {};
 for(var i = 0; i < lines.length; i++) {
 var line = lines[i].split('#')[0];
 var equalsIndex = line.indexOf('=');
 if(equalsIndex != -1) {
 var key = line.split(0, equalsIndex).trim();
 var value = line.split(equalsIndex + 1).trim();
 process.env[key] = value;
 map[key] = value;
 }
 }
 return map;
};

Version 2:

'use strict';
// dependencies
const FileSystem = require('fs');
// static
const DEFAULT_FILE_PATH = '.env';
const DEFAUTL_TEXT_ENCODING = 'utf8';
const DEFAULT_RECORD_SEPARATOR = '\n';
const DEFAULT_VALUE_SEPARATOR = '=';
const DEFAULT_COMMENT_SYMBOL = '#';
const DEFAULT_EMPTY_VALUE_FLAG = null;
// interface
module.exports = envLoaderSync;
// implementation
/**
 * synchronously loads values from a file into a returned object and into process.env
 * 
 * @param {Object?} attr - options
 * @param {string?} attr.filePath - default `.env`
 * @param {string?} attr.textEncoding - default `utf8`
 * @param {string?} attr.recordSeparator - default `\n`
 * @param {string?} attr.valueSeparator - default `=`
 * @param {string?} attr.commentSymbol - default `#`
 * @param {any?} attr.emptyValueFlag - default `null`, returned value for empty value strings
 * @param {boolean?} attr.toTryGuessingTypes - default false,
 * if true, tries to JSON.parse, and returns the parsed value on success, else the raw string
 * @param {boolean?} attr.toOverwriteProcessEnv = default true,
 * whether to overwrite process.env keys
 *
 * @effects - adds/overwrite keys in process.env
 * (only if toOverwriteProcessEnv === true)
 *
 * @return {Object.<{string, string|any}>} - map of values loaded
 * (type is any if toTryGuessingTypes === true, else string)
 */
/* public */ function envLoaderSync(attr) {
 // parameters
 if(attr === undefined) {
 attr = Object.create(null);
 }
 else if(!attr || typeof attr !== 'object' || Array.isArray(attr)) {
 console.error('envLoaderSync: if you provide an attr argument, it must be an object');
 throw Error('E_ATTR_NOT_OBJECT');
 }
 const filePath = getOptionalValue(attr, 'filePath', 'string', DEFAULT_FILE_PATH);
 const textEncoding = getOptionalValue(attr, 'textEncoding', 'string', DEFAUTL_TEXT_ENCODING);
 const recordSeparator = getOptionalValue(attr, 'recordSeparator', 'string', DEFAULT_RECORD_SEPARATOR);
 const valueSeparator = getOptionalValue(attr, 'valueSeparator', 'string', DEFAULT_VALUE_SEPARATOR);
 const commentSymbol = getOptionalValue(attr, 'commentSymbol', 'string', DEFAULT_COMMENT_SYMBOL);
 const emptyValueFlag = getOptionalValue(attr, 'emptyValueFlag', null, DEFAULT_EMPTY_VALUE_FLAG);
 const toTryGuessingTypes = getOptionalValue(attr, 'toTryGuessingTypes', 'boolean', false);
 const toOverwriteProcessEnv = getOptionalValue(attr, 'toOverwriteProcessEnv', 'boolean', true);
 // load file
 let fileContent;
 try {
 fileContent = FileSystem.readFileSync(filePath, textEncoding);
 }
 catch(err) {
 console.error('envLoaderSync:', err);
 throw Error('E_UNABLE_TO_OPEN_FILE');
 }
 // create output data structure
 const map = Object.create(null);
 // parse file content
 const records = fileContent.split(recordSeparator);
 const recordsLength = records.length;
 for(let i = 0; i < recordsLength; ++i) {
 let record = records[i];
 // ignore comment parts of records
 const commentIndex = record.indexOf(commentSymbol);
 if(commentIndex !== -1) {
 record = record.slice(0, commentIndex);
 }
 // ignore records that have no value separator
 const separatorIndex = record.indexOf(valueSeparator);
 if(separatorIndex === -1) {
 continue;
 }
 // store & update record as key-value pair
 const key = record.slice(0, separatorIndex).trim();
 let value = record.slice(separatorIndex + 1).trim();
 if(key === '') {
 continue; // empty key not allowed
 }
 if(value === '') {
 value = emptyValueFlag; // empty values replaced with flag
 }
 else if(toTryGuessingTypes) {
 try {
 value = JSON.parse(value);
 }
 catch(err) {} // not an error; default to stay as original string
 }
 map[key] = value;
 if(toOverwriteProcessEnv) {
 process.env[key] = value;
 }
 }
 return map;
}
/**
 * returns a value from an object by key
 * if missing in object, returns default value
 * if asType provided and value doesn't match the type, throws error
 *
 * @param {Object} fromObject - the object to extract values from
 * @param {string} asKey - the key to extract from the object
 * @param {string?} asType - if provided, throws error on value type mismatch
 * @param {any} defaultValue - returned if key not found
 *
 * @return {any} - value extracted from the object or default value
 */
/* private */ function getOptionalValue(fromObject, asKey, asType, defaultValue) {
 if(asKey in fromObject) {
 let value = fromObject[asKey];
 if(asType === null || typeof value === asType) {
 return value;
 }
 console.error('envLoaderSync: attr.' + asKey + ' must be of type ' + asType);
 throw Error('E_INVALID_TYPE');
 }
 return defaultValue;
}

Which version would you consider to be better, what would you change about each, what can be improved upon in general?

If you needed an env loader for your project, what would you prefer?

Answer 1

Nice question! This is a great demonstration of opposing design aesthetics.

I definitely prefer the first one. It is straightforward to read and does what it says. The story is clear. I'd prefer that the function didn't modify process.env directly as part of the loop (see toOverwriteProcessEnv), but it's all small gripe and not a big deal.

The second one has all these huge comments that make it hard (or impossible) to see the code all at once, and the constants actually make it harder to follow, forcing the readers eyes to jump around. Most of the flags look like "scope creep" to me... it's allowing a much more flexible file format than what 99% of the users will need. Is this a real requirement or speculation? A couple of the flags would be potentially useful, but I think I'd still prefer option #1.