TC39 migration: flip experimentalDecorators to Stage 3 decorators (atomic)#18647
TC39 migration: flip experimentalDecorators to Stage 3 decorators (atomic)#18647RaananW wants to merge 21 commits into
Conversation
Rewrite Misc/decorators.ts serialize*/expandToProperty/nativeOverride/ addAccessorsForMaterialProperty to TC39 Stage 3 decorator signatures (returning ClassAccessorDecoratorResult where needed). Add GetDirectStoreFromMetadata to decorators.functions.ts for the TC39 context.metadata write path. Drop experimentalDecorators and set useDefineForClassFields:false in tsconfig.json/tsconfig.build.json, add esnext.decorators lib; add esnext.decorators + es2022.object to test lib. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add the accessor keyword to every @expandToProperty member across packages/dev/core (materials, PBR, lights, rendering, BakedVertexAnimation, standardMaterial, post-processes, layers, cameras, node blocks). Flip openpbrMaterial addAccessorsForMaterialProperty to the TC39 accessor pattern with explicit private backing fields. Edge cases: wrap self-referencing decorator args in arrow functions for deferred evaluation (LightBlock, ReflectionTextureBaseBlock, PBRMetallicRoughnessBlock); imageProcessing.ts mixin registers serialization metadata directly via GetDirectStoreFromMetadata; restructure @expandToProperty on explicit getters in backgroundMaterial and pbrSubSurfaceConfiguration. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ites Add the accessor keyword to every @expandToProperty member across the standalone material packages (cell, fire, fur, gradient, grid, lava, mix, normal, simple, terrain, triPlanar, water). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add the accessor keyword to the @expandToProperty members in fluentMaterial.pure.ts. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Rewrite XmlName/XmlAttr/XmlElem/XmlIgnore to TC39 decorator signatures that store field metadata via context.metadata, and rewrite AddXmlMeta/GetXmlFieldMeta to read from Symbol.metadata (walking the metadata prototype chain). Add a Symbol.metadata polyfill side-effect import so the decorated 3MF classes work under tree-shaken core imports. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…ta polyfill Configure vitest's esbuild transform with target es2021 and useDefineForClassFields:false so TC39 accessor decorators and class field assignment semantics match the tsc build. Polyfill Symbol.metadata in vitest.setup.ts because Node does not expose it natively yet. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
… props Rewrite nodeDecorator.editableInPropertyPage and the smart filters EditableInPropertyPage decorator to store editable-property descriptors on context.metadata, and add GetEditableProperties / GetSmartFilterEditableProperties helpers that walk the metadata prototype chain. Update the node/geometry/render-graph/particle/smart-filters editor property components, graphNode.ts, and inputNodePropertyComponent to read via the helpers instead of _propStore. customShaderBlock applies the decorator manually against Symbol.metadata. Fix flowGraphEditor RenderGenericPropStoreSections drift and the GUI editor String.at usage (bracket index). Add defensive Symbol.metadata polyfill imports for tree-shaken smart filter consumers. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Add the accessor keyword to all @property/@state/@query fields in the viewer Lit elements, type @query fields as | null, and move the @Property decorator on the environment property from the getter to the setter (accessor decorators cannot target a getter/setter pair). Add a Symbol.metadata polyfill import to the viewer package index because the viewer deep-imports core modules and bypasses core/index's polyfill. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
…pet transpilers Remove experimentalDecorators/emitDecoratorMetadata from the Monaco tsPipeline and snippetLoader compiler options so playground and snippet transpilation use TC39 Stage 3 decorator semantics like the rest of the repo. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Building or testing the sandbox has failed. If the tests failed, results can be found here: |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18647/merge/index.html#WGZLGJ#4600 Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves): https://playground.babylonjs.com/?snapshot=refs/pull/18647/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/18647/merge#BCU1XR#0 If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools. |
|
Smart Filters Editor is available to test at: |
|
You have made possible changes to the playground. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/18647/merge/ The snapshot playground with the CDN snapshot (only when available): Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly. |
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
RaananW
force-pushed
the
raananw-tc39-pr3-compiler-flip
branch
from
4ff0ba9 to
c9a4db9
Compare
|
Devhost visualization test reporter: |
1 similar comment
|
Devhost visualization test reporter: |
|
Smart Filters Editor is available to test at: |
|
You have changed file(s) that made possible changes to the sandbox. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/18647/merge/ |
|
You have made possible changes to the playground. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/18647/merge/ The snapshot playground with the CDN snapshot (only when available): Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly. |
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
| // `declare`, not `override`: the value is created by the base TargetCamera constructor. Under TC39 | ||
| // decorators a plain field redeclaration is materialized as a constructor field-init that runs after | ||
| // super() and would clobber that value back to undefined; `declare` keeps this a type-only narrowing. |
There was a problem hiding this comment.
Maybe include this comment on the other similar ones? I don't think ppl will understand the syntax.
There was a problem hiding this comment.
Only two declare sites exist (freeCamera.pure.ts:64 and flyCamera.pure.ts:101) and both already carry the identical comment block.
| } | ||
|
|
||
| // eslint-disable-next-line @typescript-eslint/naming-convention | ||
| const __bjsPropStoreKey = "__bjs_prop_store__"; |
There was a problem hiding this comment.
Would it be cleaner to use a Symbol here?
There was a problem hiding this comment.
Staing consistent with __bjsSerializableKey whcih is a string as well. But i don't mind doing that if you feel it is needed
…r Vite 8 Vite 8 (#18649) transforms via Rolldown/Oxc, which only lowers legacy (experimental) decorators and leaves TC39 Stage 3 decorators + the 'accessor' keyword untransformed, causing 'SyntaxError: Invalid or unexpected token' when Node imports @dev/core in the unit suite. Vite 8 also ignores the 'esbuild' config option entirely. Add a 'pre' Vite plugin that runs transformWithEsbuild on all non- node_modules .ts/.tsx files (esbuild does lower TC39 decorators), with useDefineForClassFields:false to preserve class-field semantics, and set oxc:false so Rolldown does not re-run its own transform. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
Extract the shared resolve-once wrapper into ApplyNativeOverride, used by both nativeOverride and nativeOverride.filter, removing the copy-paste introduced during the TC39 conversion while preserving exact call semantics. Add unit tests covering the JS-only path, native-override path, resolve-once/prototype-replacement behavior, and the filter predicate true/false branches (stubbing the _native global). Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Graph tools CI has failed you can find the test results at: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/TOOLS/refs/pull/18647/merge/testResults/ |
|
Building or testing the playground has failed. If the tests failed, results can be found here: |
|
Building or testing the sandbox has failed. If the tests failed, results can be found here: |
The deduped ApplyNativeOverride typed the native function with a 'this: This' binding, but the filter path invokes it without a this-bind (matching the original behavior). Cast to a this-less signature so tsc type-checks; the unit suite missed this because it transforms via esbuild without type-checking. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
|
Snapshot stored with reference name: Test environment: To test a playground add it to the URL, for example: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/refs/pull/18647/merge/index.html#WGZLGJ#4600 Links to test your changes to core in the published versions of the Babylon tools (does not contain changes you made to the tools themselves): https://playground.babylonjs.com/?snapshot=refs/pull/18647/merge To test the snapshot in the playground with a playground ID add it after the snapshot query string: https://playground.babylonjs.com/?snapshot=refs/pull/18647/merge#BCU1XR#0 If you made changes to the sandbox or playground in this PR, additional comments will be generated soon containing links to the dev versions of those tools. |
|
Smart Filters Editor is available to test at: |
|
You have changed file(s) that made possible changes to the sandbox. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/18647/merge/ |
|
You have made possible changes to the playground. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/18647/merge/ The snapshot playground with the CDN snapshot (only when available): Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly. |
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
… 8/Oxc) The viewer CI tests serve the viewer's own Lit-element source as raw TS (script type=module src=.../src/index.ts). Under Vite 8 (#18649) the dev server transforms via Oxc, which only lowers legacy decorators and leaves TC39 Stage 3 decorators + the 'accessor' keyword untransformed. The browser then throws 'SyntaxError: Invalid or unexpected token', the babylon-viewer custom element is never defined, and every Playwright test times out waiting for the (hidden, un-upgraded) element to become visible. Mirror the vitest fix in the viewer's vite.config.mjs: set oxc:false and add a 'pre' plugin that transforms .ts/.tsx via esbuild (which does lower TC39 decorators) with target/useDefineForClassFields matching tsconfig.build.json. Only the dev test server is affected; the published package is built by tsc. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
|
Smart Filters Editor is available to test at: |
|
You have changed file(s) that made possible changes to the sandbox. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/SANDBOX/refs/pull/18647/merge/ |
|
You have made possible changes to the playground. https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/PLAYGROUND/refs/pull/18647/merge/ The snapshot playground with the CDN snapshot (only when available): Note that neither Babylon scenes nor textures are uploaded to the snapshot directory, so some playgrounds won't work correctly. |
🟢 Memory Leak Test Results4 passed, 0 leaked out of 4 scenarios 🟢 All memory leak tests passed — no leaks detected. Passed Scenarios (4)
|
|
WebGL2 visualization test reporter: |
|
Visualization tests for WebGPU |
⚡ Performance Test Results🟢 All performance tests passed — no regressions detected. |
|
Let s merge Monday so we do not have to monitor tomorrow :-) |
TC39 migration: flip
experimentalDecorators→ Stage 3 decorators (atomic)Builds on merged #18631 (Symbol.metadata serialization store) and #18630 (Native ES5 downlevel pipeline). This PR flips the single root-level
experimentalDecoratorsflag off, switching the entire monorepo to TC39 Stage 3 decorator semantics in one atomic change, and converts every decorator implementation + accessor call-site accordingly.Per-commit breakdown
decorators.tsTC39 signatures (ClassAccessorDecoratorResult),decorators.functions.ts→context.metadata/GetDirectStoreFromMetadata;tsconfig.json/tsconfig.build.json/tsconfig.test.json(dropexperimentalDecorators, addesnext.decorators/es2022.objectlibs).accessorkeyword on every@expandToPropertysite indev/core;openpbrMaterial@addAccessorsForMaterialPropertynew accessor pattern.dev/materials/src/*.fluentMaterial.pure.tsaccessor conversions.XmlName/XmlAttr/XmlElem/XmlIgnore→ TC39context.metadata+AddXmlMeta/GetXmlFieldMeta.useDefineForClassFieldsclass-field semantics +Symbol.metadatapolyfill in setup.nodeDecorator.ts_propStore→Symbol.metadata+ exportedGetEditableProperties(); smartFilterseditableInPropertyPage+GetSmartFilterEditableProperties(); editor consumers updated.accessoron viewer Lit@property/@state/@query;@query→| null;@propertyonenvironmentmoved to setter.experimentalDecoratorsfrom MonacotsPipeline.ts+snippetLoader.ts.Direnttype fix.WebCamInputBlockself-reference in decorator arg (TS2449 arrow-wrap).Symbol.metadatapolyfill before decorated classes in all entry styles (barrel/deep/pure) via a module-load-anchoredconstin the decorator infra.declare(notoverride) the two fields clobbered by base-constructor init (see rationale below).ts.transpileModule(not Babel) — see Native resolution below.Breaking changes for consumers
accessorkeyword required on any class member decorated with@expandToPropertyor@addAccessorsForMaterialProperty(these areClassAccessorDecorators under TC39)._propStoreremoved — use exportedGetEditableProperties()/GetSmartFilterEditableProperties()instead.Symbol.metadatareplaces class-name maps for decorator metadata.The 8 UMD build targets move from ES5 to es2015 (TC39
accessorrequires es2015+; tsc rejects an ES5 target foraccessorsource with TS18045, and esbuild refuses ES5 for classes). Consumers (Playground, Sandbox, npm) get the es2015 UMD directly. Babylon Native, which needs ES5 for Chakra, gets a dedicated ES5 downlevel of the built bundle — see below.override→declarerationale (commit 14)Under TC39, decorators force
useDefineForClassFields(native define semantics). A type-onlyoverridefield with no initializer previously emitted nothing; under define semantics it would emit a field that clobbers a base-constructor-set value withundefined. In practice only two fields are actually created by a base constructor and get clobbered this way —FreeCamera.movementandFlyCamera.movement(both created by theTargetCameraconstructor) — so only those two are converted to type-onlydeclarenarrowings.declarealso emits nothing, so the conversion is provably behavior-identical to the old compiler while remaining correct under define semantics.A blanket
override→declaresweep (~136 fields) was considered and rejected to keep the diff reviewable: the minimal fix is what the full unit suite requires, and the type checker + unit suite guard against any future regression if a new narrowing field ever starts clobbering a base value.Pure-entry metadata fix (commit 13)
tsc's TC39 emit computes
context.metadatafromSymbol.metadataat the top of each classstatic {}block, before the decorator factory runs. The polyfill was only imported by the barrel (index.ts), so@babylonjs/core/pure(which eagerly evaluates decorated FrameGraph/node blocks) sawcontext.metadata === undefinedand every scene crashed at load (ES6 visualization timeouts). Fix: an exported module-loadconst MetadataSymbol = GetMetadataSymbol()indecorators.functions.ts, referenced by the retained decorator helpers, so the polyfill runs before any importing class body — in every entry style. Theconst X = fn()idiom keeps the module classified pure by the side-effect detector and is preserved by bundlers when reachable, so tree-shaking is unaffected.Native (Chakra) resolution
TC39
accessorrequires an es2015+ target (tsc emits TS18045 foraccessorat an ES5 target, and esbuild refuses ES5 for classes), so the 8 UMD tsconfigs build at es2015. Babylon Native runs the builtbabylon.max.jsunder an (archived) ChakraCore build. A first attempt reused the PR2@babel/preset-envpass to re-lower es2015→ES5, but Babel emits_wrapNativeSuper+Reflect.construct+_isNativeReflectConstructfor classes that extend native built-ins (Error,Array). Those helpers execute at class-definition (load) time and hard-crash this Chakra build (exit 3, uncatchable — no JS stack). V8/node tolerate them, so the crash is invisible locally andes-check(parse-only) does not catch it.Fix:
scripts/downlevelNativeScripts.mjsnow downlevels the built UMD withts.transpileModuletargeting ES5 instead of Babel. tsc lowers classes with__extendsand never emitsReflect.construct/_wrapNativeSuper, matching the ES5 emit master has shipped to Chakra for years. Verified locally on the builtbabylon.max.js: the downleveled output passeses-check es5, loads in node, and contains zeroReflect.construct/_wrapNativeSuperoccurrences. (transpileModule— syntactic transpile, no type-check — is used because a fulltsccheck overflows on the multi-MB single-file bundle; syntactic lowering is exactly what a downlevel needs.)Validation
format:checklint:check(eslint + treeshaking + side-effects-sync)check:treeshaking-all(16/16)build:es6build:umd(es2015)test:unitbabylon.max.js→ts.transpileModuleES5 downlevel +es-check es5Reflect.construct/_wrapNativeSuper, loads in node)experimentalDecorators(source)