From e8ef0ed4c42b3949a80c4fca03b2b7a7dddc5fd1 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 4 Nov 2026 15:00:04 +0000 Subject: [PATCH] [fix] preserve isolation of minified require bundles @rollup/plugin-terser defaults to toplevel: true for the cjs output. The minified module bundle then starts with top-level vars that hold the require wrapper's cache and factories, so a second minified module bundle in the same scope overwrote them (Maximum call stack size exceeded, r is not a function). Terser now runs with toplevel: false; plain and standalone files are unchanged. bundle-isolation.test.js (module targets) checks the added globals, the exports and cache after another require bundle is loaded, the require chain before and after the bundle, MODULE_NOT_FOUND and lazy evaluation. --- CHANGELOG.md | 9 +++ README.md | 6 ++ rollup.config.mjs | 4 +- test/bundle-isolation.test.js | 143 ++++++++++++++++++++++++++++++++++ 4 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 test/bundle-isolation.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e1472b..a718be9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -31,6 +31,15 @@ All other results are unchanged for the same `Math.random` draws. ### Build +- The minified module bundle `dist/sd-random.min.js` keeps the state of its + `require` wrapper (module cache, factories, previous `require`) in its own + closure: `@rollup/plugin-terser` defaults to `toplevel: true` for the cjs + output and turned the wrapper into global variables, so two minified module + bundles in one scope overwrote each other (`Maximum call stack size + exceeded`, `r is not a function`). Terser now runs with `toplevel: false`; + the plain and standalone files are unchanged. + `test/bundle-isolation.test.js` checks the globals, the cache and the + `require` chain of the module bundle. - The bundles are built with Rollup and Babel 7 instead of gulp/browserify, with the same formats, global names and ES5 output. `npm run docs` replaces the gulp `doc` task; documentation.js 14 writes `doc/API.md` in a newer diff --git a/README.md b/README.md index 06920f4..f9576c5 100644 --- a/README.md +++ b/README.md @@ -44,6 +44,12 @@ compare with independent reference values (scipy, mpmath, CPython's `random.gammavariate`); the sample moment checks use a seeded generator, so they give the same result on every run. +For the `module` targets `test/bundle-isolation.test.js` loads the module bundle as a script into a fresh +global scope and checks that it adds only `require`, keeps its exports and +cache when another `require` bundle is loaded after it, passes other names +along the `require` chain (`MODULE_NOT_FOUND` for unknown ones) and evaluates +the module on the first `require`. + `npm run test:build` (CI on Linux) copies the working tree, runs `npm ci` and the build, checks the six artifacts (ES5, parsed source maps) and that nothing outside `dist/` changed, runs the tests against the four bundles (plain and diff --git a/rollup.config.mjs b/rollup.config.mjs index 274e689..13bef98 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -27,7 +27,9 @@ function outputs(file, output) { output = Object.assign({exports: 'named', esModule: true}, output); return [ Object.assign({file: file + '.js', sourcemap: 'inline'}, output), - Object.assign({file: file + '.min.js', sourcemap: true, plugins: [terser()]}, output) + // toplevel: false, as for the other formats: @rollup/plugin-terser turns it on for cjs, + // which moves the variables of the require wrapper into the global scope + Object.assign({file: file + '.min.js', sourcemap: true, plugins: [terser({toplevel: false})]}, output) ]; } diff --git a/test/bundle-isolation.test.js b/test/bundle-isolation.test.js new file mode 100644 index 0000000..6704163 --- /dev/null +++ b/test/bundle-isolation.test.js @@ -0,0 +1,143 @@ +// Isolation of the module bundle dist/sd-random(.min).js. The bundle joins the global +// `require` chain (format of browserify's `require` option) and must keep its +// module cache and factories in its own closure. Each case runs the files as +// classic scripts in a fresh vm context and checks behaviour, not variable names. +// Runs for the module and module.min targets (SD_RANDOM_TARGET). +'use strict'; + +const test = require('node:test'); +const assert = require('node:assert/strict'); +const fs = require('fs'); +const path = require('path'); +const vm = require('vm'); + +const NAME = 'sd-random'; +const target = process.env.SD_RANDOM_TARGET || 'standalone'; +const root = path.resolve(__dirname, '..'); +const dist = path.resolve(root, process.env.SD_RANDOM_DIST || 'dist'); +const min = /^module(\.min)?$/.exec(target); +const skip = min ? false : 'module bundle targets only'; +const bundle = path.join(dist, NAME + (min && min[1] ? '.min' : '') + '.js'); + +// module bundles this one requires, loaded before it +const DEPENDENCIES = []; +const DEPENDENCY_NAMES = []; +// globals the bundle may add when its API is used, besides `require` +const AFTER_USE = []; +// API calls on `exports` in the context (Math.random returns 0.5); EXPECTED is the string they return +const USE = 'return JSON.stringify([exports.Uniform(2, 4), exports.Uniform(0, 10)]);'; +const EXPECTED = '[3,5]'; +// the module reads Math.random when evaluated (`export const random = Math.random`) +const LAZY = { + setup: 'var randomReads = 0, half = Math.random; Object.defineProperty(Math, "random", {configurable: true, get: function () { randomReads++; return half; }});', + evaluated: 'randomReads > 0', + require: 'require(' + JSON.stringify(NAME) + ')' +}; + +const code = new Map(); +function script(file) { + if (!code.has(file)) { + code.set(file, new vm.Script(fs.readFileSync(file, 'utf8'), {filename: file})); + } + return code.get(file); +} + +// A fresh global scope; Math.random returns 0.5. +function context() { + const ctx = vm.createContext({}); + const run = (src) => vm.runInContext(src, ctx); + run('Math.random = function () { return 0.5; };'); + const names = () => Object.getOwnPropertyNames(run('this')).sort(); + return {ctx, run, names, load: (file) => script(file).runInContext(ctx)}; +} + +const added = (before, after) => after.filter(n => !before.includes(n)); + +// a `require` defined before the bundles, as browserify defines it +const BEFORE = `require = function (id, jumped) { + calls.push([id, jumped]); + if (id === 'iso-before') { return before; } + var error = new Error("Cannot find module '" + id + "'"); + error.code = 'MODULE_NOT_FOUND'; + throw error; +};`; +// a `require` bundle loaded after them +const AFTER = `require = (function (previous) { + var own = {exports: after}; + function newRequire(id, jumped) { + if (id === 'iso-after') { return own.exports; } + var current = typeof require === 'function' && require; + if (!jumped && current && current !== newRequire) { return current(id, true); } + if (previous) { return previous(id, true); } + var error = new Error("Cannot find module '" + id + "'"); + error.code = 'MODULE_NOT_FOUND'; + throw error; + } + return newRequire; +})(typeof require === 'function' && require);`; + +test('adds only `require` to the global scope, also after its API is used', {skip}, () => { + const c = context(); + const before = c.names(); + DEPENDENCIES.forEach(c.load); + c.load(bundle); + assert.deepEqual(added(before, c.names()), ['require']); + assert.equal(c.run('(function (exports) {' + USE + '})(require(' + JSON.stringify(NAME) + '))'), EXPECTED); + assert.deepEqual(added(before, c.names()), ['require'].concat(AFTER_USE).sort()); +}); + +test('keeps its exports and cache when the same bundle is loaded again after it', {skip}, () => { + const c = context(); + DEPENDENCIES.forEach(c.load); + c.load(bundle); + c.run('var first = require, exportsOfFirst = first(' + JSON.stringify(NAME) + ');'); + c.run(DEPENDENCY_NAMES.map(n => 'var dep_' + n.replace(/-/g, '_') + ' = first(' + JSON.stringify(n) + ');').join('\n')); + c.load(bundle); + const result = JSON.parse(c.run(`(function (name) { + var second = require, exportsOfSecond = second(name); + return JSON.stringify({ + newRequire: second !== first, + firstCached: first(name) === exportsOfFirst, + secondOwn: exportsOfSecond !== exportsOfFirst && second(name) === exportsOfSecond, + dependenciesShared: [${DEPENDENCY_NAMES.map(n => `first('${n}') === dep_${n.replace(/-/g, '_')} && second('${n}') === dep_${n.replace(/-/g, '_')}`).concat('true').join(', ')}].every(Boolean), + firstWorks: (function (exports) {${USE}})(exportsOfFirst), + secondWorks: (function (exports) {${USE}})(exportsOfSecond) + }); + })(${JSON.stringify(NAME)})`)); + assert.deepEqual(result, {newRequire: true, firstCached: true, secondOwn: true, dependenciesShared: true, + firstWorks: EXPECTED, secondWorks: EXPECTED}); +}); + +test('passes other names to the require defined before it and to a require bundle loaded after it', {skip}, () => { + const c = context(); + c.run('var calls = [], before = {}, after = {};'); + c.run(BEFORE); + DEPENDENCIES.forEach(c.load); + c.load(bundle); + c.run('var own = require, exportsOfOwn = own(' + JSON.stringify(NAME) + ');'); + assert.equal(c.run('own("iso-before") === before'), true); + assert.deepEqual(JSON.parse(c.run('JSON.stringify(calls)')), [['iso-before', true]]); + c.run(AFTER); + assert.equal(c.run('own("iso-after") === after'), true); + assert.equal(c.run('require(' + JSON.stringify(NAME) + ') === exportsOfOwn && own(' + JSON.stringify(NAME) + ') === exportsOfOwn'), true); + const error = JSON.parse(c.run('(function () { try { own("iso-missing"); } catch (e) { return JSON.stringify({code: e.code, message: e.message}); } })()')); + assert.deepEqual(error, {code: 'MODULE_NOT_FOUND', message: "Cannot find module 'iso-missing'"}); +}); + +test('throws MODULE_NOT_FOUND for unknown names when it is the first require', {skip}, () => { + const c = context(); + DEPENDENCIES.forEach(c.load); + c.load(bundle); + const error = JSON.parse(c.run('(function () { try { require("iso-missing"); } catch (e) { return JSON.stringify({code: e.code, message: e.message}); } })()')); + assert.deepEqual(error, {code: 'MODULE_NOT_FOUND', message: "Cannot find module 'iso-missing'"}); +}); + +test('evaluates the module on the first require, not when the file is loaded', {skip}, () => { + const c = context(); + c.run(LAZY.setup); + DEPENDENCIES.forEach(c.load); + c.load(bundle); + assert.equal(c.run(LAZY.evaluated), false); + c.run(LAZY.require); + assert.equal(c.run(LAZY.evaluated), true); +});