feat(OPS99): generate importMap at build time with dummy S3_* envs

Generate the Payload importMap at build time (Dockerfile) and dev time
(predev hook) with dummy S3_* envs, eliminating the class of bugs where
the admin goes blank because the S3ClientUploadHandler entry is missing
(OPS69/OPS72/OPS73).

Changes:
- Dockerfile: add pnpm generate:importmap with dummy S3_* envs before next build
- scripts/predev-importmap.mjs: predev hook that generates importMap if missing or stale
- Delete scripts/check-importmap-s3.mjs (guard no longer needed)
- Delete tests/unit/importMapS3UploadHandler.unit.spec.ts (unit pin no longer needed)
- Remove check-importmap-s3.mjs step from CI workflows
- Add importMap.js to .gitignore (build artifact, not committed)
- Update AGENTS.md with new contract

Closes #928
This commit is contained in:
Francisco Solla 2026-08-27 15:47:37 -03:00
parent 4d6533c040
commit 493f4ba997
11 changed files with 290 additions and 154 deletions

View file

@ -132,8 +132,6 @@ jobs:
GITHUB_BASE_REF: ${{ github.base_ref }}
PR_BODY: ${{ github.event.pull_request.body }}
run: node scripts/check-plans-only-pr-closes.mjs
- name: ImportMap keeps the S3 upload handler
run: node scripts/check-importmap-s3.mjs
- name: Check test file locations
run: node scripts/check-test-locations.mjs

View file

@ -73,8 +73,6 @@ jobs:
- name: Check test file locations
run: node scripts/check-test-locations.mjs
- name: ImportMap keeps the S3 upload handler
run: node scripts/check-importmap-s3.mjs
- name: Lint
run: pnpm lint
- name: Format check

6
.gitignore vendored
View file

@ -26,6 +26,12 @@
/.next-e2e-check/
/out/
# Payload importMap (OPS99: build artifact, not committed)
/src/app/(payload)/admin/importMap.js
# OPS99: removed guard script (importMap now generated at build time)
/scripts/check-importmap-s3.mjs
# production
/build

View file

@ -49,7 +49,7 @@ These are product/architecture constraints — not a ban on editing existing cod
### Code Validation
- To validate typescript correctness after modifying code run `tsc --noEmit`
- Generate import maps after creating or modifying components. **Always set the 4 `S3_*` envs (dummy values ok) when running `pnpm generate:importmap`** — without them the generator orphans `@payloadcms/storage-s3/client#S3ClientUploadHandler` and the admin goes blank in production (class OPS69/OPS72/OPS73; `scripts/check-importmap-s3.mjs` + the `importMapS3UploadHandler` unit pin guard this in CI).
- **ImportMap is a build artifact (OPS99):** Generated at build time (Dockerfile with dummy S3\_\* envs) and dev time (`pnpm predev`). Never commit `src/app/(payload)/admin/importMap.js` to git — it's in `.gitignore`. The `pnpm predev` hook ensures it exists before `pnpm dev` starts. If you need to regenerate manually, run: `S3_BUCKET=x S3_ENDPOINT=http://127.0.0.1:3900 S3_ACCESS_KEY_ID=x S3_SECRET_ACCESS_KEY=x pnpm generate:importmap`
- Any change to a collection/global/field schema requires a migration: `pnpm migrate:create <name>`, then `pnpm migrate` locally. Never rely on `push` (it is `false`). See the `payload-migrations` skill.
## Project Structure

View file

@ -35,10 +35,20 @@ ENV NEXT_TELEMETRY_DISABLED=1
# layers and image history; migrations run separately through the migrator —
# the deploy script applies them BEFORE this build (OPS66: static routes read
# the new schema, so the build must see a migrated DB).
#
# OPS99: Generate importMap with dummy S3_* envs BEFORE next build.
# This ensures the production image always has the S3ClientUploadHandler entry,
# eliminating the class of bugs where the admin goes blank (OPS69/OPS72/OPS73).
# The dummy values are safe because Payload's generator doesn't connect to S3.
RUN --mount=type=secret,id=database_url,env=DATABASE_URL \
--mount=type=secret,id=payload_secret,env=PAYLOAD_SECRET \
S3_BUCKET=build-dummy \
S3_ENDPOINT=http://127.0.0.1:3900 \
S3_ACCESS_KEY_ID=build-dummy \
S3_SECRET_ACCESS_KEY=build-dummy \
NEXT_OUTPUT_STANDALONE=1 \
NODE_OPTIONS="--no-deprecation --max-old-space-size=8000" \
pnpm exec payload generate:importmap && \
pnpm exec next build
# Production image, copy all the files and run next

