Skip to content

Commit 7fc542a

Browse files
committed
fix: load env from root
1 parent 24c8508 commit 7fc542a

10 files changed

Lines changed: 368 additions & 7 deletions

File tree

.env.example

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,19 @@
11
# ─────────────────────────────────────────────────────────────────────────────
2+
# Where this file goes: the workspace root, as `.env` — beside this one.
3+
# ─────────────────────────────────────────────────────────────────────────────
4+
# cp .env.example .env
5+
#
6+
# One file, read by all three programs: the web app, the operator CLI
7+
# (`pnpm forum …`) and the worker. It used to have to live in `apps/forum/`
8+
# because that is where Next looks, which left the CLI reading nothing — so
9+
# `pnpm forum migrate` would report success having migrated fixture mode.
10+
#
11+
# `.env.local` is read first if present and wins where the two overlap; a real
12+
# environment variable beats both, so CI, Docker and the e2e suite are never
13+
# affected by whatever is in a checkout. Nothing here is loaded by the test
14+
# suite.
15+
#
16+
# ─────────────────────────────────────────────────────────────────────────────
217
# Data source
318
# ─────────────────────────────────────────────────────────────────────────────
419
# fixture = deterministic in-memory repositories (no database required).

.gitignore

Lines changed: 27 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -19,11 +19,33 @@ __v0_jsx-dev-runtime.ts
1919
*.tsbuildinfo
2020
next-env.d.ts
2121

22+
# Build output.
23+
#
24+
# `.next*/` rather than the two names that happen to exist today. The Next build
25+
# directory is chosen at *runtime* by FORUM_DIST_DIR — `.next` for `pnpm dev`,
26+
# `.next-e2e` for the Playwright server, and whatever a developer passes to run a
27+
# second server beside their own. Listing them individually means the next value
28+
# anybody uses is untracked-but-not-ignored, which is how a hundred megabytes of
29+
# compiled chunks arrive in a `git add -A`.
30+
#
31+
# `dist/` at any depth, not just `apps/worker/dist/`: the CLI declares a
32+
# `dist/index.js` bin and every package can be built the same way.
33+
.next*/
34+
dist/
35+
.turbo/
36+
coverage/
37+
38+
# Test artefacts. `test-results/` is Playwright's per-run output (traces,
39+
# screenshots, videos of failures) and can be large.
40+
playwright-report/
41+
blob-report/
42+
test-results/
43+
44+
# A local board's uploads — UPLOADS_DIR, `.uploads` by default, with
45+
# FILESTORE_DRIVER=local. Member content rather than source, and the one thing
46+
# here that would be a disclosure rather than merely noise.
47+
.uploads/
48+
2249
# Common ignores
2350
node_modules
24-
.next/
25-
.next-e2e/
2651
.DS_Store
27-
playwright-report/
28-
test-results/
29-
apps/worker/dist/

apps/cli/src/index.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@
1212

1313
import process from 'node:process'
1414

