fix(build): keep UMD exports overridable while preserving live bindings by mvaligursky · Pull Request #8839 · playcanvas/engine

mvaligursky · 2026-06-04T12:12:22Z

Summary

Follow-up to #8837. That PR fixed the live binding of mutable UMD exports (notably pc.app, see #8836) but, as a side effect, made every UMD export getter-only. This silently broke any consumer that reassigns a member of the global pc namespace.

The most visible casualty is the Editor's classic-script worker (classic-script.worker.ts), which parses scripts by overriding pc.createScript / pc.registerScript with stubs:

pc.createScript = function (name, app, script) { /* stub */ };
pc.registerScript = function (script, name, app) { /* stub */ };

After #8837 those assignments hit a getter-only property and don't take effect, so the engine's real createScript → registerScript runs in the worker and throws:

Uncaught TypeError: Cannot read properties of undefined (reading 'scripts')
  at registerScript  // const registry = app ? app.scripts : AppBase.getApplication().scripts;

AppBase.getApplication() is undefined in the worker (no application is ever created there), so .scripts throws. Reported on the forum (classic script parsing broken in 2.19.3).

Fix

Wrap each export getter in an accessor that has both a getter and a setter:

for (const [key, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(pc))) {
    Object.defineProperty(exports, key, descriptor.get ? {
        get: descriptor.get,
        set: (value) => Object.defineProperty(exports, key, { value, writable: true, enumerable: true, configurable: true }),
        enumerable: descriptor.enumerable,
        configurable: true
    } : descriptor);
}

