[RFC/Discussion] Migrate from Experimental Decorators to TC39 Stage 3 Decorators#18142
[RFC/Discussion] Migrate from Experimental Decorators to TC39 Stage 3 Decorators#18142RaananW wants to merge 6 commits intoBabylonJS:masterfrom
Conversation
|
Please make sure to label your PR with "bug", "new feature" or "breaking change" label(s). |
814ab01 to
59b6c5f
Compare
|
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/18142/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/18142/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/18142/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: |
|
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: |
|
Graph tools CI has failed you can find the test results at: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/TOOLS/refs/pull/18142/merge/testResults/ |
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
|
Smart Filters Editor is available to test at: |
|
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: |
|
Graph tools CI has failed you can find the test results at: https://snapshots-cvgtc2eugrd3cgfd.z01.azurefd.net/TOOLS/refs/pull/18142/merge/testResults/ |
|
Visualization tests for WebGPU |
|
WebGL2 visualization test reporter: |
- Rewrite all decorator definitions for TC39 (decorators.ts, decorators.functions.ts, nodeDecorator.ts) - Use Symbol.metadata for per-class metadata storage - Add 'accessor' keyword to all @expandToProperty and @addAccessorsForMaterialProperty properties - Flip addAccessorsForMaterialProperty pattern: decorator on public accessor - Rewrite 3MF XML decorators and Smart Filters decorators for TC39 - Update Lit viewer components with accessor keyword for @property/@state/@query - Remove experimentalDecorators from all tsconfig files - Add esnext.decorators and es2022.object to lib - Bump UMD build targets from ES5 to ES2015 (accessor requires it) - Replace _propStore access with getEditableProperties() helper - Add unit tests for decorator metadata and expandToProperty - Fix class self-references in decorator arguments (TS2449) - Remove experimentalDecorators from Playground/Snippet Loader compiler options
- Resolve merge conflicts in convertShaders.ts and tsconfig.build.json - Add useDefineForClassFields: false to prevent class field override issues - Add 'declare' to type-narrowing _effectWrapper overrides in 18 PostProcess subclasses to prevent TC39 decorator lowering from emitting void 0 assignments that overwrite parent constructor values - Rename getEditableProperties/getSmartFilterEditableProperties to PascalCase per repo naming convention - Add eslint-disable for internal double-underscore metadata keys - Fix test assertion in decorators.test.ts (property stored under key name, not sourceName) - Add Symbol.metadata polyfill to vitest.setup.ts for Node.js compatibility - Fix missing curly braces in xml.interfaces.ts - Update customShaderBlock.test.ts: className is empty string with TC39 decorators (no target available in decorator context)
…ide for type-narrowing fields, fix barrel import, bump dictionary mode threshold
da742fe to
a999fe7
Compare
I'm assuming babel can still take the results to translate to ES5 if necessary? |
It should be possible. I can try finding the babel rule for that. |
We already do this in a number of places when building a bundle, so now we will have to do this for the UMD files also. |
- Keep TC39 decorator imports (GetDirectStoreFromMetadata, GetEditableProperties, GetSmartFilterEditableProperties) - Keep es2015 target for UMD builds (TC39 decorators require es2015+) - Keep noImplicitAny and remove experimentalDecorators from tsconfigs - Add gltf2 interface stub from master to vitest config alongside esbuild config - Accept master's package-lock.json
…ttern Fields added on master (subsurface*, geometryThinWalled) used the old experimental decorator pattern. Migrate them to use TC39 'accessor' keyword with addAccessorsForMaterialProperty. Also remove deprecated 'baseUrl' from tsconfig.json (not needed with explicit paths, and deprecated in TS 6.0).
|
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/18142/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/18142/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/18142/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: |
[RFC/Discussion] Migrate from Experimental Decorators to TC39 Stage 3 Decorators
Motivation
Babylon.js currently uses TypeScript's
experimentalDecorators— a legacy implementation based on an early (now-abandoned) TC39 proposal. TypeScript 5.0+ ships native support for the TC39 Stage 3 decorator proposal, which has reached consensus and is being implemented by browsers and runtimes.Benefits of Moving to TC39 Standard Decorators
experimentalDecoratorsflag is a legacy feature. The TC39 proposal is the standard path forward and will eventually be natively supported by all JavaScript engines.tsconfig.jsonflag. This simplifies configuration for both the library and consumers.Symbol.metadataprovides a clean, per-class metadata storage mechanism that naturally follows the prototype chain — no more manual class-name-based maps.accessorkeyword andClassAccessorDecoratorResultprovide a first-class way to intercept property access, replacing fragileObject.definePropertyhacks on prototypes.__esDecorate/__runInitializershelpers and aSymbol.metadatapolyfill automatically.Breaking Changes
1.
accessorkeyword on serialized/expandable propertiesAll properties decorated with
@expandToProperty(and related serialization decorators that useexpandToProperty) now require theaccessorkeyword. This is a TC39 requirement for decorators that need to intercept get/set.Before:
After:
Impact: Any subclass that overrides these properties must also use
accessor. Theaccessorkeyword creates an auto-getter/setter pair, so direct field access patterns still work identically at runtime.2.
Symbol.metadatareplaces class-name-based metadata mapsThe old system stored decorator metadata in a global map keyed by
getClassName(). The new system usesSymbol.metadataon each class constructor, which is the TC39 standard mechanism.Impact: Code that directly accessed
DecoratorInitialStoreorMergedStorefromdecorators.functions.tsmust migrate to the newGetDirectStore/GetMergedStorefunctions that read fromSymbol.metadata. In practice, this is internal machinery — most consumers never touch these directly.3.
_propStoreremoved — usegetEditableProperties()helperThe
editableInPropertyPagedecorator previously stored property metadata on a static_propStoreproperty on each class. This has been replaced withSymbol.metadata-based storage.Before:
After:
Impact: Any custom editor or tooling that reads
_propStoremust migrate to the newgetEditableProperties()function. Similarly, Smart Filters consumers should usegetSmartFilterEditableProperties().4. UMD build target bumped from ES5 to ES2015
The
accessorkeyword requires at minimum ES2015. All 8 UMDtsconfig.build.jsonfiles were updated from"target": "ES5"to"target": "es2015".Impact: The UMD bundles will no longer run in environments that only support ES5 (e.g., IE11). Given that Babylon.js already requires WebGL2/WebGPU, this is not expected to be a practical concern.
5.
addAccessorsForMaterialPropertypattern changeThe
addAccessorsForMaterialPropertydecorator is now anaccessordecorator applied directly to the public property, rather than being called on a private backing property.Before:
After:
What Was Done
Core Decorator Rewrites
packages/dev/core/src/Misc/decorators.ts: Complete TC39 rewrite of all decorator functions (serialize*,expandToProperty,nativeOverride,addAccessorsForMaterialProperty). All accessor decorators are now generic (<This, V>) and returnClassAccessorDecoratorResult.packages/dev/core/src/Misc/decorators.functions.ts: Rewritten to useSymbol.metadatawith a__bjs_serializable__key.GetMergedStorewalks the metadata prototype chain with WeakMap caching.packages/dev/core/src/Decorators/nodeDecorator.ts: Rewritten with__bjs_prop_store__metadata key. New exportedgetEditableProperties(target)helper.packages/dev/smartFilters/src/editorUtils/editableInPropertyPage.ts: Rewritten with__bjs_sf_prop_store__metadata key. NewgetSmartFilterEditableProperties(target)helper.Call-Site Updates (74 files, ~300 decorator usages)
@expandToProperty: Addedaccessorkeyword to all 294 usages.openpbrMaterial.ts: All 66@addAccessorsForMaterialPropertyusages flipped to the new pattern._propStoreaccess togetEditableProperties()/getSmartFilterEditableProperties().Framework-Specific Fixes
accessorkeyword to all@property()/@state()/@query()decorated fields. Fixed@querytypes to use| null. Fixedenvironmentproperty: moved@property()from getter to setter.XmlName,XmlAttr,XmlElem,XmlIgnoreto usecontext.metadatawithAddXmlMeta/GetXmlFieldMetahelpers.TypeScript Configuration
tsconfig.build.json: RemovedexperimentalDecorators: true, added"esnext.decorators"and"es2022.object"tolib.tsconfig.json: RemovedexperimentalDecorators: true.tsconfig.test.json: Added"esnext.decorators"and"es2022.object"tolib.tsconfig.build.jsonfiles: Changed target fromES5toes2015.experimentalDecoratorsfrom Monaco/TypeScript compiler options.Edge Cases Resolved
LightBlock,ReflectionTextureBaseBlock,PBRMetallicRoughnessBlock) referenced their own class inonValidationcallbacks passed to decorators. Wrapped in arrow functions for deferred evaluation.WebCamInputBlock: Class self-reference inoptionsgetter — similarly deferred.imageProcessing.ts: Had a manual decorator call that bypassed the decorator API — replaced with direct metadata store access.backgroundMaterial.ts/pbrSubSurfaceConfiguration.ts: Had@expandToPropertyon explicit getters (not valid foraccessordecorators) — restructured.String.at()in GUI editor: Not available with current lib settings — replaced with bracket index access.convertShaders.ts: Fixed pre-existingDirenttype issue unrelated to decorators.Testing
Build Verification
build:es6— Full NX build passes: 10 projects + 29 dependencies compiled successfully.build:umd— Full NX build passes: 7 projects + 25 dependencies compiled successfully.Unit Tests
✅ New decorator test suite (
packages/dev/core/test/unit/Decorators/decorators.test.ts):@serialize*decorators correctly store metadata viaSymbol.metadataGetMergedStoreproperly walks the prototype chain (parent + child metadata merged)@expandToPropertycorrectly intercepts get/set on accessor properties✅ Runtime integration test (
packages/dev/core/test/unit/Decorators/decorators.inline-test.ts):expandToPropertyauto-syncs backing propertiesWhat Still Needs Testing
experimentalDecoratorsFiles Changed (74 files)
Click to expand full file list
Core decorator infrastructure:
packages/dev/core/src/Misc/decorators.tspackages/dev/core/src/Misc/decorators.functions.tspackages/dev/core/src/Decorators/nodeDecorator.tsTypeScript configuration:
tsconfig.build.jsontsconfig.jsontsconfig.test.jsonpackages/public/umd/*/tsconfig.build.jsonMaterials with @expandToProperty (36 files):
packages/dev/core/src/BakedVertexAnimation/bakedVertexAnimationManager.tspackages/dev/core/src/Lights/light.tspackages/dev/core/src/Materials/Background/backgroundMaterial.tspackages/dev/core/src/Materials/GaussianSplatting/gaussianSplattingSolidColorMaterialPlugin.tspackages/dev/core/src/Materials/PBR/openpbrMaterial.tspackages/dev/core/src/Materials/PBR/pbrAnisotropicConfiguration.tspackages/dev/core/src/Materials/PBR/pbrBRDFConfiguration.tspackages/dev/core/src/Materials/PBR/pbrBaseMaterial.tspackages/dev/core/src/Materials/PBR/pbrBaseSimpleMaterial.tspackages/dev/core/src/Materials/PBR/pbrClearCoatConfiguration.tspackages/dev/core/src/Materials/PBR/pbrIridescenceConfiguration.tspackages/dev/core/src/Materials/PBR/pbrMaterial.tspackages/dev/core/src/Materials/PBR/pbrMetallicRoughnessMaterial.tspackages/dev/core/src/Materials/PBR/pbrSheenConfiguration.tspackages/dev/core/src/Materials/PBR/pbrSpecularGlossinessMaterial.tspackages/dev/core/src/Materials/PBR/pbrSubSurfaceConfiguration.tspackages/dev/core/src/Materials/imageProcessing.tspackages/dev/core/src/Materials/material.decalMapConfiguration.tspackages/dev/core/src/Materials/material.detailMapConfiguration.tspackages/dev/core/src/Materials/meshDebugPluginMaterial.tspackages/dev/core/src/Materials/standardMaterial.tspackages/dev/core/src/Rendering/GlobalIllumination/giRSMManager.tspackages/dev/core/src/Rendering/IBLShadows/iblShadowsPluginMaterial.tspackages/dev/core/src/Rendering/reflectiveShadowMap.tspackages/dev/gui/src/3D/materials/fluent/fluentMaterial.tspackages/dev/materials/src/cell/cellMaterial.tspackages/dev/materials/src/fire/fireMaterial.tspackages/dev/materials/src/fur/furMaterial.tspackages/dev/materials/src/gradient/gradientMaterial.tspackages/dev/materials/src/grid/gridMaterial.tspackages/dev/materials/src/lava/lavaMaterial.tspackages/dev/materials/src/mix/mixMaterial.tspackages/dev/materials/src/normal/normalMaterial.tspackages/dev/materials/src/simple/simpleMaterial.tspackages/dev/materials/src/terrain/terrainMaterial.tspackages/dev/materials/src/triPlanar/triPlanarMaterial.tspackages/dev/materials/src/water/waterMaterial.tsNode Material blocks:
packages/dev/core/src/Materials/Node/Blocks/Dual/lightBlock.tspackages/dev/core/src/Materials/Node/Blocks/Dual/reflectionTextureBaseBlock.tspackages/dev/core/src/Materials/Node/Blocks/PBR/pbrMetallicRoughnessBlock.tsEditor/tooling consumers:
packages/dev/sharedUiComponents/src/nodeGraphSystem/graphNode.ts*/genericNodePropertyComponent.tsx(node editor, geometry editor, render graph editor, particle editor, smart filters editor)packages/tools/smartFiltersEditorControl/src/properties/inputNodePropertyComponent.tsx3MF serializer:
packages/dev/serializers/src/3MF/core/xml/xml.interfaces.tsSmart Filters:
packages/dev/smartFilters/src/editorUtils/editableInPropertyPage.tspackages/dev/smartFilters/src/blockFoundation/customShaderBlock.tspackages/dev/smartFilters/src/utils/buildTools/convertShaders.tspackages/dev/smartFilters/test/unit/customShaderBlock.test.tsViewer (Lit web components):
packages/tools/viewer/src/viewerElement.tspackages/tools/viewer/src/viewerAnnotationElement.tsPlayground / Snippet Loader:
packages/tools/playground/src/tools/monaco/ts/tsPipeline.tspackages/tools/snippetLoader/src/snippetLoader.tsGUI Editor:
packages/tools/guiEditor/src/components/propertyTab/propertyGrids/gui/gridPropertyGridComponent.tsxTests (new):
packages/dev/core/test/unit/Decorators/decorators.test.tspackages/dev/core/test/unit/Decorators/decorators.inline-test.tsDiscussion Points
accessorkeyword change affects any code subclassing Babylon.js materials. How should we communicate/version this?This PR was created as a proof-of-concept to facilitate discussion. All feedback welcome.