Skip to content

fix(build): preserve live bindings of mutable UMD exports (pc.app)#8837

Merged
mvaligursky merged 1 commit into
mainfrom
mv-fix-umd-pc-app-live-binding
Jun 4, 2026
Merged

fix(build): preserve live bindings of mutable UMD exports (pc.app)#8837
mvaligursky merged 1 commit into
mainfrom
mv-fix-umd-pc-app-live-binding

Conversation

@mvaligursky
Copy link
Copy Markdown
Contributor

@mvaligursky mvaligursky commented Jun 4, 2026

Summary

Since 2.19.0, pc.app is permanently null in the UMD / global build (playcanvas.js), breaking any project or script that reads the global pc.app (e.g. the classic orbit-camera mouse-input script used by the WebXR AR Starter Kit). The ESM build is unaffected.

Fixes #8836. Also explains forum thread #42337.

Root cause

2.19.0 migrated the JS builds to esbuild (#8722). The UMD output wraps the engine as an esbuild IIFE whose exports are live getters (app: () => app), then bridges that namespace to the outer UMD exports (= window.pc) via a hand-written footer:

var pc = (() => { … __export(index_exports, { app: () => app, … }); return __toCommonJS(index_exports); })();
Object.assign(exports, pc);   // ← snapshots every getter to its load-time value

Object.assign invokes each getter once and copies the value. At module-load time no application exists, so app is null. window.pc.app becomes a frozen null data property; when an app is later constructed and the inner module's app = this runs, window.pc.app never updates.

This only affects mutable exports. app is the one that bites in practice. Scripts using this.app are unaffected (which is why the regression looked project-specific), but scripts reading the global pc.app break.

Fix

Copy the property descriptors rather than the values, preserving the getter (and thus the live binding) on the UMD namespace:

Object.defineProperties(exports, Object.getOwnPropertyDescriptors(pc));

Verification

Built both UMD bundles and loaded them as a browser global (window.pc) in a vm sandbox, then constructed an AppBase:

build pc.app descriptor pc.app === new AppBase(canvas)
before (Object.assign) value = null (frozen) false — stays null
after (Object.defineProperties) getter (live) true — updates correctly

All 1272 exports remain present; only the binding semantics for mutable exports change.

A regression test would ideally assert pc.app updates after construction in the UMD build, but the current mocha suite runs against src/ (ESM, where live bindings already work), so there's no build-output test harness to hook into — noting it here as a follow-up.

🤖 Generated with Claude Code

@claude
fix(build): preserve live bindings of mutable UMD exports (pc.app)
6686fbf 
The UMD footer bridged the esbuild IIFE namespace to the global `exports`
object with `Object.assign(exports, pc)`. `Object.assign` invokes each
getter once and copies its *value*, so the mutable `app` export (null
until an application is created) was snapshotted to null and `pc.app`
never updated.

Copy the property descriptors instead, so the getter — and therefore the
live binding — is preserved on the UMD namespace. The ESM build was
already unaffected as it uses native live bindings.

Fixes #8836

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
@mvaligursky mvaligursky self-assigned this Jun 4, 2026
@mvaligursky mvaligursky mentioned this pull request Jun 4, 2026
Closed
kpal81xd
kpal81xd approved these changes Jun 4, 2026
View reviewed changes
Copy link
Copy Markdown
Contributor

@kpal81xd kpal81xd left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good and makes sense since values were copied and not the references (which the descriptors include via getters/setters). Just testing in the Editor to confirm

@kpal81xd
Copy link
Copy Markdown
Contributor

kpal81xd commented Jun 4, 2026

Verified all good in the Editor - Maybe worth adding a test

@mvaligursky mvaligursky merged commit 2aa7112 into main Jun 4, 2026
8 checks passed
@mvaligursky mvaligursky deleted the mv-fix-umd-pc-app-live-binding branch June 4, 2026 10:42
mvaligursky added a commit that referenced this pull request Jun 4, 2026
@mvaligursky @claude
fix(build): preserve live bindings of mutable UMD exports (pc.app) (#…
668b11a 
…8837)

The UMD footer bridged the esbuild IIFE namespace to the global `exports`
object with `Object.assign(exports, pc)`. `Object.assign` invokes each
getter once and copies its *value*, so the mutable `app` export (null
until an application is created) was snapshotted to null and `pc.app`
never updated.

Copy the property descriptors instead, so the getter — and therefore the
live binding — is preserved on the UMD namespace. The ESM build was
already unaffected as it uses native live bindings.

Fixes #8836

Co-authored-by: Martin Valigursky <mvaligursky@snapchat.com>
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
@kpal81xd kpal81xd mentioned this pull request Jun 4, 2026
Open
4 tasks
@mvaligursky mvaligursky mentioned this pull request Jun 4, 2026
Merged
mvaligursky pushed a commit that referenced this pull request Jun 4, 2026
@claude
docs/fix(build): address PR review on UMD footer
93f41a2 
- 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>
mvaligursky added a commit that referenced this pull request Jun 4, 2026
@mvaligursky @claude
fix(build): keep UMD exports overridable while preserving live bindin…
79a559a 
…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>
mvaligursky added a commit that referenced this pull request Jun 4, 2026
@mvaligursky @claude
fix(build): keep UMD exports overridable while preserving live bindin…
09346d8 
…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>
@kpal81xd kpal81xd mentioned this pull request Jun 4, 2026
Open
kpal81xd added a commit that referenced this pull request Jun 4, 2026
@kpal81xd
test(build): add post-build tests for ESM/UMD bundles and API surface…
fe87182 
… 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@kpal81xd kpal81xd kpal81xd approved these changes

Assignees

@mvaligursky mvaligursky

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

pc.app - is not available. Breaking projects.

2 participants

@mvaligursky @kpal81xd