diff --git a/lib/cmd/vite/plugins/browser-compat.js b/lib/cmd/vite/plugins/browser-compat.js index f185ba1..6328777 100644 --- a/lib/cmd/vite/plugins/browser-compat.js +++ b/lib/cmd/vite/plugins/browser-compat.js @@ -23,8 +23,34 @@ const path = require('path'); * * Most built-ins are imported but never called in browser code paths; an empty * object is sufficient. A handful of built-ins (events, stream, util, buffer, - * http, https) are subclassed or extended by npm packages — those need a richer - * stub that provides the correct prototype chain so inheritance works. + * string_decoder, http, https, url) are subclassed, instantiated, or extended by + * npm packages — those need a richer stub that provides the correct prototype + * chain (and any constructors consumers instantiate) so nothing throws. + * + * ── Stub import-safety invariant ───────────────────────────────────────────── + * + * Every stub MUST evaluate without throwing when imported. A stub is dead code + * at runtime (guarded by isNode/process.browser), but its module body still runs + * at bundle-eval time. Two rules keep that safe: + * + * 1. Constructor-like built-ins (buffer, events, stream classes, http.Agent) + * are exposed as FUNCTIONS with a real `.prototype`, never plain objects. + * Libraries subclass them (`X.prototype = Object.create(Buffer.prototype)`) + * and feature-detect methods at module-eval; a plain-object stub throws + * "Object prototype may only be an Object or null: undefined" and takes the + * whole bundle down before Kiln boots (the safe-buffer crash fixed in #253). + * + * 2. Prefer the browser's own global over a hand-written shim. A real + * `globalThis.Buffer` / `fetch` / `URL` is correct AND costs zero bundle + * bytes; fall back to a minimal function-shaped shim only when the page + * provides no global. preferGlobal() is the single source of truth for this + * pattern (buffer, node-fetch, url). + * + * Rule 1 is the safe-buffer/#253 crash: an incomplete stub shape throws at eval + * and takes the whole bundle down before Kiln boots. When adding or changing a + * stub, validate that it imports cleanly against the libraries that actually + * reach it (e.g. crypto-browserify's create-hash → safe-buffer chain), not just + * that the specifier resolves. * * ── node: prefix ──────────────────────────────────────────────────────────── * @@ -114,23 +140,37 @@ const path = require('path'); const VIRTUAL_PREFIX = '\0clay-vite-compat:'; // Modules that can be safely replaced with an empty object namespace. +// +// Bare specifiers only. `node:`-prefixed ids (node:fs, node:buffer, …) are +// normalized to their bare name in resolveId() and routed here or to RICH_STUBS, +// so the prefixed variants don't need listing. Critically, a rich built-in's +// node: form must NOT appear here: listing e.g. `node:buffer` would short-circuit +// it to an empty stub and re-introduce the subclass-at-eval crash (see the +// import-safety invariant above) that the rich buffer stub exists to prevent. const SIMPLE_STUBS = new Set([ 'assert', 'child_process', 'cluster', 'crypto', 'dgram', 'dns', 'domain', 'fs', 'module', 'net', 'os', 'path', 'perf_hooks', 'punycode', 'querystring', - 'readline', 'repl', 'string_decoder', 'sys', 'timers', 'tls', 'tty', 'url', + 'readline', 'repl', 'sys', 'timers', 'tls', 'tty', 'v8', 'vm', 'worker_threads', 'zlib', 'hiredis', - // node: prefix variants - 'node:path', 'node:fs', 'node:os', 'node:crypto', 'node:url', - 'node:stream', 'node:events', 'node:util', 'node:buffer', 'node:http', 'node:https', - 'node:assert', 'node:child_process', 'node:cluster', 'node:dgram', 'node:dns', - 'node:domain', 'node:module', 'node:net', 'node:perf_hooks', 'node:punycode', - 'node:querystring', 'node:readline', 'node:repl', 'node:string_decoder', 'node:sys', - 'node:timers', 'node:tls', 'node:tty', 'node:v8', 'node:vm', 'node:worker_threads', - 'node:zlib', ]); // Modules that need a richer stub because libraries extend/inherit from them. -const RICH_STUBS = new Set(['events', 'stream', 'util', 'buffer', 'http', 'https', 'node-fetch']); +const RICH_STUBS = new Set(['events', 'stream', 'util', 'buffer', 'string_decoder', 'http', 'https', 'node-fetch', 'url']); + +/** + * Build a "prefer the browser's real global, else fall back" expression for use + * inside a stub's ESM source. Centralizes rule 2 of the import-safety invariant + * (prefer a real, zero-cost browser global) so every constructor-like stub + * applies it identically instead of hand-rolling the globalThis check. + * + * @param {string} globalName - the global to prefer, e.g. 'Buffer', 'URL' + * @param {string} fallbackExpr - JS expression used when the global is absent + * @returns {string} an expression string such as + * "(typeof globalThis !== 'undefined' && globalThis.URL) ? globalThis.URL : (function URL() {})" + */ +function preferGlobal(globalName, fallbackExpr) { + return `(typeof globalThis !== 'undefined' && globalThis.${globalName}) ? globalThis.${globalName} : (${fallbackExpr})`; +} const EVENTS_STUB = ` function EventEmitter() { this._events = this._events || {}; this._maxListeners = 10; } @@ -258,12 +298,52 @@ _clayBuffer.alloc = function(size) { return new Uint8Array(size > 0 ? size : 0); _clayBuffer.allocUnsafe = function(size) { return new Uint8Array(size > 0 ? size : 0); }; _clayBuffer.allocUnsafeSlow = function(size) { return new Uint8Array(size > 0 ? size : 0); }; _clayBuffer.concat = function(list) { return (list || []).reduce(function(a, b) { return Array.from(a).concat(Array.from(b)); }, []); }; -var _Buffer = (typeof globalThis !== 'undefined' && globalThis.Buffer) ? globalThis.Buffer : _clayBuffer; +var _Buffer = ${preferGlobal('Buffer', '_clayBuffer')}; export var Buffer = _Buffer; export function SlowBuffer(size) { return _Buffer.alloc(size > 0 ? size : 0); } export default { Buffer: _Buffer, SlowBuffer: SlowBuffer }; `; +// Browser stub for Node's `url`. +// +// The browser ships the real WHATWG URL/URLSearchParams — prefer them (rule 2: +// correct and zero bundle cost). The fallbacks are function-shaped (rule 1) so +// importing `url` and subclassing/feature-detecting these never throws when no +// global exists (e.g. a non-DOM worker). Legacy url.parse()/format() are +// intentionally omitted — they were absent from the previous empty stub too, so +// this change is purely additive (it only adds the two WHATWG constructors). +const URL_STUB = ` +var _URL = ${preferGlobal('URL', 'function URL() {}')}; +var _URLSearchParams = ${preferGlobal('URLSearchParams', 'function URLSearchParams() {}')}; +export var URL = _URL; +export var URLSearchParams = _URLSearchParams; +export default { URL: _URL, URLSearchParams: _URLSearchParams }; +`; + +// Browser stub for Node's `string_decoder`. +// +// `string_decoder` is both a Node builtin AND a userland npm package, so browser- +// compat intercepts the bare specifier — which means an empty stub silently +// breaks any code that constructs a StringDecoder. cipher-base (the base class +// behind crypto-browserify's create-hash / createHmac) does +// `new StringDecoder(enc).write(buf)` inside `.digest('hex' | 'base64' | …)`; an +// empty stub throws "StringDecoder is not a constructor" the instant a hash or +// HMAC is stringified in the browser. +// +// This minimal StringDecoder delegates to the Buffer's own toString(encoding), so +// with a real global Buffer (rule 2) a one-shot `.digest(enc)` returns the right +// string. The full decoder's multi-byte streaming boundary handling is +// unnecessary here — digests write the whole buffer in a single call. +const STRING_DECODER_STUB = ` +export function StringDecoder(encoding) { this.encoding = encoding || 'utf8'; } +StringDecoder.prototype.write = function(buf) { + if (buf == null) return ''; + return typeof buf.toString === 'function' ? buf.toString(this.encoding) : String(buf); +}; +StringDecoder.prototype.end = function(buf) { return buf == null ? '' : this.write(buf); }; +export default { StringDecoder: StringDecoder }; +`; + // node-fetch v1/v2 have no browser field; stub to native fetch so server-only // dependencies (encoding → iconv-lite → safer-buffer) never enter the browser bundle. const NODE_FETCH_STUB = ` @@ -299,17 +379,31 @@ const VITE_BROWSER_EXTERNAL = '__vite-browser-external'; const EMPTY_MODULE = 'export default {}; export {};'; +// Map of rich built-in name → ESM stub source. A lookup table (rather than a +// switch) keeps loadRichStub flat as the set of rich stubs grows. Every name here +// must also be in RICH_STUBS so resolveId routes it to the rich branch. +const RICH_STUB_SOURCES = { + events: EVENTS_STUB, + stream: STREAM_STUB, + util: UTIL_STUB, + buffer: BUFFER_STUB, + url: URL_STUB, + string_decoder: STRING_DECODER_STUB, + http: HTTP_STUB, + https: HTTP_STUB, + 'node-fetch': NODE_FETCH_STUB, +}; + +/** + * Resolve a rich built-in's ESM stub source by bare module name. + * + * @param {string} mod - the bare module name (e.g. 'buffer', 'string_decoder') + * @returns {string} the ESM stub source, or the empty module for unknown names + */ function loadRichStub(mod) { - switch (mod) { - case 'events': return EVENTS_STUB; - case 'stream': return STREAM_STUB; - case 'util': return UTIL_STUB; - case 'buffer': return BUFFER_STUB; - case 'http': - case 'https': return HTTP_STUB; - case 'node-fetch': return NODE_FETCH_STUB; - default: return EMPTY_MODULE; - } + return Object.prototype.hasOwnProperty.call(RICH_STUB_SOURCES, mod) + ? RICH_STUB_SOURCES[mod] + : EMPTY_MODULE; } // Short-circuit Vite's `__vite-browser-external` virtual when lenient mode diff --git a/lib/cmd/vite/plugins/browser-compat.test.js b/lib/cmd/vite/plugins/browser-compat.test.js index 78fd33b..cf2fcae 100644 --- a/lib/cmd/vite/plugins/browser-compat.test.js +++ b/lib/cmd/vite/plugins/browser-compat.test.js @@ -62,8 +62,8 @@ describe('viteBrowserCompatPlugin', () => { const SIMPLE_BUILTINS = [ 'assert', 'child_process', 'cluster', 'crypto', 'dgram', 'dns', 'domain', 'fs', 'module', 'net', 'os', 'path', 'perf_hooks', - 'punycode', 'querystring', 'readline', 'repl', 'string_decoder', 'sys', - 'timers', 'tls', 'tty', 'url', 'v8', 'vm', 'worker_threads', 'zlib', + 'punycode', 'querystring', 'readline', 'repl', 'sys', + 'timers', 'tls', 'tty', 'v8', 'vm', 'worker_threads', 'zlib', 'hiredis', ]; @@ -75,15 +75,27 @@ describe('viteBrowserCompatPlugin', () => { }); it.each([ - 'node:path', 'node:fs', 'node:os', 'node:crypto', 'node:url', - 'node:stream', 'node:events', 'node:util', 'node:buffer', - ])('stubs node:-prefixed built-in "%s"', (id) => { + 'node:path', 'node:fs', 'node:os', 'node:crypto', 'node:assert', 'node:zlib', + ])('stubs node:-prefixed simple built-in "%s"', (id) => { const { resolvedId } = runPlugin(id); expect(resolvedId).not.toBeNull(); expect(resolvedId).toContain('simple:'); }); + // node:-prefixed rich built-ins must route to the rich stub, not an empty one. + // Otherwise a library subclassing e.g. node:buffer's Buffer crashes at module + // evaluation — the same class of bug as the safe-buffer #253 crash. + it.each([ + 'node:buffer', 'node:stream', 'node:events', 'node:util', + 'node:http', 'node:https', 'node:url', + ])('routes node:-prefixed rich built-in "%s" to the rich stub', (id) => { + const { resolvedId } = runPlugin(id); + + expect(resolvedId).not.toBeNull(); + expect(resolvedId).toContain('rich:'); + }); + it('loads an empty ESM namespace for simple stubs', () => { const { code } = runPlugin('fs'); @@ -92,7 +104,7 @@ describe('viteBrowserCompatPlugin', () => { }); describe('resolveId — rich stubs', () => { - const RICH_MODS = ['events', 'stream', 'util', 'buffer', 'http', 'https', 'node-fetch']; + const RICH_MODS = ['events', 'stream', 'util', 'buffer', 'string_decoder', 'http', 'https', 'node-fetch', 'url']; it.each(RICH_MODS)('stubs rich module "%s"', (id) => { const { resolvedId } = runPlugin(id); @@ -213,6 +225,58 @@ describe('viteBrowserCompatPlugin', () => { expect(code).toContain('globalThis.fetch'); }); + + it('url stub exports URL and URLSearchParams', () => { + const { code } = runPlugin('url'); + + expect(code).toContain('export var URL'); + expect(code).toContain('export var URLSearchParams'); + }); + + it('url stub prefers the real global URL/URLSearchParams when present', () => { + const { code } = runPlugin('url'); + const FakeURL = function URL() {}; + const FakeUSP = function URLSearchParams() {}; + const exports = evalEsmStub(code, { URL: FakeURL, URLSearchParams: FakeUSP }); + + expect(exports.URL).toBe(FakeURL); + expect(exports.URLSearchParams).toBe(FakeUSP); + }); + + it('url fallback URL/URLSearchParams are function-shaped when no global exists', () => { + const { code } = runPlugin('url'); + const exports = evalEsmStub(code, {}); // browser path: no global URL + + expect(typeof exports.URL).toBe('function'); + expect(typeof exports.URLSearchParams).toBe('function'); + expect(() => Object.create(exports.URL.prototype)).not.toThrow(); + }); + + // cipher-base (crypto-browserify → create-hash/createHmac) constructs a + // StringDecoder inside .digest(enc); an empty stub throws "not a constructor". + // The decoder delegates to the buffer's own toString(encoding). + it('string_decoder stub exposes a constructable StringDecoder that decodes via Buffer.toString', () => { + const { code } = runPlugin('string_decoder'); + const exports = evalEsmStub(code, {}); + const StringDecoder = exports.StringDecoder; + + expect(typeof StringDecoder).toBe('function'); + + const fakeBuf = { toString: enc => `decoded:${enc}` }; + const decoder = new StringDecoder('hex'); + + expect(decoder.write(fakeBuf)).toBe('decoded:hex'); + expect(decoder.end()).toBe(''); + }); + + // Regression: node:buffer must load the function-shaped buffer stub, never an + // empty one — an empty node:buffer stub reintroduces the #253 subclass crash. + it('node:buffer loads the function-shaped buffer stub, not an empty stub', () => { + const { code } = runPlugin('node:buffer'); + + expect(code).toContain('export var Buffer'); + expect(code).not.toBe('export default {}; export {};'); + }); }); describe('custom stubs (site-specific overrides)', () => {