get delegates to the live binding → pc.app still updates (keeps pc.app - is not available. Breaking projects. #8836 fixed).
set redefines the property as a writable value → pc.createScript = … and similar overrides take effect again.

This restores the writable-export semantics of the pre-2.19 (Rollup) UMD build without regressing the live-binding fix.

Verification

Built the UMD bundle, loaded it as a browser global in a vm sandbox:

check	result
`Object.keys(pc).length`	1272 (unchanged)
`pc.app` before app / after `new pc.AppBase()`	`null` → `=== app` (live) ✅
`pc.createScript = stub` takes effect	✅ (no throw)
`pc.registerScript = stub` takes effect	✅
`pc.app` still live after overriding other members	✅
`pc.script.createLoadingScreen = …`	✅

Before this change (current main / 2.19.3) the override assignments throw "Cannot set property createScript … which has only a getter" and silently leave the engine implementation in place — reproducing the worker crash.

Note

There's also a latent footgun worth a separate look: registerScript hard-requires AppBase.getApplication() when no app is passed, which is fragile in app-less contexts (workers, headless parsing). Out of scope here.

🤖 Generated with Claude Code

#8837 fixed the live binding of mutable UMD exports (notably `pc.app`) by copying the esbuild namespace getters onto `exports` with `Object.defineProperties(exports, Object.getOwnPropertyDescriptors(pc))`. That made every export getter-only, which silently broke consumers that reassign members of the global `pc` namespace — e.g. the Editor's classic-script worker overrides `pc.createScript` / `pc.registerScript`. With the assignment failing, the engine's real `createScript` ran in the worker and threw `Cannot read properties of undefined (reading 'scripts')` from `AppBase.getApplication()` (no application exists in the worker). Wrap each export getter in an accessor that delegates reads to the live binding (so `pc.app` still updates) but also has a setter, so assignments like `pc.createScript = ...` redefine the property as a writable value and take effect again. Restores the writable-export behaviour of the pre-2.19 (Rollup) UMD build without regressing the live-binding fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

mvaligursky · 2026-06-04T12:15:26Z

Reproduced the exact reported failure (parsing posteffect-bloom.js in the Editor's classic-script worker) by replaying the worker flow against the UMD build — importScripts(engine) → install the pc.createScript/pc.registerScript stubs → parse a classic script that calls pc.createScript('bloom'):

build	worker stub used	parse result
current `main` / 2.19.3 (getter-only exports)	❌ no — assignment silently fails	`TypeError: Cannot read properties of undefined (reading 'scripts')` from `registerScript` → `AppBase.getApplication().scripts`
with this PR	✅ yes	parses cleanly, no error

Matches the reported stack trace exactly:

registerScript @ playcanvas-2.19.3.js:100723
createScript   @ playcanvas-2.19.3.js:100700   // engine fn ran because the worker override didn't take
posteffect-bloom.js:260
classic-script.worker.ts:429

Copilot

Pull request overview

This PR updates the UMD build footer bridge logic so global pc exports retain live-binding behavior (e.g. pc.app) while also remaining overridable by consumers (e.g. pc.createScript = stub), addressing a regression introduced by the descriptor-copy approach from #8837.

Changes:

Replace the UMD Object.defineProperties bridge with per-export accessor wrappers that add a setter to allow overrides.
Preserve live bindings by delegating reads to the original export getters while allowing writes to “lock in” an overridden value.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

mvaligursky · 2026-06-04T12:28:48Z

+    //   - keeps the property writable, so consumers can still override members of `pc` (e.g. the
+    //     Editor's classic-script worker reassigns `pc.createScript`). A bare descriptor copy makes
+    //     the export getter-only and breaks such overrides
+    //     (see https://github.com/playcanvas/engine/issues/8836).


Good catch — fixed in 93f41a2. The second reference now points to #8837 (the PR that introduced the getter-only descriptor copy).

mvaligursky · 2026-06-04T12:28:49Z

+    return /* js */ `for (const [key, descriptor] of Object.entries(Object.getOwnPropertyDescriptors(pc))) {
+\tObject.defineProperty(exports, key, descriptor.get ? {
+\t\tget: descriptor.get,
+\t\tset: (value) => Object.defineProperty(exports, key, { value, writable: true, enumerable: true, configurable: true }),


Fixed in 93f41a2 — the setter now uses enumerable: descriptor.enumerable for parity with the original export. (In practice all accessor-path exports are enumerable; only the non-enumerable __esModule marker takes the plain-descriptor branch, so there's no behavioural change — but the parity is correct.)

- Point the override-regression reference to #8837 (the PR that made exports getter-only), not #8836 (the pc.app live-binding issue). - Preserve the original export's enumerability in the override setter instead of hardcoding `enumerable: true`, keeping attribute parity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Copilot

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

…gs (#8839) * fix(build): keep UMD exports overridable while preserving live bindings #8837 fixed the live binding of mutable UMD exports (notably `pc.app`) by copying the esbuild namespace getters onto `exports` with `Object.defineProperties(exports, Object.getOwnPropertyDescriptors(pc))`. That made every export getter-only, which silently broke consumers that reassign members of the global `pc` namespace — e.g. the Editor's classic-script worker overrides `pc.createScript` / `pc.registerScript`. With the assignment failing, the engine's real `createScript` ran in the worker and threw `Cannot read properties of undefined (reading 'scripts')` from `AppBase.getApplication()` (no application exists in the worker). Wrap each export getter in an accessor that delegates reads to the live binding (so `pc.app` still updates) but also has a setter, so assignments like `pc.createScript = ...` redefine the property as a writable value and take effect again. Restores the writable-export behaviour of the pre-2.19 (Rollup) UMD build without regressing the live-binding fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs/fix(build): address PR review on UMD footer - Point the override-regression reference to #8837 (the PR that made exports getter-only), not #8836 (the pc.app live-binding issue). - Preserve the original export's enumerability in the override setter instead of hardcoding `enumerable: true`, keeping attribute parity. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Martin Valigursky <mvaligursky@snapchat.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

… gate The mocha suite runs against src/ only, and CI just runs publint on the bundles, so build-output regressions ship undetected (e.g. #8836 pc.app frozen null, #8839 getter-only exports breaking pc.createScript overrides). Adds: - test/bundles/: a post-build mocha suite (npm run test:build) that loads the built bundles (rel/dbg/prf/min x UMD/ESM) and asserts they load, boot an Application, export-parity with src/index.js, pc.app live binding, exports overridable, and that every UMD export is a live+overridable accessor. - api-extractor: a type-surface gate. npm run api:check verifies the built .d.ts against the committed etc/playcanvas.api.md; npm run api:update regenerates it. Wired into the CI typescript-declarations job. - CI: run test:build in the build job; api:check in the declarations job. Closes #8838

… gate The mocha suite runs against src/ and CI only publints the bundles, so build-output regressions ship undetected (e.g. #8836 pc.app null in UMD, #8839 getter-only exports breaking pc.createScript overrides). - test/bundles/: post-build mocha suite (npm run test:build) loading the built bundles (rel/dbg/prf/min x UMD/ESM) - each loads (UMD global + CJS, ESM import) and boots an Application; runtime exports match src/index.js; pc.app live binding (#8836/#8837); UMD exports overridable (#8839); plus a blanket invariant that every UMD export is a live + overridable accessor. - api-extractor: public API surface gate. npm run api:check verifies the built .d.ts against etc/playcanvas.api.md (compact colored diff on failure); npm run api:update regenerates it. Scratch report under build/.api-extractor/. New dev dependency: @microsoft/api-extractor. - CI: run test:build in the build job; api:check in the typescript-declarations job. Closes #8838

The mocha suite runs against src/ and CI only publints the bundles, so build-output regressions ship undetected (e.g. #8836 pc.app null in UMD, #8839 getter-only exports breaking pc.createScript overrides). test/bundles/: a post-build mocha suite (npm run test:build) that loads the built bundles (rel/dbg/prf/min x UMD/ESM) - each loads (UMD as a browser global and via CommonJS, ESM via import) and boots an Application; runtime exports match src/index.js; pc.app live binding (#8836/#8837); UMD exports overridable (#8839); plus a blanket invariant that every UMD export is a live + overridable accessor. Runs in the build CI job. Closes #8838

mvaligursky self-assigned this Jun 4, 2026

mvaligursky mentioned this pull request Jun 4, 2026

pc.app - is not available. Breaking projects. #8836

Closed

vercel Bot deployed to Preview – engine-api-docs June 4, 2026 12:12 View deployment

vercel Bot deployed to Preview – engine June 4, 2026 12:13 View deployment

mvaligursky requested a review from Copilot June 4, 2026 12:19

Copilot started reviewing on behalf of mvaligursky June 4, 2026 12:19 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

vercel Bot deployed to Preview – engine-api-docs June 4, 2026 12:27 View deployment

vercel Bot deployed to Preview – engine June 4, 2026 12:28 View deployment

mvaligursky requested a review from Copilot June 4, 2026 12:29

Copilot started reviewing on behalf of mvaligursky June 4, 2026 12:30 View session

Copilot AI reviewed Jun 4, 2026

View reviewed changes

mvaligursky merged commit 79a559a into main Jun 4, 2026
9 checks passed

mvaligursky deleted the mv-fix-umd-exports-overridable branch June 4, 2026 12:34

mvaligursky mentioned this pull request Jun 4, 2026

Add post-build tests for the main build targets (ESM and UMD) #8838

Closed

4 tasks

kpal81xd mentioned this pull request Jun 4, 2026

test(build): add post-build tests for ESM/UMD bundles and API surface gate #8845

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(build): keep UMD exports overridable while preserving live bindings#8839

fix(build): keep UMD exports overridable while preserving live bindings#8839
mvaligursky merged 2 commits into
mainfrom
mv-fix-umd-exports-overridable

mvaligursky commented Jun 4, 2026

Uh oh!

mvaligursky commented Jun 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

mvaligursky Jun 4, 2026

Uh oh!

mvaligursky Jun 4, 2026

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

mvaligursky commented Jun 4, 2026

Summary

Fix

Verification

Note

Uh oh!

mvaligursky commented Jun 4, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

mvaligursky Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

mvaligursky Jun 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants