🍕 Fix Kiln plugins under Vite: client env redirect, bootstrap order, init error reporting#252
Merged
Merged
Conversation
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 <cursoragent@cursor.com>
|
coverage: 87.084% (+0.2%) from 86.85% — jordan/kiln-init-diagnostics into master |
Ensure edit mode module scripts are ordered so bootstrap/env globals initialize before kiln plugin code executes. Co-authored-by: Cursor <cursoragent@cursor.com>
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 <cursoragent@cursor.com>
…fallback
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.<VAR> 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 <cursoragent@cursor.com>
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 <cursoragent@cursor.com>
jjpaulino
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Three related changes that make Kiln plugins (Pyxis image picker, Agora product picker, …) and other env-reading client code behave under the Vite pipeline the same way they did under Browserify:
lib/cmd/vite/scripts.js): defineprocess.env→window.process.envso client reads resolve to the runtime-hydrated object (.clay/_env-init.jspopulates it fromwindow.kiln.preloadData._envVars, gated byclient-env.json).process.env.NODE_ENVstays a build-time literal so guards still tree-shake. No build-time values are inlined.lib/cmd/vite/index.js): in edit mode, emit the bootstrap (env-init) module script before the kiln edit entry sowindow.process.envis hydrated before plugin code reads it.lib/cmd/vite/generate-kiln-edit.js): wrap the site Kiln plugin initializer in sync + async error handling and emit a structured error payload (name,message,stack, optionalcode/nestedcause) so edit-mode init failures are explicit instead of silent.Why
Under Vite,
@rollup/plugin-commonjsrewrites a CJS module's bareprocess.envinto a build-time-empty stub (literallyvar pVt = {}), soprocess.env.PYXIS_HOST || '…'always collapsed to the fallback — silently breaking every Kiln plugin / universal service that reads env. They worked under Browserify because itstransformEnvplugin rewritesprocess.env→window.process.env. This PR reproduces that behavior in Vite (a redirect, not an inline), ensures the env object is hydrated before plugins run, and surfaces any remaining init failures.Deliberately a redirect, not build-time inlining: the Docker image build is bare, and inlining from the build env would bake non-allowlisted secrets (e.g.
CORAL_GRAPHQL_TOKEN) into the public bundle. The runtime object is allowlist-gated, so client-env exposure stays identical to Browserify.Test plan
npx jest lib/cmd/vite— 14 suites, 193 tests passnpx eslinton all six changed files — cleanprocess.env.X(incl. non-allowlisted secrets) compiles towindow.process.env.X;NODE_ENVfolds to a literal; no secret literal appears in outputNote for reviewers / merge
The commit history is winding: an intermediate experiment added build-time env inlining (
Inline client process.env vars…,Match Browserify env handling…) that the final commit (Simplify Vite client env to a pure window.process.env redirect) removes entirely. Please squash-merge — the net diff is exactly the three changes above.Made with Cursor