Skip to content

Commit

Permalink
Iterate on .env files: make the behavior override (#10094)
Browse files Browse the repository at this point in the history
Follow up to #10093. After
discussing with @orta, the `--add-env-files` flag being additive instead
of overriding wasn't a sticking point in the original implementation,
and the more feedback I get, it seems like most people expect later
files to override earlier ones, so let's switch the behavior.

In tandem, at first I tried to rename the flag back to just `--env-file`
or `--env-files`, but I'm afraid both give this cryptic error:

```
redwood-app % yarn rw --env-file prod
node: prod: not found

# Or `yarn rw --env-files prod`, which also gives this error,
# which I didn't know node would also process? Their docs only say `--env-file`...
```

The above was with my framework changes. I tried it again without my
framework changes and the error persists. As far as I can tell, `rw`
never executes and it seems like the Node.js binary itself is evaluating
the `--env-file` flag. It supports `--env-file` now, and expects it to
be a full path (so `.env.prod`). But this isn't how I thought node
processes `process.argv` at all... I thought that all flags would've
been passed to the script (here, `rw`) for processing. But maybe not,
and if so that means we can just pass options to node via `yarn rw
--my-node-option`. Which I still don't quite believe and if it is true,
then I'm not sure what to make of yet cause people have been wanting to
pass node options for a while and I always thought
`NODE_OPTIONS="--my-node-option" yarn rw ...` was the only way.

So TL;DR, that's why I've left the name here and I'd like to keep that
friction point out of the scope of this PR being considered mergeable.
  • Loading branch information
jtoar authored Mar 2, 2024
1 parent 0ecf1ad commit b41ca33
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 48 deletions.
17 changes: 6 additions & 11 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -37,24 +37,19 @@
DataDog/import-in-the-middle#57
* This version does not support Node.js 18.19 or later

- Add support for additional env var files (#9961) and (#10093)
- Add support for loading more env var files (#9961, #10093, and #10094)

Fixes #9877. This PR adds CLI functionality to load more `.env` files via `NODE_ENV` and an `--add-env-files` flag.

Env vars loaded via `NODE_ENV` override the values in `.env`; if there are conflicts, they win out:
Env vars loaded via either of these methods override the values in `.env`:

```
# Loads '.env.production', which overwrites values in '.env'
# Loads '.env.production', which overrides values in '.env'
NODE_ENV=production yarn rw exec myScript
```

Env vars loaded via `--add-env-files` only add to `process.env`; they will not override anything that was previously there:
```bash
# Add new env vars defined in '.env.stripe' and '.env.nakama'
yarn rw exec myScript --add-env-files stripe nakama
# Or you can specify the flag twice:
# Load '.env.stripe' and '.env.nakama', which overrides values
yarn rw exec myScript --add-env-files stripe --add-env-files nakama
# Or you can specify the flag once:
yarn rw exec myScript --add-env-files stripe nakama
```

Note that this feature is mainly for local scripting. Most deploy providers don't let you upload `.env` files (unless you're using baremetal) and usually have their own way of determining environments.
Expand Down
40 changes: 20 additions & 20 deletions packages/cli/src/__tests__/loadEnvFiles.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ import path from 'path'
import { afterEach, beforeAll, describe, expect, it, test } from 'vitest'

import {
loadBasicEnvFiles,
loadDefaultEnvFiles,
loadNodeEnvDerivedEnvFile,
addUserSpecifiedEnvFiles,
loadUserSpecifiedEnvFiles,
} from '../lib/loadEnvFiles'

describe('loadEnvFiles', () => {
Expand All @@ -19,18 +19,18 @@ describe('loadEnvFiles', () => {

it("doesn't load .env files if there are none to load", () => {
const cwd = __dirname
loadBasicEnvFiles(cwd)
loadDefaultEnvFiles(cwd)
loadNodeEnvDerivedEnvFile(cwd)
addUserSpecifiedEnvFiles(cwd, [])
loadUserSpecifiedEnvFiles(cwd, [])

expect(process.env).toEqual(originalProcessEnv)
})

it("doesn't load .env files if not instructed to", () => {
const cwd = path.join(__dirname, '__fixtures__/redwood-app-env-prod')
loadBasicEnvFiles(cwd)
loadDefaultEnvFiles(cwd)
loadNodeEnvDerivedEnvFile(cwd)
addUserSpecifiedEnvFiles(cwd, [])
loadUserSpecifiedEnvFiles(cwd, [])

expect(process.env).toEqual(originalProcessEnv)
})
Expand All @@ -39,9 +39,9 @@ describe('loadEnvFiles', () => {
expect(process.env).not.toHaveProperty('PROD_DATABASE_URL')

const cwd = path.join(__dirname, '__fixtures__/redwood-app-env-prod')
loadBasicEnvFiles(cwd)
loadDefaultEnvFiles(cwd)
loadNodeEnvDerivedEnvFile(cwd)
addUserSpecifiedEnvFiles(cwd, ['prod'])
loadUserSpecifiedEnvFiles(cwd, ['prod'])

expect(process.env).toHaveProperty(
'PROD_DATABASE_URL',
Expand All @@ -58,9 +58,9 @@ describe('loadEnvFiles', () => {
expect(process.env).not.toHaveProperty('PROD_DATABASE_URL')

const cwd = path.join(__dirname, '__fixtures__/redwood-app-env-many')
loadBasicEnvFiles(cwd)
loadDefaultEnvFiles(cwd)
loadNodeEnvDerivedEnvFile(cwd)
addUserSpecifiedEnvFiles(cwd, ['dev', 'prod'])
loadUserSpecifiedEnvFiles(cwd, ['dev', 'prod'])

expect(process.env).toHaveProperty(
'DEV_DATABASE_URL',
Expand All @@ -78,13 +78,13 @@ describe('loadEnvFiles', () => {
expect(process.env).not.toHaveProperty('TEST_COLLISION')

const cwd = path.join(__dirname, '__fixtures__/redwood-app-env-collision')
loadBasicEnvFiles(cwd)
loadDefaultEnvFiles(cwd)
loadNodeEnvDerivedEnvFile(cwd)
addUserSpecifiedEnvFiles(cwd, ['base', 'collision'])
loadUserSpecifiedEnvFiles(cwd, ['base', 'collision'])

expect(process.env).toHaveProperty(
'DATABASE_URL',
'postgresql://user:password@localhost:5432/mydb'
'postgresql://user:password@localhost:5432/mycollisiondb'
)
expect(process.env).toHaveProperty('TEST_BASE', '1')
expect(process.env).toHaveProperty('TEST_COLLISION', '1')
Expand All @@ -96,9 +96,9 @@ describe('loadEnvFiles', () => {

process.env.NODE_ENV = 'bazinga'
const cwd = path.join(__dirname, '__fixtures__/redwood-app-env-node-env')
loadBasicEnvFiles(cwd)
loadDefaultEnvFiles(cwd)
loadNodeEnvDerivedEnvFile(cwd)
addUserSpecifiedEnvFiles(cwd, [])
loadUserSpecifiedEnvFiles(cwd, [])

expect(process.env).toHaveProperty(
'PROD_DATABASE_URL',
Expand All @@ -113,13 +113,13 @@ describe('loadEnvFiles', () => {

process.env.NODE_ENV = 'bazinga'
const cwd = path.join(__dirname, '__fixtures__/redwood-app-env-node-env')
loadBasicEnvFiles(cwd)
loadDefaultEnvFiles(cwd)
loadNodeEnvDerivedEnvFile(cwd)
addUserSpecifiedEnvFiles(cwd, ['prod'])
loadUserSpecifiedEnvFiles(cwd, ['prod'])

expect(process.env).toHaveProperty(
'PROD_DATABASE_URL',
'postgresql://user:password@localhost:5432/bazinga'
'postgresql://user:password@localhost:5432/myproddb'
)
expect(process.env).toHaveProperty('BAZINGA', '1')
})
Expand All @@ -128,9 +128,9 @@ describe('loadEnvFiles', () => {
const cwd = path.join(__dirname, '__fixtures__/redwood-app-env-node-env')

try {
loadBasicEnvFiles(cwd)
loadDefaultEnvFiles(cwd)
loadNodeEnvDerivedEnvFile(cwd)
addUserSpecifiedEnvFiles(cwd, ['missing'])
loadUserSpecifiedEnvFiles(cwd, ['missing'])
} catch (error) {
// Just testing that the error message reports the file it tried to load.
expect(error.message).toMatch(/\.env\.missing/)
Expand Down
7 changes: 4 additions & 3 deletions packages/cli/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,12 +175,13 @@ async function runYargs() {
describe: 'Working directory to use (where `redwood.toml` is located)',
})
.option('add-env-files', {
describe: 'Load additional .env files. These are incremental',
describe:
'Load additional .env files. Values defined in files specified later override earlier ones.',
array: true,
})
.example(
'yarn rw exec MigrateUsers --add-env-files prod stripe-prod',
'"Run a script, and also include .env.prod and .env.stripe-prod"'
'yarn rw exec migrateUsers --add-env-files stripe nakama',
"Run a script, also loading env vars from '.env.stripe' and '.env.nakama'"
)
.option('telemetry', {
describe: 'Whether to send anonymous usage telemetry to RedwoodJS',
Expand Down
15 changes: 5 additions & 10 deletions packages/cli/src/lib/loadEnvFiles.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,17 @@ export function loadEnvFiles() {

const { base } = getPaths()

// These override.
loadBasicEnvFiles(base)
loadDefaultEnvFiles(base)
loadNodeEnvDerivedEnvFile(base)

// These are additive. I.e., They don't override existing env vars.
// defined in .env.defaults, .env, or .env.${NODE_ENV}
//
// Users have to opt-in to loading these files via `--add-env-files`.
const { addEnvFiles } = Parser(hideBin(process.argv), {
array: ['add-env-files'],
default: {
addEnvFiles: [],
},
})
if (addEnvFiles.length > 0) {
addUserSpecifiedEnvFiles(base, addEnvFiles)
loadUserSpecifiedEnvFiles(base, addEnvFiles)
}

process.env.REDWOOD_ENV_FILES_LOADED = 'true'
Expand All @@ -40,7 +35,7 @@ export function loadEnvFiles() {
/**
* @param {string} cwd
*/
export function loadBasicEnvFiles(cwd) {
export function loadDefaultEnvFiles(cwd) {
dotenvDefaultsConfig({
path: path.join(cwd, '.env'),
defaults: path.join(cwd, '.env.defaults'),
Expand Down Expand Up @@ -70,7 +65,7 @@ export function loadNodeEnvDerivedEnvFile(cwd) {
/**
* @param {string} cwd
*/
export function addUserSpecifiedEnvFiles(cwd, addEnvFiles) {
export function loadUserSpecifiedEnvFiles(cwd, addEnvFiles) {
for (const suffix of addEnvFiles) {
const envPath = path.join(cwd, `.env.${suffix}`)
if (!fs.pathExistsSync(envPath)) {
Expand All @@ -79,6 +74,6 @@ export function addUserSpecifiedEnvFiles(cwd, addEnvFiles) {
)
}

dotenvConfig({ path: envPath })
dotenvConfig({ path: envPath, override: true })
}
}
10 changes: 6 additions & 4 deletions tasks/server-tests/bothServer.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,9 @@ describe('rw serve', () => {
--version Show version number [boolean]
--cwd Working directory to use (where
\`redwood.toml\` is located)
--add-env-files Load additional .env files. These
are incremental [array]
--add-env-files Load additional .env files. Values
defined in files specified later
override earlier ones. [array]
--telemetry Whether to send anonymous usage
telemetry to RedwoodJS [boolean]
--webPort, --web-port The port for the web server to
Expand Down Expand Up @@ -67,8 +68,9 @@ describe('rw serve', () => {
--version Show version number [boolean]
--cwd Working directory to use (where
\`redwood.toml\` is located)
--add-env-files Load additional .env files. These
are incremental [array]
--add-env-files Load additional .env files. Values
defined in files specified later
override earlier ones. [array]
--telemetry Whether to send anonymous usage
telemetry to RedwoodJS [boolean]
--webPort, --web-port The port for the web server to
Expand Down

0 comments on commit b41ca33

Please sign in to comment.