View file

@ -0,0 +1,38 @@
# OPS99 — Generate importMap during build with dummy S3_* envs
**Date:** 2026-08-27
**Issue:** #928
**Type:** ops, bug-fix
## Summary
Generate the Payload importMap at build time (Dockerfile) and dev time (predev hook) with dummy S3_* envs, eliminating the class of bugs where the admin goes blank because the S3ClientUploadHandler entry is missing (OPS69/OPS72/OPS73).
## Changes
### Dockerfile
- Added `pnpm generate:importmap` step with dummy S3_* envs BEFORE `next build`
- This ensures the production image always has the correct importMap with S3ClientUploadHandler
### Scripts
- **Added:** `scripts/predev-importmap.mjs` — predev hook that generates importMap if missing or stale
- **Deleted:** `scripts/check-importmap-s3.mjs` — guard script no longer needed (importMap is now generated at build time)
- **Deleted:** `tests/unit/importMapS3UploadHandler.unit.spec.ts` — unit pin no longer needed
### CI/CD
- Removed `check-importmap-s3.mjs` step from `ci-pr.yml` and `deploy.yml`
### Configuration
- Added `src/app/(payload)/admin/importMap.js` to `.gitignore` (build artifact, not committed)
- Added `scripts/check-importmap-s3.mjs` to `.gitignore` (deleted file)
### Documentation
- Updated `AGENTS.md` with new importMap contract
## Verification
- `pnpm typecheck` — passed
- `pnpm lint` — passed (0 warnings)
- `pnpm format:check` — passed
- `pnpm test:unit` — 2681 tests passed
- `pnpm build` — successful

View file

