feat: Support computed object properties - #2950
Conversation
cieplypolar
changed the title
|
pkg.pr.new packages benchmark commit |
Bundle size comparison (
|
| 🟢 Decreased | ➖ Unchanged | 🔴 Increased (max 0.60%) | ❔ Unknown |
|---|---|---|---|
| 0 | 319 | 6 | 0 |
import * as ... in PR vs import * as ... in target (did bundle size increase?):
| Test | tsdown |
|---|---|
| tgpu_resolveWithContext.ts | 162.87 kB ( |
| tgpu_resolve.ts | 162.94 kB ( |
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu.
Resolution Time Benchmark---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Random Branching (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.75, 1.47, 3.34, 5.01, 5.45, 9.57, 18.10, 19.59]
line [0.68, 1.44, 3.02, 5.02, 5.19, 8.57, 18.00, 19.20]
line [0.72, 1.44, 3.10, 4.70, 5.34, 9.22, 16.16, 20.34]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Linear Recursion (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.23, 0.50, 0.61, 0.75, 0.93, 0.91, 1.16, 1.28]
line [0.20, 0.37, 0.54, 0.61, 0.85, 0.89, 1.08, 1.16]
line [0.23, 0.38, 0.57, 0.68, 0.89, 0.95, 1.16, 1.22]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Full Tree (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.75, 1.91, 2.77, 5.33, 9.35, 20.09, 42.60, 84.39]
line [0.72, 1.86, 2.74, 5.45, 9.60, 22.24, 40.10, 85.41]
line [0.91, 1.95, 4.11, 6.40, 10.18, 20.20, 42.13, 83.98]
|
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
ℹ️ Minor suggestions only — no blocking issues found. The feature is well-tested and the evaluation-order fix for #2944 is correctly verified.
Reviewed changes — the PR adds computed-property ([key]: value) support to TGSL object literals and fixes #2944 (struct field values are now evaluated in JS source order, key before value). Reviewed against the branch base impr/separate-acorn-babel-parsers (the acorn/babel parser split itself lives in that base and is not in this diff):
- tinyest AST (
nodes.ts): newobjectProperty(107) andobjectExprWithComputedProps(108) node types, threaded through every consumer —wgslGenerator._expression,glslGenerator.#handleStructReturn,tseynitstringify, and the obfuscator. - transpilers.ts:
Property/ObjectPropertyhandlers emit the new node shape; an object literal with any computed key becomesobjectExprWithComputedProps, otherwise keys are still collapsed into a plain record (now with duplicate-key detection). - codegen: struct/AutoStruct literals resolve in source order (key before value), extra (non-struct-member) fields are still evaluated so side effects run in JS order, and duplicate or runtime-known keys are rejected with clear errors via the new
_resolveObjectPropertyKey. - eslint/docs/tests:
noUnsupportedSyntaxdrops the computed-key ban; two example shaders reordered to match struct field order; broad new coverage inwgslGenerator.test.ts,glslGenerator.test.ts,parsers.test.ts,tseynit.test.tsandobfuscation.test.ts.
Verification this run: all six affected test files pass (252 tests), test:types passes for typegpu, tinyest-for-wgsl, unplugin-typegpu and @typegpu/gl, and a scratch probe confirmed the acorn and babel key handlers accept the same key shapes for the common cases.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
fcebfb2 to
8ae3c36
Compare
There was a problem hiding this comment.
Important
The re-based commits standardized the duplicate-key error wording to Duplicate object property key: 'X'. in the transpilers and the GLSL generator, but the two inline snapshots that pin those messages were not updated. Both tests fail on the head commit and build-and-test (Test unit) is red — please update the snapshots before merging.
Reviewed changes — incremental re-review of commit range fcebfb2..8ae3c369. The branch was force-pushed: the six feature commits were rewritten onto the unchanged base tip 53f47c73 (same subjects, new SHAs). The delta is small and behavior-neutral apart from the message change below:
- Standardized the duplicate-key error text to
Duplicate object property key: 'X'.across both transpiler handlers and the GLSL#handleStructReturn, and added the// Normalize to objectProperty[]/_resolveObjectPropertyKeyplumbing comments. - Renamed
properties→objlocals in the transpilers and retitled / reworded several object-order tests (e.g. a redundant struct-constructor snapshot replaced withvoid tgpu.resolve([f]);not.toContain('vary_extra')widened tonot.toContain('extra')). - Left the WGSL generator's richer two-key message (
... found: 'X' and 'Y'.) and its snapshot untouched.
Verification this run: reproduced both failures locally with vitest (tinyest-for-wgsl parsers → 1 failed, @typegpu/gl glslGenerator → 1 failed); all other affected suites pass (wgslGenerator 100, tseynit 30, obfuscation 26, eslint-plugin 38), and the two reordered doc examples (ray-marching, vaporrave) still match their snapshots.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
8ae3c36 to
1850ed8
Compare
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — incremental re-review of commit range 8ae3c369..1850ed89. The delta is exactly the two inline snapshots the last review flagged as CI-breaking: both were updated to the standardized Duplicate object property key: 'X'. wording.
- Updated the
rejects duplicate non-computed object keyssnapshot intinyest-for-wgsl/tests/parsers.test.tstoDuplicate object property key: 'field'., matching both transpiler handlers. - Updated the
rejects duplicate keys in entry point returnsnapshot in@typegpu/gl'sglslGenerator.test.tstoDuplicate object property key: 'uv'., matching#handleStructReturn's throw.
Verified this run: both previously-failing suites now pass (tinyest-for-wgsl parsers 22/22, @typegpu/gl glslGenerator 36/36), so build-and-test (Test unit) should be green again. The WGSL generator's richer two-key message and the docs example snapshots were left untouched, consistent with the last review.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes — incremental re-review of commit range 1850ed89..ec5ca2a7 (single commit ec5ca2a7c "better error in glsl"). The new commit aligns the GLSL duplicate-key error with the WGSL generator's format:
- GLSL generator:
#handleStructReturn'sresolveUniqueKeynow throwsDuplicate object property key found: 'X' and 'Y'.usingstringifyNode(glslGenerator.ts:1030), matchingwgslGenerator.ts:906exactly, instead of the previous single-keyDuplicate object property key: 'X'.. - Test snapshot: the
rejects duplicate keys in entry point returnsnapshot inglslGenerator.test.ts:745updated to the two-key message ('uv: d.vec2f(1, 2)'and'[getKey()]: d.vec2f(3, 4)'), consistent with the WGSL snapshot atwgslGenerator.test.ts:2243.
Verified this run: @typegpu/gl glslGenerator 36/36 tests pass and @typegpu/gl test:types is green. stringifyNode is already exported through typegpu/~internal (internal.ts:10), so the new import resolves cleanly. The change is error-message text only — codegen is unaffected.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
ec5ca2a to
0c24d42
Compare
0c24d42 to
adadba2
Compare
There was a problem hiding this comment.
Important
The new obfuscates computed struct keys test fails on the head commit — it calls transpileFn, which is no longer imported into obfuscation.test.ts (the import was switched to transpileFnBabel alone). This breaks the unplugin test suite and the build-and-test gate, so it needs a one-line fix before merging.
Reviewed changes — incremental re-review since the prior pullfrog review (head 0c24d42, force-pushed to adadba218; net delta is the legacy-transpiler computed-property support):
- Extended the deprecated legacy
transpileFninparsers.ts(createLegacyTraspilers) to support computed object properties, reusing the sharedProperty/ObjectPropertytranspilers and emitting theobjectProperty/objectExprWithComputedPropsnodes instead of throwing. - Updated
parsers.test.tsso the legacy path asserts computed keys transpile (snapshot includes[108,[[107,"id",[5,"1"],true],...]]) plus existing spread/method/dup-key rejection tests. - Reworked
obfuscation.test.ts: converted existing usages totranspileFnBabeland added the newobfuscates computed struct keyscase (which is the one that fails).
Verified this run: tinyest-for-wgsl parsers suite passes with the legacy change; the failing obfuscation test reproduces locally and passes immediately once line 429 uses transpileFnBabel (snapshot unchanged).
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
adadba2 to
5384ac1
Compare
In addition to supporting computed object properties, this preserves evaluation order of comptime-known keys and values mentioned in #2944.