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

chore: don't convert strings to numbers/booleans in the API #398

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/env-cmd.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export async function EnvCmd(
const proc = spawn(command, commandArgs, {
stdio: 'inherit',
shell: options.useShell,
env: env as Record<string, string>,
env,
})

// Handle any termination signals for parent and child proceses
Expand Down
50 changes: 31 additions & 19 deletions src/parse-env-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,16 @@ export async function getEnvFileVars(envFilePath: string): Promise<Environment>

// Get the file extension
const ext = extname(absolutePath).toLowerCase()
let env: Environment = {}
let env: unknown;
if (IMPORT_HOOK_EXTENSIONS.includes(ext)) {
// For some reason in ES Modules, only JSON file types need to be specifically delinated when importing them
let attributeTypes = {}
if (ext === '.json') {
attributeTypes = { [importAttributesKeyword]: { type: 'json' } }
}
const res = await import(pathToFileURL(absolutePath).href, attributeTypes) as Environment | { default: Environment }
if ('default' in res) {
env = res.default as Environment
const res: unknown = await import(pathToFileURL(absolutePath).href, attributeTypes)
if (typeof res === 'object' && res && 'default' in res) {
env = res.default
} else {
env = res
}
Expand All @@ -35,7 +35,7 @@ export async function getEnvFileVars(envFilePath: string): Promise<Environment>
env = await env
}

return env;
return normalizeEnvObject(env, absolutePath)
}

const file = readFileSync(absolutePath, { encoding: 'utf8' })
Expand Down Expand Up @@ -83,22 +83,10 @@ export function parseEnvVars(envString: string): Environment {
// inline comments.
value = value.split('#')[0].trim();
}

value = value.replace(/\\n/g, '\n');

// Convert string to JS type if appropriate
if (value !== '' && !isNaN(+value)) {
matches[key] = +value
}
else if (value === 'true') {
matches[key] = true
}
else if (value === 'false') {
matches[key] = false
}
else {
matches[key] = value
}
matches[key] = value
}
return JSON.parse(JSON.stringify(matches)) as Environment
}
Expand All @@ -118,3 +106,27 @@ export function stripEmptyLines(envString: string): string {
const emptyLinesRegex = /(^\n)/gim
return envString.replace(emptyLinesRegex, '')
}

/**
* If we load data from a file like .js, the user
* might export something which is not an object.
*
* This function ensures that the input is valid,
* and converts the object's values to strings, for
* consistincy. See issue #125 for details.
*/
export function normalizeEnvObject(input: unknown, absolutePath: string): Environment {
if (typeof input !== 'object' || !input) {
throw new Error(`env-cmd cannot load “${absolutePath}” because it does not export an object.`)
}

const env: Environment = {};
for (const [key, value] of Object.entries(input)) {
// we're intentionally stringifying the value here, to
// match what `child_process.spawn` does when loading
// env variables.
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
env[key] = `${value}`
}
return env
}
2 changes: 1 addition & 1 deletion src/types.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import type { Command } from '@commander-js/extra-typings'

// Define an export type
export type Environment = Partial<Record<string, string | number | boolean>>
export type Environment = Partial<Record<string, string>>

export type RCEnvironment = Partial<Record<string, Environment>>

Expand Down
44 changes: 28 additions & 16 deletions test/parse-env-file.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ describe('parseEnvVars', (): void => {
assert(envVars.BOB === 'COOL')
assert(envVars.NODE_ENV === 'dev')
assert(envVars.ANSWER === '42 AND COUNTING')
assert(envVars.NUMBER === 42)
assert(envVars.BOOLEAN === true)
assert(envVars.NUMBER === '42')
assert(envVars.BOOLEAN === 'true')
})

