From b46060f4a96d2670c29a188c66c3e6b338f45c5c 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/contract/bundle-isolation.test.js | 145 +++++++++++++++++++++++++ 4 files changed, 163 insertions(+), 1 deletion(-) create mode 100644 test/contract/bundle-isolation.test.js diff --git a/CHANGELOG.md b/CHANGELOG.md index 5d356e5..7c75d9e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,15 @@ committed `dist/` is still the `0.2.11` build. ### Build and tests +- The minified module bundle `dist/sd-model.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/contract/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 (`SilverDecisions.Model`, the global `require` exposing `sd-model`) and ES5 output. The UMD bundle registers an diff --git a/README.md b/README.md index f00e3fc..1fc57c6 100644 --- a/README.md +++ b/README.md @@ -51,6 +51,12 @@ contract tests load: `SD_MODEL_DIST` points at another `dist/` directory, `SD_UTILS_BUNDLE` at another sd-utils `require` bundle. +For the `module` targets `test/contract/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`. + The contract tests cover the API used by sd-computations, sd-tree-designer and SilverDecisions: exports and bundle globals, domain classes, editing trees, ids and parent-child links, undo / redo, validation results, the DTO diff --git a/rollup.config.mjs b/rollup.config.mjs index a6b0016..36c27ce 100644 --- a/rollup.config.mjs +++ b/rollup.config.mjs @@ -46,7 +46,9 @@ function outputs(file, output) { const common = Object.assign({exports: 'named', esModule: true, freeze: false}, output); return [ Object.assign({file: file + '.js', sourcemap: 'inline'}, common), - Object.assign({file: file + '.min.js', sourcemap: true, plugins: [terser()]}, common) + // 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})]}, common) ]; } diff --git a/test/contract/bundle-isolation.test.js b/test/contract/bundle-isolation.test.js new file mode 100644 index 0000000..66e9686 --- /dev/null +++ b/test/contract/bundle-isolation.test.js @@ -0,0 +1,145 @@ +// Isolation of the module bundle dist/sd-model(.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_MODEL_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-model'; +const target = process.env.SD_MODEL_TARGET || 'standalone'; +const root = path.resolve(__dirname, '../..'); +const dist = path.resolve(root, process.env.SD_MODEL_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'); + +const installed = (name, file) => path.join(path.dirname(require.resolve(name + '/package.json', {paths: [root]})), 'dist', file); +// module bundles this one requires, loaded before it (SD_UTILS_BUNDLE as in load.js) +const DEPENDENCIES = [path.resolve(process.env.SD_UTILS_BUNDLE || installed('sd-utils', 'sd-utils.js'))]; +const DEPENDENCY_NAMES = ['sd-utils']; +// globals the bundle may add when its API is used, besides `require` +const AFTER_USE = []; +// API calls on `exports` in the context (serialize uses Utils of sd-utils); EXPECTED is the string they return +const USE = 'var d = exports.domain, m = new exports.DataModel(); m.addNode(new d.DecisionNode(new d.Point(0, 0))); ' + + 'return JSON.stringify([JSON.parse(m.serialize(true)).trees.length, m.getRoots().length]);'; +const EXPECTED = '[1,1]'; +// sd-utils (lodash, loglevel) reads the global `window` when sd-model's factory requires it +const LAZY = { + setup: 'var windowReads = 0; Object.defineProperty(this, "window", {configurable: true, get: function () { windowReads++; return undefined; }});', + evaluated: 'windowReads > 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); +});