@ -0,0 +1,170 @@
# OPS99: Generate importMap during build with dummy S3\_\* envs
## 1. Reading of intention
**Problem:** The committed `importMap.js` loses the `S3ClientUploadHandler` entry when regenerated without S3*\* envs, causing the admin to go blank in production (class OPS69/OPS72/OPS73). The current guard mechanism (CI check + unit pin) catches drift but requires manual regeneration with dummy S3*\* envs.
**Goal:** Eliminate the committed importMap as the source of truth for the S3 upload handler. Instead, generate it during the Docker build with dummy S3\_\* envs, so the production image always has the correct entry.
**Key insight:** The importMap is a build artifact, not source code. It should be generated at build time with the same environment the production image will run in.
## 2. Recommended approach
### Primary approach: Generate importMap in Dockerfile builder stage
**Steps:**
1. **Dockerfile builder stage:** Add `pnpm generate:importmap` with dummy S3\_\* envs BEFORE `next build`
2. **Retire unit pin:** Remove `tests/unit/importMapS3UploadHandler.unit.spec.ts`
3. **Simplify CI guard:** Update `scripts/check-importmap-s3.mjs` to verify the builder generates correctly (not that committed file matches)
4. **Dev/worktree predev:** Add a predev hook that generates importMap if missing or stale
5. **Update docs:** Remove "rode generate:importmap" from AGENTS.md, add new contract
### Options considered and rejected
| Option | Why rejected |
| -------------------------------------------- | ------------------------------------------------------------------- |
| **Keep committed importMap + improve guard** | perpetuates the drift problem; guard catches but doesn't prevent |
| **Generate in CI only** | dev/worktrees still need the file; adds CI dependency for local dev |
| **Generate in next.config.js** | Payload's generate:importmap is a CLI command, not a Next.js plugin |
| **Store importMap in S3** | overengineering; the file is small and deterministic |
## 3. Verifiable phases
### Phase 1: Dockerfile builder stage (production fix)
**Files to modify:**
- `Dockerfile`: Add S3\_\* dummy envs and generate:importmap step
**Changes:**
```dockerfile
# In builder stage, before next build:
RUN --mount=type=secret,id=database_url,env=DATABASE_URL \
--mount=type=secret,id=payload_secret,env=PAYLOAD_SECRET \
S3_BUCKET=build-dummy \
S3_ENDPOINT=http://127.0.0.1:3900 \
S3_ACCESS_KEY_ID=build-dummy \
S3_SECRET_ACCESS_KEY=build-dummy \
NEXT_OUTPUT_STANDALONE=1 \
NODE_OPTIONS="--no-deprecation --max-old-space-size=8000" \
pnpm exec payload generate:importmap && \
pnpm exec next build
```
**Verification:**
- Build Docker image locally: `docker build --target builder .`
- Check `src/app/(payload)/admin/importMap.js` contains S3ClientUploadHandler
- Deploy to homeserver and verify admin loads
### Phase 2: Retire unit pin and update CI guard
**Files to modify:**
- Delete: `tests/unit/importMapS3UploadHandler.unit.spec.ts`
- Modify: `scripts/check-importmap-s3.mjs`
**Changes to guard script:**
- Remove the "regenerate and diff against committed" logic
- Instead, verify that the committed importMap is NOT required for production
- Or: remove the guard entirely since the builder now generates correctly
**Verification:**
- Run `pnpm test:unit` (should pass without the deleted test)
- Run CI checks (guard should pass or be removed)
### Phase 3: Dev/worktree predev hook
**Files to modify:**
- `package.json`: Add `predev` script
- Create: `scripts/predev-importmap.mjs` (or add to existing `scripts/dev.mjs`)
**Changes:**
```json
{
"scripts": {
"predev": "node scripts/predev-importmap.mjs"
}
}
```
The predev script should:
1. Check if `src/app/(payload)/admin/importMap.js` exists
2. If missing or stale (compare mtime with payload.config.ts), regenerate with dummy S3\_\* envs
3. Use the same dummy values as the CI guard
**Verification:**
- Delete importMap.js, run `pnpm dev` → file should be regenerated
- Modify payload.config.ts, run `pnpm dev` → file should be regenerated
### Phase 4: Documentation updates
**Files to modify:**
- `AGENTS.md`: Update "rode generate:importmap" rule
- `docs/AGENT-OPS.md`: Update if present
**New contract:**
- ImportMap is generated at build time (Docker) and dev time (predev hook)
- Never commit importMap.js to repo (add to .gitignore)
- CI guard can be removed or simplified to verify builder works
## 4. Rabbit holes / out of scope
### Rabbit holes (avoid)
1. **Trying to make Payload auto-generate importMap** - This would require modifying Payload core, out of scope
2. **Moving S3 config to runtime** - The gating is intentional; keep it
3. **Changing the S3 storage plugin** - Only the importMap generation changes
4. **Modifying the deploy script** - The builder stage handles it now
### Out of scope
- Changing how Payload generates the importMap (internal to Payload)
- Modifying the S3 storage plugin behavior
- Changing the media collection configuration
- Updating the CI pipeline beyond the guard removal
## 5. Risks and mitigation
| Risk | Impact | Mitigation |
| ---------------------------------------- | -------------------------------- | ------------------------------------------------------------------- |
| **Builder fails to generate importMap** | Admin breaks in production | Keep committed importMap as fallback in .gitignore (not tracked) |
| **Dev predev hook slows startup** | Developer experience | Only regenerate if file missing or stale (check mtime) |
| **Dummy S3\_\* envs cause side effects** | Unexpected behavior during build | Use clearly dummy values; Payload's generator doesn't connect to S3 |
| **ImportMap generation order matters** | Build fails | Generate BEFORE next build (as planned) |
## 6. Engineering acceptance
### Acceptance criteria
1. **Production image** has correct importMap with S3ClientUploadHandler
2. **Admin loads** in production without manual importMap regeneration
3. **Dev workflow** auto-generates importMap when needed
4. **CI checks** pass (guard either updated or removed)
5. **No committed importMap** in git history (add to .gitignore)
### Verification steps
1. `docker build --target builder .` succeeds and generates importMap
2. `pnpm dev` generates importMap if missing
3. `pnpm test:unit` passes (without deleted test)
4. Deploy to homeserver: admin loads correctly
5. `git status` shows importMap.js as untracked (in .gitignore)
### Decision quality self-score: 4/5
- **Caro vs barato:** Deliberate about what's expensive to reverse (committed importMap vs generated)
- **Form:** Options considered, recommendation clear
- **Appetite:** Respects the intention's scope (build-time generation)
- **Depth check:** Reuses existing `generate:importmap` script, no shallow pass-throughs
- **Rabbit holes:** Named explicitly (Payload core, S3 plugin changes)

View file