it('should parse out all env vars in string with format \'key=value\'', (): void => {
Expand Down Expand Up @@ -133,7 +133,7 @@ describe('parseEnvString', (): void => {
const env = parseEnvString('BOB=COOL\nNODE_ENV=dev\nANSWER=42\n')
assert(env.BOB === 'COOL')
assert(env.NODE_ENV === 'dev')
assert(env.ANSWER === 42)
assert(env.ANSWER === '42')
})
})

Expand All @@ -142,29 +142,29 @@ describe('getEnvFileVars', (): void => {
const env = await getEnvFileVars('./test/test-files/test.json')
assert.deepEqual(env, {
THANKS: 'FOR WHAT?!',
ANSWER: 42,
ANSWER: '42',
ONLY: 'IN PRODUCTION',
GALAXY: 'hitch\nhiking',
BRINGATOWEL: true,
BRINGATOWEL: 'true',
})
})

it('should parse a json file keeping all newlines intact', async (): Promise<void> => {
const env = await getEnvFileVars('./test/test-files/test-newlines.json')
assert.deepEqual(env, {
THANKS: 'FOR WHAT?!',
ANSWER: 42,
ANSWER: '42',
ONLY: 'IN\n PRODUCTION',
GALAXY: 'hitch\nhiking\n\n',
BRINGATOWEL: true,
BRINGATOWEL: 'true',
})
})

it('should parse a js/cjs file', async (): Promise<void> => {
const env = await getEnvFileVars('./test/test-files/test.cjs')
assert.deepEqual(env, {
THANKS: 'FOR ALL THE FISH',
ANSWER: 0,
ANSWER: '0',
GALAXY: 'hitch\nhiking',
})
})
Expand All @@ -173,15 +173,15 @@ describe('getEnvFileVars', (): void => {
const env = await getEnvFileVars('./test/test-files/test-async.cjs')
assert.deepEqual(env, {
THANKS: 'FOR ALL THE FISH',
ANSWER: 0,
ANSWER: '0',
})
})

it('should parse a mjs file', async (): Promise<void> => {
const env = await getEnvFileVars('./test/test-files/test.mjs')
assert.deepEqual(env, {
THANKS: 'FOR ALL THE FISH',
ANSWER: 0,
ANSWER: '0',
GALAXY: 'hitch\nhiking',
})
})
Expand All @@ -190,21 +190,21 @@ describe('getEnvFileVars', (): void => {
const env = await getEnvFileVars('./test/test-files/test-async.mjs')
assert.deepEqual(env, {
THANKS: 'FOR ALL THE FISH',
ANSWER: 0,
ANSWER: '0',
})
})

it('should parse an env file', async (): Promise<void> => {
const env = await getEnvFileVars('./test/test-files/test')
assert.deepEqual(env, {
THANKS: 'FOR WHAT?!',
ANSWER: 42,
ANSWER: '42',
ONLY: 'IN=PRODUCTION',
GALAXY: 'hitch\nhiking',
BRINGATOWEL: true,
a: 1,
b: 2,
c: 3,
BRINGATOWEL: 'true',
a: '1',
b: '2',
c: '3',
d: "=",
e: "equals symbol = = ",
json_no_quotes: "{\"foo\": \"bar\"}",
Expand All @@ -223,4 +223,16 @@ describe('getEnvFileVars', (): void => {
assert.match(e.message, /file path/gi)
}
})

for (const fileExt of ['cjs', 'mjs', 'json']) {
it(`should throw an error when importing a ${fileExt} file with an invalid export`, async () => {
try {
await getEnvFileVars(`./test/test-files/invalid.${fileExt}`)
assert.fail('Should not get here!')
} catch (e) {
assert.instanceOf(e, Error)
assert.match(e.message, /does not export an object/gi)
}
})
}
})
1 change: 1 addition & 0 deletions test/test-files/invalid.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
module.exports = undefined;
1 change: 1 addition & 0 deletions test/test-files/invalid.json
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
123
1 change: 1 addition & 0 deletions test/test-files/invalid.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default "this is invalid; it's not an object";
Loading