From 5b9892a9077c26a057cda6ab01e07857522cd81e Mon Sep 17 00:00:00 2001 From: Jordan Paulino Date: Fri, 27 Feb 2026 13:01:57 -0500 Subject: [PATCH 01/96] =?UTF-8?q?=F0=9F=8D=95=20Add=20pack-next=20(esbuild?= =?UTF-8?q?)=20bundler=20and=20Node=2020=20compile=20fixes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Why this change exists The legacy `clay compile` command uses Browserify (via megabundler) to produce browser bundles. Browserify generates a runtime `window.modules` registry and synchronous `window.require()` function. This architecture has two problems: 1. The output is CommonJS/IIFE — not native ES Modules — so browsers cannot use ` + +// ` +`` +``` + +`omitCacheBusterOnModules` is set to `true` when the `modulepreload: true` option is passed to amphora-html's `setup()`. In `sites/amphora/renderers.js`: + +```javascript +modulepreload: true // enables and strips ?version= from module URLs +``` + +The old Browserify tags are completely unaffected — they still receive `?version=` as before. The change is strictly scoped to `type="module"` and `modulepreload` tags. + +## 15. Bundler Comparison + +For the full technical rationale of why Vite was chosen over esbuild, bare Rollup, and +Browserify — including measured performance data and a pipeline-by-pipeline pros/cons +analysis — see: + +**[`BUNDLER-COMPARISON.md`](./BUNDLER-COMPARISON.md)** + +## 16. Services Pattern and Browser Bundle Hygiene + +The `sites` repo organizes its service layer into three tiers: + +- `services/server/` — Node.js only. May use any server package. +- `services/client/` — Browser only. Communicates with server services over HTTP/REST. +- `services/universal/` — Safe to run in **both** environments. Must not import server-only + packages at module level. + +Clay's Vite pipeline bundles `model.js` files and Kiln plugins for the browser (they run in +Kiln edit mode). Every `require()` in those files — and their entire transitive dependency +chain — ends up in the browser bundle. Any violation of the three-tier contract (e.g. a +`services/universal/` file that imports `clay-log` at the top) silently pulls Node-only +packages into the browser. + +Under the old Browserify pipeline, violations were invisible: Browserify bundled everything +into megabundles and never complained about Node-only packages. Under Vite, unresolvable +Node built-ins produce explicit build errors or runtime crashes. `serviceRewritePlugin` +automatically redirects `services/server/*` to `services/client/*` in browser builds. + +### Why this matters for bundle size + +If `serviceRewritePlugin` is not respected (e.g. a `client.js` file imports +`services/server/query` directly instead of `services/client/query`), the server service's +full dependency tree enters the browser bundle. This can add hundreds of KB of Node-only +transitive dependencies (Elasticsearch clients, `node-fetch`, `iconv-lite` encoding tables, +etc.) that the browser never needs and can never actually use. + +See [`lib/cmd/vite/plugins/service-rewrite.js`](https://github.com/clay/claycli/blob/jordan/yolo-update/lib/cmd/vite/plugins/service-rewrite.js) +for the full implementation and bundle-size impact documentation. + +### Known violations (already fixed) + +| Service | Problem | Fix applied | +|---|---|---| +| `services/universal/styles.js` | Imported full PostCSS toolchain (~666 KB gz) | Moved to `services/server/styles.js`; added no-op `services/client/styles.js` stub | +| `services/universal/coral.js` | Imported `node-fetch` + `iconv-lite` (~576 KB gz) | Moved to `services/server/coral.js`; added `services/client/coral.js` stub with only browser-safe exports | +| `services/universal/log.js` | Imported `clay-log` (Node-only) at top level | Moved `clay-log` import inside lazy conditional branches | diff --git a/cli/build.js b/cli/build.js deleted file mode 100644 index 91057b45..00000000 --- a/cli/build.js +++ /dev/null @@ -1,94 +0,0 @@ -'use strict'; - -const { build, watch } = require('../lib/cmd/build'); -const log = require('./log').setup({ file: __filename }); - -function builder(yargs) { - return yargs - .usage('Usage: $0 [options]') - .option('watch', { - alias: 'w', - type: 'boolean', - description: 'Watch for file changes and rebuild automatically', - default: false, - }) - .option('minify', { - alias: 'm', - type: 'boolean', - description: 'Minify output (also enabled by CLAYCLI_COMPILE_MINIFIED env var)', - default: !!process.env.CLAYCLI_COMPILE_MINIFIED, - }) - .option('entry', { - alias: 'e', - type: 'array', - description: 'Additional entry-point file paths (supplements the default component globs)', - default: [], - }) - .example('$0', 'Build all component scripts with esbuild') - .example('$0 --watch', 'Rebuild on every file change') - .example('$0 --minify', 'Build and minify for production'); -} - -async function handler(argv) { - const options = { - minify: argv.minify, - extraEntries: argv.entry || [], - }; - - if (argv.watch) { - try { - const ctx = await watch({ - ...options, - onRebuild(errors) { - // In watch mode only surface errors — esbuild reports warnings for - // every file it touches on each incremental rebuild, not just the - // changed file, which floods the terminal with irrelevant noise. - // Warnings are still visible in full during `make compile`. - if (errors.length > 0) { - errors.forEach(e => log('error', e.text, { location: e.location })); - } else { - log('info', '[js] Rebuilt successfully'); - } - }, - }); - - log('info', 'Watching for changes — press Ctrl+C to stop'); - - process.on('SIGINT', () => { - ctx.dispose().then(() => process.exit(0)); - }); - - process.on('SIGTERM', () => { - ctx.dispose().then(() => process.exit(0)); - }); - } catch (error) { - log('error', 'Watch setup failed', { error: error.message }); - process.exit(1); - } - } else { - try { - const result = await build(options); - - if (result.errors.length > 0) { - result.errors.forEach(e => log('error', e.text, { location: e.location })); - process.exit(1); - } - - // esbuild warnings are pre-existing code issues (duplicate object keys, - // typeof-null, etc.) that are not actionable build failures. Log a count - // so they are visible without flooding the terminal with full locations. - if (result.warnings.length > 0) { - log('warn', `${result.warnings.length} esbuild warning(s) — run with --log-level=verbose to see details`); - } - } catch (error) { - log('error', 'Build failed', { error: error.message }); - process.exit(1); - } - } -} - -exports.aliases = ['pack-next', 'pn']; // pack-next kept for backward compat with existing Makefiles -exports.builder = builder; -exports.command = 'build'; -exports.describe = 'Compile component scripts and assets with esbuild'; -exports.handler = handler; diff --git a/cli/index.js b/cli/index.js index 3ae55532..abf49e27 100755 --- a/cli/index.js +++ b/cli/index.js @@ -21,14 +21,9 @@ const yargs = require('yargs'), e: 'export', i: 'import', l: 'lint', - b: 'build', p: 'pack', - pn: 'build', // legacy alias: pn → build - 'pack-next': 'build', // legacy alias: clay pack-next → clay build - r: 'rollup', - rollup: 'rollup', // Rollup + esbuild transform (parallel to clay build) v: 'vite', - vite: 'vite', // Vite pipeline (parallel to clay rollup) + vite: 'vite', }, listCommands = Object.keys(commands).concat(Object.values(commands)); diff --git a/cli/rollup.js b/cli/rollup.js deleted file mode 100644 index ababa663..00000000 --- a/cli/rollup.js +++ /dev/null @@ -1,78 +0,0 @@ -'use strict'; - -const { build, watch } = require('../lib/cmd/rollup'); -const log = require('./log').setup({ file: __filename }); - -function builder(yargs) { - return yargs - .usage('Usage: $0 [options]') - .option('watch', { - alias: 'w', - type: 'boolean', - description: 'Watch for file changes and rebuild automatically', - default: false, - }) - .option('minify', { - alias: 'm', - type: 'boolean', - description: 'Minify output (also enabled by CLAYCLI_COMPILE_MINIFIED env var)', - default: !!process.env.CLAYCLI_COMPILE_MINIFIED, - }) - .option('entry', { - alias: 'e', - type: 'array', - description: 'Additional entry-point file paths (supplements the default component globs)', - default: [], - }) - .example('$0', 'Build all component scripts with Rollup (+ esbuild transform)') - .example('$0 --watch', 'Rebuild on every file change') - .example('$0 --minify', 'Build and minify for production'); -} - -async function handler(argv) { - const options = { - minify: argv.minify, - extraEntries: argv.entry || [], - }; - - if (argv.watch) { - try { - const ctx = await watch({ - ...options, - onRebuild(errors) { - if (errors.length > 0) { - errors.forEach(e => log('error', e.message || String(e))); - } else { - log('info', '[js] Rebuilt successfully'); - } - }, - }); - - log('info', 'Watching for changes — press Ctrl+C to stop'); - - process.on('SIGINT', () => { - ctx.dispose().then(() => process.exit(0)); - }); - - process.on('SIGTERM', () => { - ctx.dispose().then(() => process.exit(0)); - }); - } catch (error) { - log('error', 'Watch setup failed', { error: error.message }); - process.exit(1); - } - } else { - try { - await build(options); - } catch (error) { - log('error', 'Build failed', { error: error.message }); - process.exit(1); - } - } -} - -exports.aliases = []; -exports.builder = builder; -exports.command = 'rollup'; -exports.describe = 'Compile component scripts and assets with Rollup (esbuild transform)'; -exports.handler = handler; diff --git a/index.js b/index.js index 0f3d9968..f7920a7b 100644 --- a/index.js +++ b/index.js @@ -7,7 +7,7 @@ module.exports.lint = require('./lib/cmd/lint'); module.exports.import = require('./lib/cmd/import'); module.exports.export = require('./lib/cmd/export'); module.exports.compile = require('./lib/cmd/compile'); -module.exports.build = require('./lib/cmd/build'); +module.exports.vite = require('./lib/cmd/vite'); module.exports.gulp = require('gulp'); // A reference to the Gulp instance so that external tasks can reference a common package module.exports.mountComponentModules = require('./lib/cmd/pack/mount-component-modules'); module.exports.getWebpackConfig = require('./lib/cmd/pack/get-webpack-config'); diff --git a/lib/cmd/build/fonts.js b/lib/cmd/build/fonts.js deleted file mode 100644 index 79e2ee78..00000000 --- a/lib/cmd/build/fonts.js +++ /dev/null @@ -1,106 +0,0 @@ -'use strict'; - -const path = require('path'); -const fs = require('fs-extra'); -const { globSync } = require('glob'); - -const CWD = process.cwd(); - -const FONT_EXTS = 'css,woff,woff2,otf,ttf'; -const FONTS_SRC_GLOB = path.join(CWD, 'styleguides', '*', 'fonts', `*.{${FONT_EXTS}}`); - -// Output destinations -const CSS_DEST = path.join(CWD, 'public', 'css'); -const BINARY_DEST = path.join(CWD, 'public', 'fonts'); - -const ASSET_HOST = process.env.CLAYCLI_COMPILE_ASSET_HOST - ? process.env.CLAYCLI_COMPILE_ASSET_HOST.replace(/\/$/, '') - : ''; -const ASSET_PATH = process.env.CLAYCLI_COMPILE_ASSET_PATH || ''; - -/** - * Extract the styleguide name from an absolute font file path. - * Expected structure: .../styleguides/{sg}/fonts/{file} - * - * @param {string} srcPath - * @returns {string} styleguide name - */ -function getStyleguide(srcPath) { - const parts = srcPath.split(path.sep); - const sgIdx = parts.lastIndexOf('styleguides'); - - return parts[sgIdx + 1]; -} - -/** - * Process fonts: - * - CSS files: apply $asset-host / $asset-path substitution, then concatenate - * all per-styleguide CSS into public/css/_linked-fonts.{sg}.css so that - * amphora-html can find and inline the @font-face declarations. - * - Binary files (.woff, .woff2, .otf, .ttf): copy as-is to - * public/fonts/{styleguide}/ for cases where fonts are self-hosted. - * - * @returns {Promise} count of files processed - */ -async function buildFonts() { - const files = globSync(FONTS_SRC_GLOB); - - if (files.length === 0) return 0; - - // Group by styleguide - const byStyleguide = {}; - - for (const srcPath of files) { - const sg = getStyleguide(srcPath); - - if (!byStyleguide[sg]) { - byStyleguide[sg] = { css: [], binary: [] }; - } - - const ext = path.extname(srcPath).toLowerCase(); - - if (ext === '.css') { - byStyleguide[sg].css.push(srcPath); - } else { - byStyleguide[sg].binary.push(srcPath); - } - } - - await Promise.all([ - fs.ensureDir(CSS_DEST), - fs.ensureDir(BINARY_DEST), - ]); - - await Promise.all(Object.entries(byStyleguide).map(async ([sg, { css, binary }]) => { - // Write _linked-fonts.{sg}.css — amphora-html looks for this file to inject - // @font-face declarations into every page that uses this styleguide. - if (css.length > 0) { - const cssChunks = await Promise.all(css.map(async (srcPath) => { - let content = await fs.readFile(srcPath, 'utf8'); - - content = content.replace(/\$asset-host/g, ASSET_HOST); - content = content.replace(/\$asset-path/g, ASSET_PATH); - - return content; - })); - - await fs.writeFile( - path.join(CSS_DEST, `_linked-fonts.${sg}.css`), - cssChunks.join('\n'), - 'utf8' - ); - } - - // Copy binary font files (self-hosted scenarios) - await Promise.all(binary.map(async (srcPath) => { - const destPath = path.join(BINARY_DEST, sg, path.basename(srcPath)); - - await fs.ensureDir(path.dirname(destPath)); - await fs.copy(srcPath, destPath, { overwrite: true }); - })); - })); - - return files.length; -} - -module.exports = { buildFonts, FONTS_SRC_GLOB }; diff --git a/lib/cmd/build/get-script-dependencies.js b/lib/cmd/build/get-script-dependencies.js deleted file mode 100644 index 865ca468..00000000 --- a/lib/cmd/build/get-script-dependencies.js +++ /dev/null @@ -1,314 +0,0 @@ -'use strict'; - -const fs = require('fs-extra'); -const { globSync } = require('glob'); -const path = require('path'); - -const { KILN_EDIT_ENTRY_KEY, VIEW_INIT_ENTRY_KEY } = require('./scripts'); - -const DEST = path.resolve(process.cwd(), 'public', 'js'); -const MANIFEST_PATH = path.join(DEST, '_manifest.json'); - -/** - * Read and return the _manifest.json written by `clay build`. - * Returns null when no manifest has been built yet. - * - * @returns {object|null} - */ -function readManifest() { - try { - return fs.readJsonSync(MANIFEST_PATH); - } catch { - return null; - } -} - -/** - * Convert an old-style Browserify module-ID path back to a component key - * as it appears in _manifest.json. - * - * Old format: '/assetPath/js/article.client.js' - * Manifest key: 'components/article/client' - * - * Handles components, layouts, and kiln plugins. - * - * @param {string} publicPath - e.g. 'https://cdn.example.com/js/article.client.js' - * @returns {string|null} - */ -function publicPathToManifestKey(publicPath) { - const basename = publicPath.split('/').pop().replace(/\.js$/, ''); - - // article.client → components/article/client - // article.model → components/article/model - // article.kiln → components/article/kiln - const componentMatch = basename.match(/^(.+)\.(client|model|kiln)$/); - - if (componentMatch) { - return `components/${componentMatch[1]}/${componentMatch[2]}`; - } - - // layout-foo.client → layouts/layout-foo/client - const layoutMatch = basename.match(/^(.+layout[^.]*)\.(client|model)$/i); - - if (layoutMatch) { - return `layouts/${layoutMatch[1]}/${layoutMatch[2]}`; - } - - return null; -} - -/** - * Returns true when the chunk should be included in the output scripts list. - * Skips already-seen chunks, and when chunksOnly is true also skips individual - * component client files (only shared /chunks/ paths pass through). - * - * @param {string} chunk - * @param {boolean} chunksOnly - * @param {Set} seen - * @returns {boolean} - */ -function shouldIncludeChunk(chunk, chunksOnly, seen) { - if (seen.has(chunk)) return false; - if (chunksOnly && !chunk.includes('/chunks/')) return false; - return true; -} - -/** - * Recursively collect the entry file + all its imported chunks. - * - * @param {string} key - Manifest key e.g. 'components/article/client' - * @param {object} manifest - Parsed _manifest.json - * @param {Set} seen - Already-visited entries (avoids duplicates) - * @param {object} [opts] - * @param {string} [opts.base] - Public URL base prefix e.g. '/js' - * @param {boolean} [opts.chunksOnly] - Skip non-shared-chunk imports - * @returns {string[]} - */ -function collectScripts(key, manifest, seen, opts) { - const { base = '/js', chunksOnly = false } = opts || {}; - const entry = manifest[key]; - - if (!entry || seen.has(key)) return []; - seen.add(key); - - const scripts = []; - - // Guard entry.file against duplicates — a global entry (e.g. global/js/ads) - // may have already been added as a shared import of a previously processed - // entry (e.g. components/init). - if (!seen.has(entry.file)) { - seen.add(entry.file); - scripts.push(entry.file.replace(/^\/js/, base)); - } - - for (const chunk of entry.imports || []) { - if (!shouldIncludeChunk(chunk, chunksOnly, seen)) continue; - seen.add(chunk); - scripts.push(chunk.replace(/^\/js/, base)); - } - - return scripts; -} - -/** - * Return an array of hashed script URLs that should be loaded for the given - * component scripts in view mode. - * - * Drop-in replacement for `getDependencies` (Browserify version) when - * _manifest.json is present. - * - * @param {string[]} scripts - Array of public script paths from amphora media - * @param {string} assetPath - Site asset prefix (e.g. 'https://cdn.example.com') - * @param {object} [options] - * @param {boolean} [options.edit] - true when in Kiln edit mode - * @returns {string[]} - */ -function getDependenciesNext(scripts, assetPath, options) { - const manifest = readManifest(); - - if (!manifest) { - throw new Error( - 'clay build: _manifest.json not found.\n' + - 'Run `clay build` first to generate the build output.' - ); - } - - const base = (assetPath || '') + '/js'; - const seen = new Set(); - - if (options && options.edit) { - return getEditDeps(manifest, base, seen); - } - - return getViewDeps(scripts, manifest, base, seen); -} - -/** - * Return an array of hashed script URLs that should be loaded for a page - * rendered with the clay build (esbuild/ESM) pipeline. - * - * Only emits _view-init, global entry points, and their shared chunks. - * Per-component client.js files are intentionally excluded — _view-init - * scans the DOM and imports them on demand, so eager