From 2cda9a0fd8584604848271e2e1c58499f37ed4b5 Mon Sep 17 00:00:00 2001 From: Jordan Paulino Date: Tue, 15 Sep 2026 15:03:06 -0400 Subject: [PATCH] =?UTF-8?q?=F0=9F=8D=95=20Document=20the=20nodeGlobals=20/?= =?UTF-8?q?=20serverOnlyPackages=20config=20landed=20in=20#260?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit #260's console output (scanBareNodeGlobals's end-of-build summary) already pointed readers at "CLAY-VITE.md § Node globals in browser bundles" -- that section never actually existed. Adds it: the v5 insert-module-globals history (so "why doesn't Vite just polyfill this" isn't re-litigated), the no-blanket-polyfill policy, the bare-Buffer diagnostic, the nodeGlobals contract and its two modes, the browser-native (btoa) exit path that motivated this in the first place, and serverOnlyPackages as the more general shape of the same leak. Placed directly after "Kiln plugin PostCSS chain" under Section 8, matching that section's own established depth and voice. Docs only -- no .js file touched. npm test (lint + 456 tests) green. Co-Authored-By: Claude Sonnet 5 --- CLAY-VITE.md | 111 +++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 111 insertions(+) diff --git a/CLAY-VITE.md b/CLAY-VITE.md index 84ad042..d116272 100644 --- a/CLAY-VITE.md +++ b/CLAY-VITE.md @@ -823,6 +823,117 @@ build pipeline with its own post-processing step over `public/css` (asset hashin must run after the *last* claycli invocation that includes `js`, or its own changes to this specific file are silently overwritten on the next one. +### Node globals in browser bundles + +#### Why this needs its own explanation + +The legacy Browserify pipeline polyfilled Node globals for free. Browserify's bundler runs every +module through `insert-module-globals`, which detects a bare reference to `Buffer`, `process`, or +`global` and rewrites it into an explicit `require('buffer').Buffer`-style import, pulling in the +[feross/buffer](https://www.npmjs.com/package/buffer) polyfill automatically. A component's +`Buffer.from(...)` — written with no accompanying `require('buffer')` at all — just worked, silently. + +`clay vite` has no equivalent. Vite/Rollup never rewrites a bare identifier into an import; a +module that references `Buffer` without importing it compiles cleanly and then throws +`ReferenceError: Buffer is not defined` the first time that code path actually runs in the browser +— with zero build-time signal. `browser-compat.js`'s stubs (see the Vite plugins table above) +don't help here either: they only intercept an *explicit* `require('buffer')`/`import 'buffer'`. +They have nothing to intercept when there is no import at all. + +#### The policy: no blanket polyfilling + +The obvious "fix" — reintroduce something like `insert-module-globals` — is deliberately not on the +table. Universal, automatic Node-global injection is exactly what made Browserify bundles carry +megabytes of dead weight for code paths that only ever ran on the server; a bundle that pulls in a +full `Buffer` polyfill because one file references it as a bare identifier reintroduces the same +bloat this migration exists to remove. The fix here is visibility plus a narrow, explicit opt-in — +never an automatic one. + +#### The diagnostic + +Every Vite build runs a `transform` hook (`lib/cmd/vite/scripts.js`, `scanBareNodeGlobals` / +`viteBareNodeGlobalsPlugin`) that scans first-party source (skipping `node_modules` and virtual +module ids) for `Buffer` used as a free identifier — a whole-word match that isn't a property-access +target (`foo.Buffer`) and isn't already covered by an explicit `require('buffer')`/`import` in the +same file. Every hit is collected and printed once, at the end of the build, as a single summary: + +``` +[clay vite] 1 Node global(s) referenced as bare identifiers in browser-reachable code +(no polyfill is provided — see CLAY-VITE.md § Node globals in browser bundles): + Buffer: + - components/podcast-transcripts/model.js +``` + +This is deliberately scoped to `Buffer` only, not `process`/`global`. Both of those are already +substituted at build time by `buildDefines()` for their common legitimate forms — +`process.env.*`, `process.browser`, `process.version(s)`, and `global → globalThis` — so scanning +for them as bare identifiers would flag mostly correct, already-handled code. And it's a regex, not +an AST walk: it can't distinguish a free identifier from a property name (`{ Buffer: 1 }`), an +accepted trade-off against adding a parser dependency for a report-only diagnostic. + +#### The fix: `nodeGlobals` in `bundlerConfig()` + +Once a bare reference is named, `bundlerConfig().nodeGlobals` is the narrow, explicit, opt-in fix — +set in `claycli.config.js`, alongside `alias`/`define`/`browserStubs`: + +```js +// claycli.config.js +module.exports = { + bundlerConfig(config) { + config.nodeGlobals = { Buffer: true }; + return config; + }, +}; +``` + +| Value | Effect | +|---|---| +| unset / `false` (default) | No plugin added. Zero bytes, zero behavior change. | +| `true` | Adds `@rollup/plugin-inject`, which auto-imports `Buffer` from `'buffer'` — but only into modules that actually reference the free identifier. Resolves through the existing `browser-compat.js` stub, which already prefers a real `globalThis.Buffer` when the page provides one. | +| `'polyfill'` | Same auto-import, but also aliases the `'buffer'` specifier to the real npm `buffer` package instead of the minimal stub, for byte-capable operations (e.g. `createHmac`) the stub can't support. Pulls in real bytes — use only where the minimal stub genuinely isn't enough. | + +This mirrors `lenientBrowserExternalize`'s shape elsewhere in this pipeline: a bounded, documented, +default-off bridge with a stated exit, not a new permanent behavior. The exit path here is the same +one that applies to any Node-only code reachable from the browser: replace the Node-specific call +with a browser-native equivalent. The single most common case — base64-encoding a string, which is +what motivated this section — has a two-line browser-native replacement that needs neither the stub +nor `nodeGlobals`: + +```js +// Node-only, throws in the browser without nodeGlobals: +const encoded = Buffer.from(str, 'utf8').toString('base64'); + +// Browser-native, works everywhere, no config needed: +const encoded = btoa(str); +// or, for non-Latin1 strings: btoa(String.fromCharCode(...new TextEncoder().encode(str))) +``` + +#### `serverOnlyPackages`: the related, more general leak + +A bare `Buffer` reference is one *symptom* of a broader shape: server-only code reachable from a +browser-facing bundle. Sometimes the leak isn't a bare Node global at all, but an entire +third-party package — an Amphora internal, an Amphora plugin — that a component's `client.js` +transitively pulls in. `service-rewrite.js`'s existing `services/server` → `services/client` +rewrite only covers Clay's own isomorphic service convention; it has no opinion on a third-party +package like this. `bundlerConfig().serverOnlyPackages` is the same shape of fix, generalized: + +```js +// claycli.config.js +module.exports = { + bundlerConfig(config) { + config.serverOnlyPackages = ['amphora/lib']; + return config; + }, +}; +``` + +An array of package-path prefixes (default `[]`, a complete no-op) that **warn** — never +block — when a matching import resolves inside a browser-reachable module, naming the module and +its importer. This is a stopgap allowlist, not a hard guarantee like the `services/server` rewrite +(which errors, because a missing client counterpart is unambiguously broken): a match just means +"this looks suspicious," and the fix is the same package `browser` field remap or browser-safe +reimplementation as any other leaked server dependency. + ### Why `_globals-init.js` exists as a separate file Clay's `global/js/*.js` files are **side-effect modules** — they set up `window.DS`,