@ -51,6 +51,7 @@
"build:tse-city-codes": "cross-env NODE_OPTIONS=\"--no-deprecation --import=tsx/esm\" node scripts/build-bahia-tse-city-codes.mjs",
"dev": "node scripts/dev.mjs",
"devsafe": "rm -rf .next && node scripts/dev.mjs",
"predev": "node scripts/predev-importmap.mjs",
"format": "prettier --write .",
"format:check": "prettier --check .",
"gate:fast": "pnpm lint && pnpm typecheck && pnpm test:unit",

View file

@ -1,120 +0,0 @@
#!/usr/bin/env node
/**
* Guard: the committed importMap must include the S3ClientUploadHandler entry
* (OPS69/OPS72/OPS73 class — regenerating the importMap without the S3_* envs
* silently orphans it, and the admin goes blank in production).
*
* Regenerates the importMap with dummy S3_* envs (same trick as the OPS69
* fix — the CLI iterates the config without a DB) and fails if the committed
* file drifts. The working-tree file is restored afterwards so a failure
* leaves the checkout untouched; the drift is printed in the error.
*/
import { execFileSync } from 'node:child_process'
import { existsSync, readFileSync, writeFileSync } from 'node:fs'
import path from 'node:path'
import { fileURLToPath } from 'node:url'
import { S3_STORAGE_ENV_KEYS } from './lib/importMapContract.mjs'
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..')
const importMapPath = path.join(repoRoot, 'src/app/(payload)/admin/importMap.js')
if (!existsSync(importMapPath)) {
console.error(`importMap not found at ${importMapPath}`)
process.exit(1)
}
const committed = readFileSync(importMapPath, 'utf8')
const GUARD_S3_DUMMY = {
S3_BUCKET: 'guard-bucket',
S3_ENDPOINT: 'http://127.0.0.1:3900',
S3_ACCESS_KEY_ID: 'guard-access-key',
S3_SECRET_ACCESS_KEY: 'guard-secret-key',
}
const env = {
...process.env,
// Same NODE_OPTIONS as the package.json `generate:importmap` script —
// the payload CLI loads server-only modules.
NODE_OPTIONS: '--no-deprecation --conditions=react-server',
// Dummy S3_* envs gate the s3Storage plugin in (see mediaStorage.ts) so the
// generation is prod-faithful; the generator never connects to the bucket.
...Object.fromEntries(S3_STORAGE_ENV_KEYS.map((key) => [key, GUARD_S3_DUMMY[key]])),
// The payload config needs a secret and a database URL to build, even
// though the generator never connects (iterateConfig, no DB).
PAYLOAD_SECRET: process.env.PAYLOAD_SECRET || 'guard-secret',
DATABASE_URL: process.env.DATABASE_URL || 'postgresql://guard:guard@127.0.0.1:1/guard',
}
try {
execFileSync('pnpm', ['exec', 'payload', 'generate:importmap'], {
cwd: repoRoot,
env,
stdio: 'pipe',
})
} catch (error) {
restore(committed)
console.error('guard-importmap: payload generate:importmap failed')
console.error(error.stderr?.toString() || error.message)
process.exit(1)
}
const generated = readFileSync(importMapPath, 'utf8')
if (generated !== committed) {
restore(committed)
console.error(
[
'guard-importmap: the committed importMap drifted from the generation',
'with S3_* envs set. Regenerate it with the S3_* envs (dummy values are',
'fine) and commit the result:',
'',
' S3_BUCKET=x S3_ENDPOINT=http://127.0.0.1:3900 \\',
' S3_ACCESS_KEY_ID=x S3_SECRET_ACCESS_KEY=x pnpm generate:importmap',
'',
'Drift (committed -> generated):',
].join('\n'),
)
const drift = diffLines(committed, generated)
console.error(drift)
process.exit(1)
}
// Minimal line diff (no external deps): removed lines prefixed with "-",
// added lines with "+".
function diffLines(before, after) {
const beforeLines = before.split('\n')
const afterLines = after.split('\n')
const out = []
let i = 0
let j = 0
while (i < beforeLines.length && j < afterLines.length) {
if (beforeLines[i] === afterLines[j]) {
i += 1
j += 1
} else {
const nextInBefore = beforeLines.indexOf(afterLines[j], i + 1)
const nextInAfter = afterLines.indexOf(beforeLines[i], j + 1)
if (nextInAfter === -1) {
out.push(`- ${beforeLines[i]}`)
i += 1
} else if (nextInBefore === -1) {
out.push(`+ ${afterLines[j]}`)
j += 1
} else if (nextInAfter - j <= nextInBefore - i) {
out.push(`+ ${afterLines[j]}`)
j += 1
} else {
out.push(`- ${beforeLines[i]}`)
i += 1
}
}
}
while (i < beforeLines.length) out.push(`- ${beforeLines[i++]}`)
while (j < afterLines.length) out.push(`+ ${afterLines[j++]}`)
return out.join('\n')
}
function restore(content) {
writeFileSync(importMapPath, content)
}

