From e56da4e35c91aea85c522f2da2e0fcc0bcfe3f21 Mon Sep 17 00:00:00 2001 From: Jordan Paulino Date: Wed, 27 May 2026 22:56:14 -0400 Subject: [PATCH 1/5] =?UTF-8?q?=F0=9F=8D=95=20Add=20verbose=20Kiln=20plugi?= =?UTF-8?q?n=20init=20diagnostics=20in=20Vite=20runtime?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Wrap site Kiln plugin initialization in structured sync/async error handling so edit-mode plugin boot failures are surfaced with clear error details instead of silently failing during startup. Co-authored-by: Cursor --- lib/cmd/vite/generate-kiln-edit.js | 41 +++++++++++++++++++++++++++++- lib/cmd/vite/generators.test.js | 4 ++- 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/lib/cmd/vite/generate-kiln-edit.js b/lib/cmd/vite/generate-kiln-edit.js index 7b2f65f..6188a27 100644 --- a/lib/cmd/vite/generate-kiln-edit.js +++ b/lib/cmd/vite/generate-kiln-edit.js @@ -70,6 +70,36 @@ async function generateViteKilnEditEntry() { // If neither is present, fall back to the namespace itself. lines.push(''); lines.push('function _resolveDefault(ns) { return (ns && ns.default !== undefined) ? ns.default : ns; }'); + lines.push('function _serializeKilnInitError(error) {'); + lines.push(' if (error == null) return { message: "Unknown error" };'); + lines.push(' if (typeof error === "string") return { message: error };'); + lines.push(' if (typeof error !== "object") return { message: String(error) };'); + lines.push(''); + lines.push(' var payload = {'); + lines.push(' name: error.name || null,'); + lines.push(' message: error.message || String(error),'); + lines.push(' stack: error.stack || null,'); + lines.push(' };'); + lines.push(''); + lines.push(' if (error.code != null) payload.code = error.code;'); + lines.push(' if (error.cause != null) payload.cause = _serializeKilnInitError(error.cause);'); + lines.push(''); + lines.push(' return payload;'); + lines.push('}'); + lines.push(''); + lines.push('function _reportKilnInitError(error) {'); + lines.push(' var payload = _serializeKilnInitError(error);'); + lines.push(' var message = payload.message || "Unknown error";'); + lines.push(' var header = "[clay vite] kiln plugin init error: " + message;'); + lines.push(''); + lines.push(' if (console.groupCollapsed) {'); + lines.push(' console.groupCollapsed(header);'); + lines.push(' console.error(payload);'); + lines.push(' console.groupEnd();'); + lines.push(' } else {'); + lines.push(' console.error(header, payload);'); + lines.push(' }'); + lines.push('}'); lines.push(''); lines.push('window.kiln = window.kiln || {};'); @@ -94,7 +124,16 @@ async function generateViteKilnEditEntry() { // Run site kiln plugin initializer if (hasKilnPlugin) { lines.push('var _initKilnPlugins = _resolveDefault(_kilnPluginNs);'); - lines.push('if (typeof _initKilnPlugins === "function") _initKilnPlugins();'); + lines.push('if (typeof _initKilnPlugins === "function") {'); + lines.push(' try {'); + lines.push(' var _kilnInitResult = _initKilnPlugins();'); + lines.push(' if (_kilnInitResult && typeof _kilnInitResult.then === "function") {'); + lines.push(' _kilnInitResult.catch(_reportKilnInitError);'); + lines.push(' }'); + lines.push(' } catch (error) {'); + lines.push(' _reportKilnInitError(error);'); + lines.push(' }'); + lines.push('}'); } await fs.ensureDir(CLAY_DIR); diff --git a/lib/cmd/vite/generators.test.js b/lib/cmd/vite/generators.test.js index caa3831..10ba2d5 100644 --- a/lib/cmd/vite/generators.test.js +++ b/lib/cmd/vite/generators.test.js @@ -169,7 +169,9 @@ describe('generate-vite env/bootstrap/kiln generators', () => { expect(content).toContain('import * as _kilnPluginNs from "../services/kiln/index.js";'); expect(content).toContain('window.kiln.componentModels["article"] = _resolveDefault(_m0);'); expect(content).toContain('window.kiln.componentKilnjs["article"] = _resolveDefault(_k0);'); - expect(content).toContain('if (typeof _initKilnPlugins === "function") _initKilnPlugins();'); + expect(content).toContain('function _reportKilnInitError(error) {'); + expect(content).toContain('var _kilnInitResult = _initKilnPlugins();'); + expect(content).toContain('_kilnInitResult.catch(_reportKilnInitError);'); }); it('does not import kiln plugin when services/kiln/index.js is missing', async () => { From a5471720a9791cd5ec830bd969eb47c208a34c8b Mon Sep 17 00:00:00 2001 From: Jordan Paulino Date: Thu, 28 May 2026 10:33:02 -0400 Subject: [PATCH 2/5] =?UTF-8?q?=F0=9F=8D=95=20Load=20Vite=20bootstrap=20be?= =?UTF-8?q?fore=20kiln=20edit=20entry.?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Ensure edit mode module scripts are ordered so bootstrap/env globals initialize before kiln plugin code executes. Co-authored-by: Cursor --- lib/cmd/vite/index.js | 6 ++++-- lib/cmd/vite/index.test.js | 6 ++++-- 2 files changed, 8 insertions(+), 4 deletions(-) diff --git a/lib/cmd/vite/index.js b/lib/cmd/vite/index.js index 13408db..a60a56c 100644 --- a/lib/cmd/vite/index.js +++ b/lib/cmd/vite/index.js @@ -95,7 +95,7 @@ function getEditScripts(assetPath) { * Populate media.moduleScripts and media.modulePreloads for amphora-html. * * In view mode: one bootstrap URL. - * In edit mode: kiln edit bundle + bootstrap. + * In edit mode: bootstrap + kiln edit bundle. * * @param {object} media * @param {string} assetPath @@ -113,7 +113,9 @@ function resolveModuleScripts(media, assetPath, options) { if (edit) { const editScripts = getEditScripts(assetPath); - media.moduleScripts = [...editScripts, ...viewScripts]; + // Keep bootstrap first so shared env/bootstrap globals are available + // before the kiln edit entry executes. + media.moduleScripts = [...viewScripts, ...editScripts]; media.modulePreloads = preloadEditBundle ? [...preloads, ...editScripts] : preloads; diff --git a/lib/cmd/vite/index.test.js b/lib/cmd/vite/index.test.js index 69806cc..eba7709 100644 --- a/lib/cmd/vite/index.test.js +++ b/lib/cmd/vite/index.test.js @@ -164,7 +164,7 @@ describe('index — view mode scripts', () => { // ── edit mode scripts ───────────────────────────────────────────────────────── describe('index — edit mode scripts', () => { - it('includes both kiln edit bundle and bootstrap in edit mode', () => { + it('includes bootstrap first, then kiln edit bundle in edit mode', () => { const { mod, cleanup } = withManifest({ [BOOTSTRAP_KEY]: { file: '/js/bootstrap-abc.js', imports: [] }, [KILN_EDIT_KEY]: { file: '/js/kiln-edit-xyz.js', imports: ['/js/vue-chunk.js'] }, @@ -174,8 +174,10 @@ describe('index — edit mode scripts', () => { mod.resolveModuleScripts(media, '', { edit: true }); cleanup(); - expect(media.moduleScripts).toContain('/js/kiln-edit-xyz.js'); expect(media.moduleScripts).toContain('/js/bootstrap-abc.js'); + expect(media.moduleScripts).toContain('/js/kiln-edit-xyz.js'); + expect(media.moduleScripts.indexOf('/js/bootstrap-abc.js')) + .toBeLessThan(media.moduleScripts.indexOf('/js/kiln-edit-xyz.js')); }); it('still serves view scripts when kiln entry is absent from manifest', () => { From a5d1696641f704706882d6ccb90e2f233da22ec6 Mon Sep 17 00:00:00 2001 From: Jordan Paulino Date: Thu, 28 May 2026 20:45:52 -0400 Subject: [PATCH 3/5] =?UTF-8?q?=F0=9F=8D=95=20Inline=20client=20process.en?= =?UTF-8?q?v=20vars=20into=20Vite=20bundle=20at=20build=20time?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CJS modules that read process.env.X at load time compile to an empty `var pVt = {}` stub under @rollup/plugin-commonjs, so reads like process.env.PYXIS_HOST resolve to undefined and never reach the runtime-hydrated window.process.env. This silently broke every Kiln plugin / universal service that reads env at module eval (pyxis, agora, glaze, …) — they worked in the legacy pipeline only because dotenv-webpack inlined those reads as string literals. Extend buildDefines() to inline client-facing process.env vars via Vite's define map, mirroring dotenv-webpack. The var list comes from client-env.json (the same browser surface amphora-html already forwards to window.process.env), with a cold-build fallback to all valid env keys so a fresh CI checkout still inlines. define only substitutes referenced tokens, so an over-inclusive list never leaks an unreferenced secret into the bundle. Add focused unit tests for inlining, the cold-build fallback, and the invalid-identifier filter. Co-authored-by: Cursor --- lib/cmd/vite/scripts.js | 92 ++++++++++++++++++++++++++++++++++++ lib/cmd/vite/scripts.test.js | 49 +++++++++++++++++++ 2 files changed, 141 insertions(+) diff --git a/lib/cmd/vite/scripts.js b/lib/cmd/vite/scripts.js index 223a56c..9307811 100644 --- a/lib/cmd/vite/scripts.js +++ b/lib/cmd/vite/scripts.js @@ -178,6 +178,92 @@ function getViteConfig(cliOptions = {}) { exports.getViteConfig = getViteConfig; +/** + * Build the `process.env.` define map that inlines client-facing env vars + * into the bundle at build time. + * + * ── Why this exists (Vite vs the legacy pipeline) ─────────────────────────── + * + * The legacy webpack `pack` pipeline used dotenv-webpack (DotenvPlugin with + * `systemvars: true`), which REPLACED every `process.env.FOO` reference with a + * string literal at build time. So a module-level `const HOST = process.env.FOO` + * captured a baked value that could never be undefined and was immune to any + * runtime mutation of `window.process.env`. + * + * Vite does not do this out of the box — only NODE_ENV and a few Node stubs are + * defined (see buildDefines). Every other `process.env.FOO` is left untouched, + * and @rollup/plugin-commonjs then rewrites the bare `process.env` of a CJS + * module into a build-time-empty local stub (literally `var pVt = {}`). So + * `const HOST = process.env.PYXIS_HOST || 'https://pyxis.nymag.com'` compiles to + * `pVt.PYXIS_HOST || 'https://pyxis.nymag.com'` → always the fallback. Crucially + * this stub is NOT `window.process.env`, so the runtime hydration in + * .clay/_env-init.js can never reach it — the read is dead at build time. This + * silently breaks every Kiln plugin / universal service that reads env at load + * time (pyxis.js, glaze/agora helpers, …): they work in the old pipeline but not + * under Vite. Only build-time inlining (define) can reach these reads. + * + * To restore the legacy behaviour we inline the SAME set of vars dotenv-webpack + * would have. The var list is sourced from client-env.json (the exact browser + * surface the clay-client-env collector found in the Rollup graph — also the + * list amphora-html forwards to window.process.env, so no new exposure), with a + * safe fallback for clean/CI builds (see buildClientEnvDefines). Values are read + * from the build process's own env (matching dotenv-webpack systemvars). + * + * Notes: + * - Only dot-access reads (`process.env.FOO`) are inlined — that covers the + * plugin/universal `const X = process.env.X` pattern. Whole-object use + * (`Object.assign({}, process.env)`) and dynamic keys are left to the existing + * runtime hydration (_env-init + window.process.env are untouched). + * - define only substitutes a value where the `process.env.FOO` token actually + * appears in compiled browser code, so an over-inclusive var list never leaks + * an unreferenced secret into the bundle. + * + * @returns {object} map of `process.env.` → JSON-stringified value + */ +function buildClientEnvDefines() { + const defines = {}; + + // Only [A-Za-z_][A-Za-z0-9_]* keys are valid as `process.env.` define + // members. Filters out npm-injected junk like `npm_package_dependencies_@babel/core`. + const isValidEnvName = name => /^[A-Za-z_][A-Za-z0-9_]*$/.test(name); + + // Source of truth for "which vars are browser-facing": + // + // 1. client-env.json — the exact set the clay-client-env collector found in + // the Rollup browser graph on the previous build. This is the same list + // amphora-html forwards to window.process.env, so inlining it adds no new + // exposure surface. Preferred when present (precise, minimal define map). + // + // 2. Cold build (no client-env.json — e.g. fresh CI checkout): fall back to + // every valid process.env key. This is SAFE because Vite's `define` only + // substitutes a value where the `process.env.` token actually appears + // in compiled browser code. A secret only lands in the bundle if browser + // code already references it — identical to the legacy dotenv-webpack + // pipeline and to the current runtime-forwarding behaviour. Without this + // fallback, the first/only build of a clean checkout would inline nothing + // and every env-reading plugin would break in production. + let names; + + try { + const parsed = JSON.parse(fs.readFileSync(path.join(CWD, 'client-env.json'), 'utf8')); + + names = Array.isArray(parsed) ? parsed : Object.keys(process.env); + } catch (_) { + names = Object.keys(process.env); + } + + for (const name of names) { + // NODE_ENV is handled explicitly in buildDefines; don't double-define it. + if (name === 'NODE_ENV' || !isValidEnvName(name)) continue; + + const value = process.env[name]; + + defines[`process.env.${name}`] = JSON.stringify(value === undefined ? '' : value); + } + + return defines; +} + /** * Build the define map injected into every module at compile time. * @@ -222,10 +308,16 @@ function buildDefines(userDefines = {}) { // of window. globalThis is universally available in ES2017+ environments. global: 'globalThis', }, + // Inline client-facing process.env. reads at build time, mirroring the + // legacy dotenv-webpack pipeline. Must come before userDefines so a site + // can still override a specific var via bundlerConfig().define if needed. + buildClientEnvDefines(), userDefines ); } +exports.buildDefines = buildDefines; + /** * Assemble the Vite plugin array for a build pass. * diff --git a/lib/cmd/vite/scripts.test.js b/lib/cmd/vite/scripts.test.js index 7d4e07e..8c22675 100644 --- a/lib/cmd/vite/scripts.test.js +++ b/lib/cmd/vite/scripts.test.js @@ -80,6 +80,55 @@ describe('vite scripts', () => { expect(cfg.sourcemap).toBe(true); }); + it('buildDefines inlines client-env vars from client-env.json at build time', async () => { + await setupTmp('claycli-vite-scripts-clientenv-'); + await fs.writeJson(path.join(tmpDir, 'client-env.json'), ['PYXIS_HOST', 'NODE_ENV']); + + const prev = process.env.PYXIS_HOST; + + process.env.PYXIS_HOST = 'https://pyxis.example.test'; + + try { + const { buildDefines } = require('./scripts'); + const define = buildDefines(); + + // PYXIS_HOST is inlined as a string literal (matches dotenv-webpack), so a + // CJS `const HOST = process.env.PYXIS_HOST` can never compile to the empty + // pVt={} stub the commonjs transform would otherwise produce. + expect(define['process.env.PYXIS_HOST']).toBe(JSON.stringify('https://pyxis.example.test')); + // NODE_ENV stays owned by buildDefines (not double-defined via client-env). + expect(define['process.env.NODE_ENV']).toBeDefined(); + } finally { + if (prev === undefined) delete process.env.PYXIS_HOST; + else process.env.PYXIS_HOST = prev; + } + }); + + it('buildDefines falls back to all env on a cold build and skips invalid identifiers', async () => { + await setupTmp('claycli-vite-scripts-coldenv-'); + // No client-env.json on a fresh CI checkout — must still inline so plugins + // are not broken in production. + + const prev = process.env.CLAY_TEST_COLD_ENV; + + process.env.CLAY_TEST_COLD_ENV = 'cold-value'; + process.env['clay.invalid-key'] = 'ignored'; + + try { + const { buildDefines } = require('./scripts'); + const define = buildDefines(); + + expect(define['process.env.CLAY_TEST_COLD_ENV']).toBe(JSON.stringify('cold-value')); + // Keys that are not valid `process.env.` members are skipped so + // esbuild's define parser never sees a malformed expression. + expect(define['process.env.clay.invalid-key']).toBeUndefined(); + } finally { + delete process.env['clay.invalid-key']; + if (prev === undefined) delete process.env.CLAY_TEST_COLD_ENV; + else process.env.CLAY_TEST_COLD_ENV = prev; + } + }); + it('buildJS runs split builds, writes manifest and client-env', async () => { await setupTmp('claycli-vite-scripts-build-'); From fc7738d8e786e51493a53eb816d3bceaf651378c Mon Sep 17 00:00:00 2001 From: Jordan Paulino Date: Thu, 28 May 2026 22:34:17 -0400 Subject: [PATCH 4/5] =?UTF-8?q?=F0=9F=8D=95=20Match=20Browserify=20env=20h?= =?UTF-8?q?andling=20in=20Vite:=20inline=20build=20vars=20+=20runtime=20fa?= =?UTF-8?q?llback?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit buildClientEnvDefines now reproduces the legacy dotenv-webpack ({ path: './.env', systemvars: true }) behaviour and adds a runtime safety net so the Vite pipeline stops regressing where the build env is bare: - Layer 1 (Browserify parity): overlay ./.env with the build process env and inline every NON-EMPTY process.env. as a string literal — exactly what the old pack pipeline shipped (a local container build started with env_file bakes the real values in). - Layer 2 (runtime catch-all): define process.env -> window.process.env so any var the build can't see (the bare Docker image build) resolves to the object .clay/_env-init.js hydrates from window.kiln.preloadData._envVars instead of a pVt={} stub. Fixes the Vite-only image/product picker break: the prior build-time inline baked "" for client vars (the Docker build has no PYXIS_HOST/AGORA_HOST), and master's `window.process.env.X = process.env.X` then clobbered the hydrated value with "". Empty/absent vars now fall through to the runtime read, turning that line into a harmless self-assign. process.env.NODE_ENV stays a literal so guards still tree-shake. Co-authored-by: Cursor --- lib/cmd/vite/scripts.js | 152 +++++++++++++++++++---------------- lib/cmd/vite/scripts.test.js | 60 +++++++++----- 2 files changed, 123 insertions(+), 89 deletions(-) diff --git a/lib/cmd/vite/scripts.js b/lib/cmd/vite/scripts.js index 9307811..596cf82 100644 --- a/lib/cmd/vite/scripts.js +++ b/lib/cmd/vite/scripts.js @@ -179,86 +179,100 @@ function getViteConfig(cliOptions = {}) { exports.getViteConfig = getViteConfig; /** - * Build the `process.env.` define map that inlines client-facing env vars - * into the bundle at build time. + * Minimal dependency-free `.env` reader (KEY=VALUE, one per line). * - * ── Why this exists (Vite vs the legacy pipeline) ─────────────────────────── + * Mirrors dotenv for the simple single-line values client code relies on + * (hosts, keys, flags). Multi-line / quoted-newline values are not client + * surface and are irrelevant here. Returns {} when the file is absent — the + * normal case inside the Docker image build, where .env is git/dockerignored. * - * The legacy webpack `pack` pipeline used dotenv-webpack (DotenvPlugin with - * `systemvars: true`), which REPLACED every `process.env.FOO` reference with a - * string literal at build time. So a module-level `const HOST = process.env.FOO` - * captured a baked value that could never be undefined and was immune to any - * runtime mutation of `window.process.env`. + * @param {string} filePath absolute path to a .env file + * @returns {object} parsed KEY→value map + */ +function readDotenvFile(filePath) { + const out = {}; + + let raw; + + try { + raw = fs.readFileSync(filePath, 'utf8'); + } catch (_) { + return out; + } + + for (const line of raw.split('\n')) { + const match = line.match(/^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=\s*(.*?)\s*$/); + + if (!match) continue; + + // Strip a single layer of matching surrounding quotes, mirroring dotenv. + out[match[1]] = match[2].replace(/^(['"])([\s\S]*)\1$/, '$2'); + } + + return out; +} + +/** + * Build the `process.env.` define map. + * + * Reproduces the legacy webpack `pack` behaviour — dotenv-webpack with + * `{ path: './.env', systemvars: true, allowEmptyValues: true }` — which INLINED + * every `process.env.FOO` as a string literal at build time, and adds a runtime + * safety net so the Vite pipeline never regresses where the build env is bare. + * + * ── Why this is needed (Vite vs the legacy pipeline) ──────────────────────── + * + * Vite does not inline arbitrary env, and @rollup/plugin-commonjs rewrites the + * bare `process.env` of a CJS module into a build-time-empty local stub + * (literally `var pVt = {}`). So `const HOST = process.env.PYXIS_HOST || '…'` + * compiles to `pVt.PYXIS_HOST || '…'` → always the fallback, and the read is dead + * at build time. This silently breaks every Kiln plugin / universal service that + * reads env (pyxis.js, glaze/agora helpers, …): they work in Browserify but not + * under Vite. * - * Vite does not do this out of the box — only NODE_ENV and a few Node stubs are - * defined (see buildDefines). Every other `process.env.FOO` is left untouched, - * and @rollup/plugin-commonjs then rewrites the bare `process.env` of a CJS - * module into a build-time-empty local stub (literally `var pVt = {}`). So - * `const HOST = process.env.PYXIS_HOST || 'https://pyxis.nymag.com'` compiles to - * `pVt.PYXIS_HOST || 'https://pyxis.nymag.com'` → always the fallback. Crucially - * this stub is NOT `window.process.env`, so the runtime hydration in - * .clay/_env-init.js can never reach it — the read is dead at build time. This - * silently breaks every Kiln plugin / universal service that reads env at load - * time (pyxis.js, glaze/agora helpers, …): they work in the old pipeline but not - * under Vite. Only build-time inlining (define) can reach these reads. + * ── Two layers, in precedence order (most-specific define wins in esbuild) ── * - * To restore the legacy behaviour we inline the SAME set of vars dotenv-webpack - * would have. The var list is sourced from client-env.json (the exact browser - * surface the clay-client-env collector found in the Rollup graph — also the - * list amphora-html forwards to window.process.env, so no new exposure), with a - * safe fallback for clean/CI builds (see buildClientEnvDefines). Values are read - * from the build process's own env (matching dotenv-webpack systemvars). + * 1. INLINE (Browserify parity): overlay ./.env with the build `process.env` + * (systemvars) and bake every NON-EMPTY value in as a string literal. Where + * the build actually holds a value — a local container started with + * `env_file`, or any build run with .env in cwd — the value is inlined exactly + * as dotenv-webpack did. NOTE: like dotenv-webpack systemvars, this exposes + * any referenced var (secrets included) in the browser bundle. * - * Notes: - * - Only dot-access reads (`process.env.FOO`) are inlined — that covers the - * plugin/universal `const X = process.env.X` pattern. Whole-object use - * (`Object.assign({}, process.env)`) and dynamic keys are left to the existing - * runtime hydration (_env-init + window.process.env are untouched). - * - define only substitutes a value where the `process.env.FOO` token actually - * appears in compiled browser code, so an over-inclusive var list never leaks - * an unreferenced secret into the bundle. + * 2. RUNTIME catch-all: `process.env` → `window.process.env`. Any var NOT + * inlined (empty at build, or the bare Docker image build) is rewritten to a + * live read of the object .clay/_env-init.js hydrates from + * `window.kiln.preloadData._envVars` (amphora-html forwards the server's live + * env in edit mode). This also means master's + * `window.process.env.X = process.env.X` becomes a harmless self-assign + * instead of clobbering the hydrated value with "". * - * @returns {object} map of `process.env.` → JSON-stringified value + * `process.env.NODE_ENV` is kept as a build-time literal by buildDefines (a + * more-specific key, so it wins over the catch-all) → `process.env.NODE_ENV === + * 'production'` still tree-shakes. + * + * @returns {object} define map (per-var literals + the `process.env` catch-all) */ function buildClientEnvDefines() { - const defines = {}; - - // Only [A-Za-z_][A-Za-z0-9_]* keys are valid as `process.env.` define - // members. Filters out npm-injected junk like `npm_package_dependencies_@babel/core`. const isValidEnvName = name => /^[A-Za-z_][A-Za-z0-9_]*$/.test(name); - // Source of truth for "which vars are browser-facing": - // - // 1. client-env.json — the exact set the clay-client-env collector found in - // the Rollup browser graph on the previous build. This is the same list - // amphora-html forwards to window.process.env, so inlining it adds no new - // exposure surface. Preferred when present (precise, minimal define map). - // - // 2. Cold build (no client-env.json — e.g. fresh CI checkout): fall back to - // every valid process.env key. This is SAFE because Vite's `define` only - // substitutes a value where the `process.env.` token actually appears - // in compiled browser code. A secret only lands in the bundle if browser - // code already references it — identical to the legacy dotenv-webpack - // pipeline and to the current runtime-forwarding behaviour. Without this - // fallback, the first/only build of a clean checkout would inline nothing - // and every env-reading plugin would break in production. - let names; - - try { - const parsed = JSON.parse(fs.readFileSync(path.join(CWD, 'client-env.json'), 'utf8')); + // Layer 2: runtime catch-all for anything not inlined below. + const defines = { 'process.env': 'window.process.env' }; - names = Array.isArray(parsed) ? parsed : Object.keys(process.env); - } catch (_) { - names = Object.keys(process.env); - } + // Layer 1: dotenv-webpack-style inline (./.env overlaid by systemvars/process.env). + const env = Object.assign(readDotenvFile(path.join(CWD, '.env')), process.env); - for (const name of names) { - // NODE_ENV is handled explicitly in buildDefines; don't double-define it. + for (const name of Object.keys(env)) { + // NODE_ENV is owned by buildDefines; invalid identifiers would break esbuild's + // define parser (e.g. npm_package_dependencies_@babel/core). if (name === 'NODE_ENV' || !isValidEnvName(name)) continue; - const value = process.env[name]; + const value = env[name]; + + // Empty/undefined → leave to the runtime catch-all; never inline a clobbering "". + if (value === undefined || value === '') continue; - defines[`process.env.${name}`] = JSON.stringify(value === undefined ? '' : value); + defines[`process.env.${name}`] = JSON.stringify(value); } return defines; @@ -308,9 +322,11 @@ function buildDefines(userDefines = {}) { // of window. globalThis is universally available in ES2017+ environments. global: 'globalThis', }, - // Inline client-facing process.env. reads at build time, mirroring the - // legacy dotenv-webpack pipeline. Must come before userDefines so a site - // can still override a specific var via bundlerConfig().define if needed. + // Client env (see buildClientEnvDefines): per-var literals inlined from the + // build env (dotenv-webpack parity) + a `process.env` → `window.process.env` + // catch-all for anything the build can't see. `process.env.NODE_ENV` above is + // more specific and still wins as a literal. Comes before userDefines so a + // site can still override a specific var via bundlerConfig().define if needed. buildClientEnvDefines(), userDefines ); diff --git a/lib/cmd/vite/scripts.test.js b/lib/cmd/vite/scripts.test.js index 8c22675..8f88358 100644 --- a/lib/cmd/vite/scripts.test.js +++ b/lib/cmd/vite/scripts.test.js @@ -80,52 +80,70 @@ describe('vite scripts', () => { expect(cfg.sourcemap).toBe(true); }); - it('buildDefines inlines client-env vars from client-env.json at build time', async () => { + it('buildDefines inlines a non-empty build env var as a literal (dotenv-webpack parity)', async () => { await setupTmp('claycli-vite-scripts-clientenv-'); - await fs.writeJson(path.join(tmpDir, 'client-env.json'), ['PYXIS_HOST', 'NODE_ENV']); const prev = process.env.PYXIS_HOST; + // When the build env holds a value (e.g. a local container started with + // env_file), it is baked in as a string literal — exactly as the legacy + // dotenv-webpack({ systemvars: true }) pipeline did. process.env.PYXIS_HOST = 'https://pyxis.example.test'; try { const { buildDefines } = require('./scripts'); const define = buildDefines(); - // PYXIS_HOST is inlined as a string literal (matches dotenv-webpack), so a - // CJS `const HOST = process.env.PYXIS_HOST` can never compile to the empty - // pVt={} stub the commonjs transform would otherwise produce. expect(define['process.env.PYXIS_HOST']).toBe(JSON.stringify('https://pyxis.example.test')); - // NODE_ENV stays owned by buildDefines (not double-defined via client-env). - expect(define['process.env.NODE_ENV']).toBeDefined(); + // The runtime catch-all is always present for vars the build can't see. + expect(define['process.env']).toBe('window.process.env'); } finally { if (prev === undefined) delete process.env.PYXIS_HOST; else process.env.PYXIS_HOST = prev; } }); - it('buildDefines falls back to all env on a cold build and skips invalid identifiers', async () => { - await setupTmp('claycli-vite-scripts-coldenv-'); - // No client-env.json on a fresh CI checkout — must still inline so plugins - // are not broken in production. + it('buildDefines leaves an empty/absent build env var to the runtime catch-all (no clobber)', async () => { + await setupTmp('claycli-vite-scripts-clientenv-empty-'); - const prev = process.env.CLAY_TEST_COLD_ENV; + const prev = process.env.PYXIS_HOST; + + // An empty build value must NOT be inlined — otherwise `process.env.PYXIS_HOST` + // folds to "" and master's `window.process.env.X = process.env.X` would clobber + // the value _env-init.js hydrated at runtime. It must fall through to the + // `process.env` → `window.process.env` catch-all instead. + process.env.PYXIS_HOST = ''; + + try { + const { buildDefines } = require('./scripts'); + const define = buildDefines(); + + expect(define['process.env.PYXIS_HOST']).toBeUndefined(); + expect(define['process.env']).toBe('window.process.env'); + } finally { + if (prev === undefined) delete process.env.PYXIS_HOST; + else process.env.PYXIS_HOST = prev; + } + }); + + it('buildDefines keeps NODE_ENV a build-time literal so guards still tree-shake', async () => { + await setupTmp('claycli-vite-scripts-nodeenv-'); + + const prev = process.env.NODE_ENV; - process.env.CLAY_TEST_COLD_ENV = 'cold-value'; - process.env['clay.invalid-key'] = 'ignored'; + process.env.NODE_ENV = 'production'; try { const { buildDefines } = require('./scripts'); const define = buildDefines(); - expect(define['process.env.CLAY_TEST_COLD_ENV']).toBe(JSON.stringify('cold-value')); - // Keys that are not valid `process.env.` members are skipped so - // esbuild's define parser never sees a malformed expression. - expect(define['process.env.clay.invalid-key']).toBeUndefined(); + // The more-specific `process.env.NODE_ENV` key wins over `process.env` in + // esbuild, so `process.env.NODE_ENV === 'production'` folds to a literal. + expect(define['process.env.NODE_ENV']).toBe(JSON.stringify('production')); + expect(define['process.env']).toBe('window.process.env'); } finally { - delete process.env['clay.invalid-key']; - if (prev === undefined) delete process.env.CLAY_TEST_COLD_ENV; - else process.env.CLAY_TEST_COLD_ENV = prev; + if (prev === undefined) delete process.env.NODE_ENV; + else process.env.NODE_ENV = prev; } }); From 2ed3c48e8c73fc6d458b54de1ad3f990b4cb0fbc Mon Sep 17 00:00:00 2001 From: Jordan Paulino Date: Thu, 28 May 2026 23:10:21 -0400 Subject: [PATCH 5/5] =?UTF-8?q?=F0=9F=8D=95=20Simplify=20Vite=20client=20e?= =?UTF-8?q?nv=20to=20a=20pure=20window.process.env=20redirect?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Drop the build-time inlining layer (and the .env reader) that was added for the Browserify-parity experiment. The Docker image build is bare, so inlining never contributed in prod; the runtime redirect (process.env -> window.process.env, hydrated from preloadData._envVars and gated by client-env.json) is what actually fixes the Kiln pickers under Vite. This matches the legacy Browserify transformEnv plugin (lib/cmd/compile/scripts.js) exactly — it rewrites process.env to window.process.env and inlines nothing — and removes the latent risk of baking non-allowlisted secrets (e.g. CORAL_GRAPHQL_TOKEN) into the public bundle on any env-present build. process.env.NODE_ENV stays a build-time literal so guards still tree-shake. Co-authored-by: Cursor --- lib/cmd/vite/scripts.js | 119 ++++++++++------------------------- lib/cmd/vite/scripts.test.js | 37 +++-------- 2 files changed, 42 insertions(+), 114 deletions(-) diff --git a/lib/cmd/vite/scripts.js b/lib/cmd/vite/scripts.js index 596cf82..646ceeb 100644 --- a/lib/cmd/vite/scripts.js +++ b/lib/cmd/vite/scripts.js @@ -179,103 +179,51 @@ function getViteConfig(cliOptions = {}) { exports.getViteConfig = getViteConfig; /** - * Minimal dependency-free `.env` reader (KEY=VALUE, one per line). + * Build the `process.env` define that redirects client env reads to the + * RUNTIME-hydrated `window.process.env`. * - * Mirrors dotenv for the simple single-line values client code relies on - * (hosts, keys, flags). Multi-line / quoted-newline values are not client - * surface and are irrelevant here. Returns {} when the file is absent — the - * normal case inside the Docker image build, where .env is git/dockerignored. + * This is the Vite equivalent of the legacy Browserify pipeline's transformEnv + * plugin (lib/cmd/compile/scripts.js), which rewrites every `process.env` in + * client code to `window.process.env` and inlines NO values ("reference window, + * so browserify doesn't bundle in `process`"). * - * @param {string} filePath absolute path to a .env file - * @returns {object} parsed KEY→value map - */ -function readDotenvFile(filePath) { - const out = {}; - - let raw; - - try { - raw = fs.readFileSync(filePath, 'utf8'); - } catch (_) { - return out; - } - - for (const line of raw.split('\n')) { - const match = line.match(/^\s*(?:export\s+)?([A-Za-z_][A-Za-z0-9_]*)\s*=\s*(.*?)\s*$/); - - if (!match) continue; - - // Strip a single layer of matching surrounding quotes, mirroring dotenv. - out[match[1]] = match[2].replace(/^(['"])([\s\S]*)\1$/, '$2'); - } - - return out; -} - -/** - * Build the `process.env.` define map. - * - * Reproduces the legacy webpack `pack` behaviour — dotenv-webpack with - * `{ path: './.env', systemvars: true, allowEmptyValues: true }` — which INLINED - * every `process.env.FOO` as a string literal at build time, and adds a runtime - * safety net so the Vite pipeline never regresses where the build env is bare. - * - * ── Why this is needed (Vite vs the legacy pipeline) ──────────────────────── + * ── Why a redirect, not an inline ─────────────────────────────────────────── * * Vite does not inline arbitrary env, and @rollup/plugin-commonjs rewrites the * bare `process.env` of a CJS module into a build-time-empty local stub * (literally `var pVt = {}`). So `const HOST = process.env.PYXIS_HOST || '…'` * compiles to `pVt.PYXIS_HOST || '…'` → always the fallback, and the read is dead - * at build time. This silently breaks every Kiln plugin / universal service that + * at build time. That silently breaks every Kiln plugin / universal service that * reads env (pyxis.js, glaze/agora helpers, …): they work in Browserify but not * under Vite. * - * ── Two layers, in precedence order (most-specific define wins in esbuild) ── - * - * 1. INLINE (Browserify parity): overlay ./.env with the build `process.env` - * (systemvars) and bake every NON-EMPTY value in as a string literal. Where - * the build actually holds a value — a local container started with - * `env_file`, or any build run with .env in cwd — the value is inlined exactly - * as dotenv-webpack did. NOTE: like dotenv-webpack systemvars, this exposes - * any referenced var (secrets included) in the browser bundle. + * Defining `process.env` → `window.process.env` makes `process.env.PYXIS_HOST` + * compile to `window.process.env.PYXIS_HOST` — a live read of the object + * .clay/_env-init.js hydrates from `window.kiln.preloadData._envVars` (amphora-html + * forwards the server's live env in edit mode, gated by client-env.json). It also + * turns master's `window.process.env.X = process.env.X` plugin line into a harmless + * self-assign instead of clobbering the hydrated value with "". * - * 2. RUNTIME catch-all: `process.env` → `window.process.env`. Any var NOT - * inlined (empty at build, or the bare Docker image build) is rewritten to a - * live read of the object .clay/_env-init.js hydrates from - * `window.kiln.preloadData._envVars` (amphora-html forwards the server's live - * env in edit mode). This also means master's - * `window.process.env.X = process.env.X` becomes a harmless self-assign - * instead of clobbering the hydrated value with "". + * We deliberately do NOT inline values from the build env. The Docker image build + * is bare (only CLAYCLI_* args), so there is nothing to inline in prod anyway; and + * inlining from the full build `process.env` would bake any referenced var — + * including secrets that are NOT on the client-env.json allowlist (e.g. + * CORAL_GRAPHQL_TOKEN) — into the public bundle on any env-present build (local + * `make assets`, future build-args, …). The runtime object is allowlist-gated, so + * a pure redirect keeps Vite's client-env exposure identical to Browserify's. * * `process.env.NODE_ENV` is kept as a build-time literal by buildDefines (a - * more-specific key, so it wins over the catch-all) → `process.env.NODE_ENV === + * more-specific key, so it wins over this catch-all) → `process.env.NODE_ENV === * 'production'` still tree-shakes. * - * @returns {object} define map (per-var literals + the `process.env` catch-all) + * @returns {object} `{ 'process.env': 'window.process.env' }` */ function buildClientEnvDefines() { - const isValidEnvName = name => /^[A-Za-z_][A-Za-z0-9_]*$/.test(name); - - // Layer 2: runtime catch-all for anything not inlined below. - const defines = { 'process.env': 'window.process.env' }; - - // Layer 1: dotenv-webpack-style inline (./.env overlaid by systemvars/process.env). - const env = Object.assign(readDotenvFile(path.join(CWD, '.env')), process.env); - - for (const name of Object.keys(env)) { - // NODE_ENV is owned by buildDefines; invalid identifiers would break esbuild's - // define parser (e.g. npm_package_dependencies_@babel/core). - if (name === 'NODE_ENV' || !isValidEnvName(name)) continue; - - const value = env[name]; - - // Empty/undefined → leave to the runtime catch-all; never inline a clobbering "". - if (value === undefined || value === '') continue; - - defines[`process.env.${name}`] = JSON.stringify(value); - } - - return defines; + // A member-expression value (not a JSON literal) tells esbuild/Vite to REWRITE + // the `process.env` token to `window.process.env` rather than substitute a + // constant. `window.process.env` on the left-hand side of an assignment is left + // alone (its `process` is a member of `window`, not the free `process` token). + return { 'process.env': 'window.process.env' }; } /** @@ -322,11 +270,12 @@ function buildDefines(userDefines = {}) { // of window. globalThis is universally available in ES2017+ environments. global: 'globalThis', }, - // Client env (see buildClientEnvDefines): per-var literals inlined from the - // build env (dotenv-webpack parity) + a `process.env` → `window.process.env` - // catch-all for anything the build can't see. `process.env.NODE_ENV` above is - // more specific and still wins as a literal. Comes before userDefines so a - // site can still override a specific var via bundlerConfig().define if needed. + // Client env (see buildClientEnvDefines): redirect `process.env` → + // `window.process.env` so reads resolve to the runtime-hydrated object + // (.clay/_env-init.js), matching the legacy Browserify transformEnv plugin — + // no build-time values are inlined. `process.env.NODE_ENV` above is more + // specific and still wins as a literal. Comes before userDefines so a site + // can still override a specific var via bundlerConfig().define if needed. buildClientEnvDefines(), userDefines ); diff --git a/lib/cmd/vite/scripts.test.js b/lib/cmd/vite/scripts.test.js index 8f88358..6bec020 100644 --- a/lib/cmd/vite/scripts.test.js +++ b/lib/cmd/vite/scripts.test.js @@ -80,46 +80,25 @@ describe('vite scripts', () => { expect(cfg.sourcemap).toBe(true); }); - it('buildDefines inlines a non-empty build env var as a literal (dotenv-webpack parity)', async () => { + it('buildDefines redirects process.env to window.process.env and never inlines build env (Browserify parity)', async () => { await setupTmp('claycli-vite-scripts-clientenv-'); const prev = process.env.PYXIS_HOST; - // When the build env holds a value (e.g. a local container started with - // env_file), it is baked in as a string literal — exactly as the legacy - // dotenv-webpack({ systemvars: true }) pipeline did. - process.env.PYXIS_HOST = 'https://pyxis.example.test'; + // Even when the build env holds a value, it must NOT be baked in: the Docker + // image build is bare, and inlining from the full build env would leak vars + // outside the client-env.json allowlist into the public bundle. Reads resolve + // at runtime via window.process.env (hydrated by .clay/_env-init.js), exactly + // like the legacy Browserify transformEnv plugin. This also turns master's + // `window.process.env.X = process.env.X` into a harmless self-assign. + process.env.PYXIS_HOST = 'https://build-env-should-not-be-inlined.test'; try { const { buildDefines } = require('./scripts'); const define = buildDefines(); - expect(define['process.env.PYXIS_HOST']).toBe(JSON.stringify('https://pyxis.example.test')); - // The runtime catch-all is always present for vars the build can't see. expect(define['process.env']).toBe('window.process.env'); - } finally { - if (prev === undefined) delete process.env.PYXIS_HOST; - else process.env.PYXIS_HOST = prev; - } - }); - - it('buildDefines leaves an empty/absent build env var to the runtime catch-all (no clobber)', async () => { - await setupTmp('claycli-vite-scripts-clientenv-empty-'); - - const prev = process.env.PYXIS_HOST; - - // An empty build value must NOT be inlined — otherwise `process.env.PYXIS_HOST` - // folds to "" and master's `window.process.env.X = process.env.X` would clobber - // the value _env-init.js hydrated at runtime. It must fall through to the - // `process.env` → `window.process.env` catch-all instead. - process.env.PYXIS_HOST = ''; - - try { - const { buildDefines } = require('./scripts'); - const define = buildDefines(); - expect(define['process.env.PYXIS_HOST']).toBeUndefined(); - expect(define['process.env']).toBe('window.process.env'); } finally { if (prev === undefined) delete process.env.PYXIS_HOST; else process.env.PYXIS_HOST = prev;