From b2b8b6add9b34586f0b39fc03353b921d2cf878f Mon Sep 17 00:00:00 2001 From: Matthias von Bargen Date: Thu, 12 Nov 2026 17:48:50 +0100 Subject: [PATCH] Stop the editor's published bundle breaking in the browser react-dom/client wasn't external, so the editor bundled react-dom as CommonJS and rolldown left require("react") calls in it. Browsers have no require, so the editor's UI failed to load in installed projects. three's addons were bundled too, giving the game and the editor separate copies of classes like TransformControls. Both are external now. The template aliases core and the editor to their src/, so neither local dev nor CI ever loaded dist/. The editor build now fails when a chunk bundles a peer dependency or calls require() at runtime, and a new e2e runs the template against the built packages (MAVONENGINE_USE_DIST=1) and opens the editor in Chrome. --- .github/workflows/node.js.yml | 24 ++++- .gitignore | 1 + e2e/editor-dist/run.mjs | 99 +++++++++++++++++++ package.json | 3 +- packages/editor/vite.config.mjs | 24 ++++- .../client/vite.config.js | 8 +- 6 files changed, 153 insertions(+), 6 deletions(-) create mode 100644 e2e/editor-dist/run.mjs diff --git a/.github/workflows/node.js.yml b/.github/workflows/node.js.yml index 30e5aaa..969c73b 100644 --- a/.github/workflows/node.js.yml +++ b/.github/workflows/node.js.yml @@ -74,12 +74,18 @@ jobs: npm link working-directory: ./packages/cli + - name: npm link editor locally + run: | + npm link + working-directory: ./packages/editor + # multiplayer-template has its own nested workspaces (client/server) that npm # workspaces does not handle recursively, so we need a separate npm ci here. - # Both in one call: each `npm link ` replaces the links of an earlier one + # Both in one call: each `npm link ` replaces the links of an earlier one. + # The editor is linked for the editor dist test; the other steps alias it to its src/ - name: Build multiplayer-template run: | - npm link @mavonengine/core @mavonengine/cli + npm link @mavonengine/core @mavonengine/cli @mavonengine/editor npm run typecheck npm run build npm run lint @@ -125,3 +131,17 @@ jobs: name: mcp-e2e-screenshots-${{ matrix.os }} path: e2e/mcp/screenshots if-no-files-found: ignore + + # The template aliases core and the editor to their src/, so nothing above loads their dist/. This runs + # the template against the built packages, like a project that installed them, and opens the editor + - name: Editor dist end-to-end test + timeout-minutes: 5 + run: npm run test:editor-dist-e2e + + - name: Upload editor dist end-to-end screenshots + if: failure() + uses: actions/upload-artifact@v4 + with: + name: editor-dist-e2e-screenshots-${{ matrix.os }} + path: e2e/editor-dist/screenshots + if-no-files-found: ignore diff --git a/.gitignore b/.gitignore index 0ec7630..129269a 100644 --- a/.gitignore +++ b/.gitignore @@ -30,3 +30,4 @@ packages/core/src/version.ts e2e/http-monitoring/screenshots e2e/scenes/screenshots e2e/mcp/screenshots +e2e/editor-dist/screenshots diff --git a/e2e/editor-dist/run.mjs b/e2e/editor-dist/run.mjs new file mode 100644 index 0000000..1a55309 --- /dev/null +++ b/e2e/editor-dist/run.mjs @@ -0,0 +1,99 @@ +/** + * End-to-end test for the built editor, the way a project that installed @mavonengine/core and + * @mavonengine/editor from npm loads them: + * + * The template's Vite config aliases both packages to their src/ in this repository, so the other tests + * never load dist/. Bundling problems (a missing external, a require() left in a chunk, a chunk that + * doesn't load) only show up in dist. Here MAVONENGINE_USE_DIST=1 turns the aliases off, so the template + * resolves the packages through node_modules, then the test opens the editor in Chrome and checks that + * its UI mounts with no errors. + * + * Needs Chrome, core and the editor built and npm-linked into the template, and the template's + * dependencies installed. + * + * node e2e/editor-dist/run.mjs + */ +import { mkdirSync, realpathSync, rmSync } from 'node:fs' +import { dirname, join } from 'node:path' +import process from 'node:process' +import { setTimeout as delay } from 'node:timers/promises' +import { fileURLToPath } from 'node:url' +import { chromium } from 'playwright-core' +import { base, check, log, repo, run, startServer, template } from '../helpers.mjs' + +const here = dirname(fileURLToPath(import.meta.url)) +const screenshots = join(here, 'screenshots') + +/** The package `name` in the template's node_modules must be this repository's, or the test checks a published one */ +function checkLinked(name, dir) { + const linked = realpathSync(join(template, 'node_modules', name)) + const expected = realpathSync(join(repo, 'packages', dir)) + check(linked === expected, `${name} in the template is this repository's packages/${dir}${linked === expected ? '' : ` (it is ${linked}, run npm link in packages/${dir} and the template)`}`) +} + +async function main() { + let server + let browser + const errors = [] + + rmSync(screenshots, { recursive: true, force: true }) + mkdirSync(screenshots, { recursive: true }) + + try { + log('\n== setup') + checkLinked('@mavonengine/core', 'core') + checkLinked('@mavonengine/editor', 'editor') + // Deps pre-bundled while the aliases pointed at src/ would be reused otherwise + rmSync(join(template, 'client/node_modules/.vite'), { recursive: true, force: true }) + await run('npm', ['run', 'build'], { cwd: join(template, 'server') }) + server = await startServer({ MAVONENGINE_DEV_CLIENT: '1', MAVONENGINE_USE_DIST: '1' }) + + log('\n== editor') + browser = await chromium.launch({ channel: 'chrome', headless: true, args: ['--use-gl=angle', '--use-angle=swiftshader', '--enable-unsafe-swiftshader'] }) + const page = await browser.newPage({ viewport: { width: 1400, height: 900 } }) + page.on('console', (message) => { + if (message.type() === 'error') + errors.push(message.text()) + }) + page.on('pageerror', error => errors.push(error.stack ?? error.message)) + + await page.goto(base) + await page.waitForFunction(() => window.Game?.activeScene?.name === 'main', null, { timeout: 60000, polling: 200 }) + check(true, 'the game loads with core from dist') + + await page.locator('canvas').click({ position: { x: 700, y: 450 }, force: true }) + await page.keyboard.press('Period') + await page.waitForFunction(() => !!window.Game.editor && document.title === 'MavonEngine | Editor', null, { timeout: 30000, polling: 200 }) + check(true, 'the editor boots') + + // The UI is loaded with a dynamic import after the editor boots; a chunk that fails to load is logged, not thrown, + // so stop waiting as soon as an error shows up + const sceneTab = page.getByRole('tab', { name: 'Scene' }) + const deadline = Date.now() + 30000 + while (!errors.length && Date.now() < deadline && !await sceneTab.isVisible()) + await delay(200) + check(await sceneTab.isVisible(), `the editor's UI mounts${errors.length ? `:\n${errors.join('\n')}` : ''}`) + + const loaded = await page.evaluate(() => performance.getEntriesByType('resource').map(e => e.name)) + check(loaded.some(url => url.includes('/editor/dist/')) && !loaded.some(url => url.includes('/editor/src/')), 'the editor came from dist, not src') + check(!loaded.some(url => url.includes('/core/src/')), 'core came from dist, not src') + await page.screenshot({ path: join(screenshots, 'editor.png') }) + + const relevant = errors.filter(e => !e.includes('favicon')) + check(relevant.length === 0, `no console errors${relevant.length ? `:\n${relevant.join('\n')}` : ''}`) + log('\nAll editor dist checks passed') + } + catch (err) { + log(`\n== server output\n${server?.output().slice(-4000)}`) + throw err + } + finally { + await browser?.close() + await server?.stop() + } +} + +main().catch((err) => { + console.error(err) + process.exitCode = 1 +}) diff --git a/package.json b/package.json index 8825306..89fbd0d 100644 --- a/package.json +++ b/package.json @@ -24,6 +24,7 @@ "test:plugin-e2e": "node e2e/plugin/run.mjs", "test:monitoring-e2e": "node e2e/http-monitoring/run.mjs", "test:scenes-e2e": "node e2e/scenes/run.mjs", - "test:mcp-e2e": "node e2e/mcp/run.mjs" + "test:mcp-e2e": "node e2e/mcp/run.mjs", + "test:editor-dist-e2e": "node e2e/editor-dist/run.mjs" } } diff --git a/packages/editor/vite.config.mjs b/packages/editor/vite.config.mjs index fe52fa6..fd19359 100644 --- a/packages/editor/vite.config.mjs +++ b/packages/editor/vite.config.mjs @@ -9,6 +9,11 @@ import glsl from 'vite-plugin-glsl' const EXTENDS_RE = /\bclass\s+\w+\s+extends\s+(?:ParticleEffect|Emitter)\b/ const ASSETS_RE = /Assets\.tsx$/ const TS_EXT_RE = /\.ts$/ +// Peer dependencies the game provides. A copy of one in the bundle means an `external` entry misses it +const PEER_IN_BUNDLE_RE = /[\\/]node_modules[\\/](?:react|react-dom|three|@mavonengine[\\/]core)[\\/]/ +// Rolldown's error for a CommonJS require() of an external it couldn't turn into an import. +// Browsers have no require, so the chunk throws when it loads +const REQUIRE_SHIM = 'in an environment that doesn\'t expose the `require` function' const editorRoot = dirname(fileURLToPath(import.meta.url)) const coreRoot = resolve(editorRoot, '../core') @@ -89,7 +94,7 @@ export default defineConfig(({ command }) => ({ formats: ['es'], }, rolldownOptions: { - external: id => ['react', 'react-dom', 'react/jsx-runtime', 'three'].includes(id) || id.startsWith('react/') || id.startsWith('@mavonengine/core') || id.startsWith(coreRoot), + external: id => ['react', 'react-dom', 'react/jsx-runtime', 'three'].includes(id) || id.startsWith('react/') || id.startsWith('react-dom/') || id.startsWith('three/') || id.startsWith('@mavonengine/core') || id.startsWith(coreRoot), plugins: [ { name: 'inject-css-into-mount-chunk', @@ -101,6 +106,23 @@ export default defineConfig(({ command }) => ({ mountChunk.code = `import './${cssFile}';\n${mountChunk.code}` }, }, + { + // Source builds alias the editor to src/, so a bundle that only breaks in the browser goes unnoticed there + name: 'check-bundle-for-browser', + generateBundle(_, bundle) { + const problems = [] + for (const chunk of Object.values(bundle)) { + if (chunk.type !== 'chunk') + continue + for (const id of chunk.moduleIds.filter(id => PEER_IN_BUNDLE_RE.test(id))) + problems.push(`${chunk.fileName} bundles ${id}, it should be external`) + if (chunk.code.includes(REQUIRE_SHIM)) + problems.push(`${chunk.fileName} calls require() at runtime, which fails in the browser`) + } + if (problems.length) + this.error(`The editor bundle would break in projects that install it:\n ${problems.join('\n ')}\nCheck build.rolldownOptions.external`) + }, + }, ], }, }, diff --git a/packages/multiplayer-template/client/vite.config.js b/packages/multiplayer-template/client/vite.config.js index bac2aa6..6944fb6 100644 --- a/packages/multiplayer-template/client/vite.config.js +++ b/packages/multiplayer-template/client/vite.config.js @@ -1,5 +1,6 @@ import { existsSync } from 'node:fs' import { dirname, resolve } from 'node:path' +import process from 'node:process' import { fileURLToPath } from 'node:url' import config from '@mavonengine/core/vite.config' import vue from '@vitejs/plugin-vue' @@ -10,6 +11,9 @@ const coreRoot = resolve(clientRoot, '../../core') const coreSrc = resolve(coreRoot, 'src') const editorRoot = resolve(clientRoot, '../../editor') const editorSrc = resolve(editorRoot, 'src/Editor.ts') +// MAVONENGINE_USE_DIST=1 resolves @mavonengine/core and @mavonengine/editor through node_modules, +// to their built dist like in a project that installed them, instead of the monorepo's source +const useSource = process.env.MAVONENGINE_USE_DIST !== '1' export default { ...config, @@ -26,8 +30,8 @@ export default { resolve: { ...config.resolve, alias: { - ...(existsSync(editorSrc) ? { '@mavonengine/editor': editorSrc } : {}), - ...(existsSync(coreSrc) ? { '@mavonengine/core': coreSrc } : {}), + ...(useSource && existsSync(editorSrc) ? { '@mavonengine/editor': editorSrc } : {}), + ...(useSource && existsSync(coreSrc) ? { '@mavonengine/core': coreSrc } : {}), '@template/server': resolve(templateRoot, 'server/src'), }, dedupe: ['@mavonengine/core', 'three', 'vue', 'react', 'react-dom', '@dimforge/rapier3d-compat'],