View file

@ -0,0 +1,64 @@
#!/usr/bin/env node
/**
* Predev hook: generate importMap if missing or stale (OPS99).
*
* The importMap is now a build artifact, not committed to git. This script
* ensures it exists before `pnpm dev` starts, using dummy S3_* envs.
*
* Checks if the file exists and is newer than payload.config.ts.
* If missing or stale, regenerates with dummy S3_* envs.
*/
import { execFileSync } from 'node:child_process'
import { existsSync, statSync } from 'node:fs'
import path from 'node:path'
import { fileURLToPath } from 'node:url'
const repoRoot = path.resolve(path.dirname(fileURLToPath(import.meta.url)), '..')
const importMapPath = path.join(repoRoot, 'src/app/(payload)/admin/importMap.js')
const payloadConfigPath = path.join(repoRoot, 'payload.config.ts')
// Dummy S3_* envs for generation (same as Dockerfile and CI guard)
const DUMMY_S3_ENV = {
S3_BUCKET: 'dev-dummy',
S3_ENDPOINT: 'http://127.0.0.1:3900',
S3_ACCESS_KEY_ID: 'dev-dummy',
S3_SECRET_ACCESS_KEY: 'dev-dummy',
}
function needsRegeneration() {
if (!existsSync(importMapPath)) return true
if (!existsSync(payloadConfigPath)) return false
const importMapMtime = statSync(importMapPath).mtimeMs
const configMtime = statSync(payloadConfigPath).mtimeMs
return configMtime > importMapMtime
}
function generateImportMap() {
console.log('[predev-importmap] generating importMap with dummy S3_* envs...')
const env = {
...process.env,
NODE_OPTIONS: '--no-deprecation --conditions=react-server',
PAYLOAD_SECRET: process.env.PAYLOAD_SECRET || 'dev-secret',
DATABASE_URL: process.env.DATABASE_URL || 'postgresql://dev:dev@127.0.0.1:5432/teqo',
...DUMMY_S3_ENV,
}
try {
execFileSync('pnpm', ['exec', 'payload', 'generate:importmap'], {
cwd: repoRoot,
env,
stdio: 'inherit',
})
console.log('[predev-importmap] importMap generated successfully')
} catch (_error) {
console.error('[predev-importmap] failed to generate importMap')
process.exit(1)
}
}
if (needsRegeneration()) {
generateImportMap()
} else {
console.log('[predev-importmap] importMap is up to date')
}

View file

@ -1,29 +0,0 @@
import { readFileSync } from 'node:fs'
import path from 'node:path'
import { describe, expect, it } from 'vitest'
import {
S3_UPLOAD_HANDLER_IMPORT_PREFIX,
S3_UPLOAD_HANDLER_KEY,
} from '../../scripts/lib/importMapContract.mjs'
// The admin goes blank in production when this entry is missing: with the
// S3_* envs set, the media collection uses the s3Storage plugin and the admin
// config references this handler — getFromImportMap returns undefined, the
// Payload root never mounts, and the admin renders an empty page with zero
// console errors (class OPS69/OPS72/OPS73; orphaned again by 1c865bd8,
// 2026-08-23). Regenerating the importMap without the S3_* envs silently
// removes the entry — this pin fails the exact drift.
const importMapPath = path.resolve(__dirname, '../../src/app/(payload)/admin/importMap.js')
describe('importMap (admin)', () => {
it('keeps the S3ClientUploadHandler entry required by the prod S3 storage', () => {
const source = readFileSync(importMapPath, 'utf8')
// Import + map entry, like the generator emits (+2 lines in the file).
expect(source).toContain(S3_UPLOAD_HANDLER_IMPORT_PREFIX)
expect(source).toContain(`${S3_UPLOAD_HANDLER_KEY}": S3ClientUploadHandler_`)
})
})