15+
import { loadEnvFiles, type LoadedEnvFiles } from '@forum/core/env-files'
16+
1517
import { importCommand } from './import'
1618
import { taskList, taskRun } from './tasks'
1719
import {
@@ -97,6 +99,13 @@ const commands: Command[] = [
9799
for (const [k, v] of rows) console.log(`${k.padEnd(width)} ${v}`)
98100

99101
console.log('\nEnvironment is valid.')
102+
console.log(
103+
envFiles.loaded.length > 0
104+
? `Loaded ${envFiles.loaded.join(', ')} from ${envFiles.root}.`
105+
: envFiles.root === undefined
106+
? 'No workspace root found — configuration came from the environment.'
107+
: `No .env files at ${envFiles.root} — configuration came from the environment.`,
108+
)
100109
if (env.DATA_SOURCE === 'fixture') {
101110
console.log(
102111
'DATA_SOURCE=fixture — in-memory sample data. Set DATABASE_URL for Postgres.',
@@ -299,7 +308,24 @@ const commands: Command[] = [
299308
* `forum --help` would advertise a capability the binary does not have.
300309
*/
301310

311+
/**
312+
* What `loadEnvFiles()` found, so `env:check` can report it.
313+
*
314+
* Assigned by `main()` before any command runs. `env:check` exists to answer
315+
* "what configuration am I actually running with", and the file that supplied it
316+
* is half that answer — an operator staring at `DATA_SOURCE fixture` needs to
317+
* know whether the CLI read their `.env` and it said fixture, or never found it.
318+
*/
319+
let envFiles: LoadedEnvFiles = { root: undefined, loaded: [] }
320+
302321
async function main(): Promise<number> {
322+
/*
323+
* First, before a command can import anything that reads `env`. The CLI is a
324+
* plain Node process: unlike `next dev`, nothing has populated `process.env`
325+
* from the workspace's `.env` by the time it starts.
326+
*/
327+
envFiles = loadEnvFiles()
328+
303329
const [name, ...rest] = process.argv.slice(2)
304330

305331
if (!name || name === '--help' || name === '-h') {

apps/forum/next.config.mjs

Lines changed: 52 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,59 @@
1+
import { existsSync } from "node:fs"
12
import path from "node:path"
23
import { fileURLToPath } from "node:url"
34

45
const here = path.dirname(fileURLToPath(import.meta.url))
56

7+
/** The workspace root — also `outputFileTracingRoot` below. */
8+
const workspaceRoot = path.join(here, "../../")
9+
10+
/**
11+
* F02 — the workspace's `.env` files, loaded from the root rather than from
12+
* `apps/forum`.
13+
*
14+
* Next loads `.env` files itself, from the directory it was started in — which
15+
* for `pnpm dev` is `apps/forum`, so the configuration had to live there while
16+
* the operator CLI and the worker read nothing at all. One root file for all
17+
* three is the whole point; see `packages/core/src/env-files.ts`, which is the
18+
* same two files in the same order for every program that is not Next.
19+
*
20+
* Done here, in the config, because it is the earliest thing Next evaluates in
21+
* every process that serves a request — earlier than `instrumentation.ts`, and
22+
* unlike it, plain Node with no Edge compilation to worry about. It is also
23+
* skipped entirely by the standalone production server, which never reads this
24+
* file: correct, since the image is configured by the container and
25+
* `.dockerignore` keeps `.env` out of it.
26+
*
27+
* Duplicated rather than imported: this file is `.mjs`, and `@forum/core` ships
28+
* TypeScript source that nothing has transpiled at the moment Next reads its
29+
* config. Four lines, and the list is stated in both places on purpose.
30+
*/
31+
const loadedEnvFiles = []
32+
for (const name of [".env.local", ".env"]) {
33+
const file = path.join(workspaceRoot, name)
34+
// First file to define a variable wins, and a real environment variable beats
35+
// both — `process.loadEnvFile` never overwrites a name that is already set.
36+
if (!existsSync(file)) continue
37+
process.loadEnvFile(file)
38+
loadedEnvFiles.push(name)
39+
}
40+
41+
/*
42+
* Next prints its own `- Environments: .env` line for files it found in this
43+
* directory, and it now finds none — so without this, moving the file to the
44+
* root would have removed the one line telling a developer which environment
45+
* their board is running with. That line is exactly what is missed when the app
46+
* turns out to be talking to a database nobody expected.
47+
*
48+
* Dev only, and only when a file was read: `next build` and the standalone
49+
* server take their configuration from the platform, where the message would be
50+
* noise. It prints once per process that loads this config, which in `next dev`
51+
* is the server itself.
52+
*/
53+
if (process.env.NODE_ENV !== "production" && loadedEnvFiles.length > 0) {
54+
console.log(`- Environments: ${loadedEnvFiles.join(", ")} (${workspaceRoot})`)
55+
}
56+
657
/**
758
* F04: `standalone` output so the Dockerfile can ship a self-contained server.
859
* F01: no `ignoreBuildErrors` — the strict typecheck is a real gate.
@@ -43,7 +94,7 @@ const nextConfig = {
4394
"@jsquash/png",
4495
"@jsquash/resize",
4596
],
46-
outputFileTracingRoot: path.join(here, "../../"),
97+
outputFileTracingRoot: workspaceRoot,
4798
images: {
4899
unoptimized: true,
49100
},

apps/worker/src/index.ts

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@
2828
* one, and it does not — nothing here is imported by the app.
2929
*/
3030
import { assertEnv, logger } from '@forum/core'
31+
import { loadEnvFiles } from '@forum/core/env-files'
3132
import { drivers } from '@forum/drivers'
3233
import { imageProcessor } from '@forum/drivers/images'
3334
import { buildSchedulerBundle } from '@forum/runtime'
@@ -59,6 +60,16 @@ const TICK_TIMEOUT_MS = 300_000
5960
let stopping = false
6061

6162
async function main(): Promise<number> {
63+
/*
64+
* Before `assertEnv()`, which memoises. In the image this finds nothing —
65+
* there is no workspace and `.dockerignore` keeps `.env` out on purpose, so
66+
* the container's own environment is the whole configuration. It is here for
67+
* `pnpm --filter @forum/worker start` against a developer's board, which
68+
* otherwise refuses to start with the fixture-mode message below while the
69+
* workspace `.env` sitting next to it says postgres.
70+
*/
71+
loadEnvFiles()
72+
6273
const env = assertEnv()
6374
if (env.DATA_SOURCE !== 'postgres') {
6475
/*

apps/worker/src/migrate.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
* a board migrated by an operator have been through identical code.
1919
*/
2020
import { logger } from '@forum/core'
21+
import { loadEnvFiles } from '@forum/core/env-files'
2122
import { runMigrations } from '@forum/db'
2223

2324
/**
@@ -30,6 +31,18 @@ import { runMigrations } from '@forum/db'
3031
*/
3132
const log = () => logger({ module: 'migrate' })
3233

34+
/*
35+
* Ahead of the first `process.env` read below, not merely ahead of the
36+
* migration: a variable consumed at module scope is one a `.env` could never
37+
* supply if this ran later.
38+
*
39+
* A no-op in the image, where the container supplies the environment and there
40+
* is no workspace to find. It costs one `existsSync` per parent directory and
41+
* it means this bundle and `forum migrate` are configured the same way when run
42+
* from a checkout — which is the point of them sharing `runMigrations()`.
43+
*/
44+
loadEnvFiles()
45+
3346
/**
3447
* Where the image puts the generated SQL.
3548
*

docker-compose.dev.yml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
# echo "<password>" | pnpm forum user:create --username you --email you@example.test
1313
# pnpm forum user:promote --user you --group administrators
1414
#
15-
# and in apps/forum/.env:
15+
# and in `.env` at the workspace root — one file for the app, the CLI and the
16+
# worker alike:
1617
#
1718
# DATA_SOURCE=postgres
1819
# DATABASE_URL=postgresql://postgres:postgres@127.0.0.1:55432/forum_test

packages/core/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
"./cache": "./src/cache.ts",
1111
"./errors": "./src/errors.ts",
1212
"./env": "./src/env.ts",
13+
"./env-files": "./src/env-files.ts",
1314
"./logger": "./src/logger.ts"
1415
},
1516
"scripts": {
Lines changed: 119 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,119 @@
1+
/**
2+
* F02 — where the environment comes from.
3+
*
4+
* These write real files and mutate the real `process.env`, because that is the
5+
* whole behaviour: `process.loadEnvFile` is Node's, and a test that stubbed it
6+
* would be asserting the shape of this module's own calls rather than the
7+
* precedence an operator actually gets. Every variable is named per test and
8+
* deleted afterwards — precedence here is "first write wins", so a name leaked
9+
* from one test would silently decide the next one's answer.
10+
*/
11+
import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'
12+
import { tmpdir } from 'node:os'
13+
import { join } from 'node:path'
14+
import { afterEach, describe, expect, it } from 'vitest'
15+
16+
import { findWorkspaceRoot, loadEnvFiles } from './env-files'
17+
18+
const created: string[] = []
19+
const touched: string[] = []
20+
21+
/** A throwaway workspace root: a directory with the marker file in it. */
22+
function workspace(files: Record<string, string>): string {
23+
const root = mkdtempSync(join(tmpdir(), 'forum-env-'))
24+
created.push(root)
25+
writeFileSync(join(root, 'pnpm-workspace.yaml'), "packages:\n - 'packages/*'\n")
26+
for (const [name, contents] of Object.entries(files)) {
27+
writeFileSync(join(root, name), contents)
28+
}
29+
return root
30+
}
31+
32+
/** A directory with no workspace above it. */
33+
function orphanDir(): string {
34+
const dir = mkdtempSync(join(tmpdir(), 'forum-noworkspace-'))
35+
created.push(dir)
36+
return dir
37+
}
38+
39+
/** Registers a variable for cleanup and returns its name. */
40+
function owned(name: string): string {
41+
touched.push(name)
42+
return name
43+
}
44+
45+
afterEach(() => {
46+
for (const name of touched.splice(0)) delete process.env[name]
47+
for (const dir of created.splice(0)) rmSync(dir, { recursive: true, force: true })
48+
})
49+
50+
describe('findWorkspaceRoot', () => {
51+
it('walks up to the directory holding pnpm-workspace.yaml', () => {
52+
const root = workspace({})
53+
const deep = join(root, 'apps', 'forum')
54+
mkdirSync(deep, { recursive: true })
55+
56+
expect(findWorkspaceRoot(deep)).toBe(root)
57+
// Inclusive of the starting directory: the CLI is often run from the root.
58+
expect(findWorkspaceRoot(root)).toBe(root)
59+
})
60+
61+
it('gives up rather than guessing when there is no workspace above', () => {
62+
expect(findWorkspaceRoot(orphanDir())).toBeUndefined()
63+
})
64+
})
65+
66+
describe('loadEnvFiles', () => {
67+
it('loads .env from the workspace root when started from a nested app', () => {
68+
const name = owned('FORUM_TEST_FROM_ROOT')
69+
const root = workspace({ '.env': `${name}=from-dot-env\n` })
70+
const deep = join(root, 'apps', 'cli')
71+
mkdirSync(deep, { recursive: true })
72+
73+
/* The case that motivated the module: `pnpm --filter @forum/cli start` runs
74+
with a cwd two levels below the file it needs. */
75+
expect(loadEnvFiles(deep)).toEqual({ root, loaded: ['.env'] })
76+
expect(process.env[name]).toBe('from-dot-env')
77+
})
78+
79+
it('prefers .env.local over .env', () => {
80+
const name = owned('FORUM_TEST_PRECEDENCE')
81+
const root = workspace({
82+
'.env': `${name}=from-dot-env\n`,
83+
'.env.local': `${name}=from-dot-env-local\n`,
84+
})
85+
86+
expect(loadEnvFiles(root).loaded).toEqual(['.env.local', '.env'])
87+
expect(process.env[name]).toBe('from-dot-env-local')
88+
})
89+
90+
it('never overwrites a variable the environment already set', () => {
91+
const name = owned('FORUM_TEST_AMBIENT_WINS')
92+
process.env[name] = 'from-the-environment'
93+
const root = workspace({ '.env': `${name}=from-dot-env\n` })
94+
95+
loadEnvFiles(root)
96+
97+
/*
98+
* The property CI, `docker run -e` and Playwright's `webServer.env` all
99+
* depend on. If a file could win, the e2e suite's explicit DATABASE_URL
100+
* would be quietly replaced by whatever a developer keeps in `.env`, and
101+
* the suite would write to their board.
102+
*/
103+
expect(process.env[name]).toBe('from-the-environment')
104+
})
105+
106+
it('reports the root and loads nothing when there are no env files', () => {
107+
const root = workspace({})
108+
expect(loadEnvFiles(root)).toEqual({ root, loaded: [] })
109+
})
110+
111+
it('is a no-op outside a workspace, which is the production case', () => {
112+
const orphan = orphanDir()
113+
// A `.env` here belongs to some other project, and is deliberately ignored.
114+
writeFileSync(join(orphan, '.env'), 'FORUM_TEST_STRAY=nope\n')
115+
116+
expect(loadEnvFiles(orphan)).toEqual({ root: undefined, loaded: [] })
117+
expect(process.env.FORUM_TEST_STRAY).toBeUndefined()
118+
})
119+
})

0 commit comments

Comments
 (0)