From 195e6a66fe2c6147d1680afe85438cb41d83b33b Mon Sep 17 00:00:00 2001 From: harshiltewari2004 Date: Fri, 18 Sep 2026 19:35:04 +0530 Subject: [PATCH 1/3] Keep p5.strands transpiler output valid for comma-joined expressions --- src/strands/strands_transpiler.js | 168 +++++++++++++----------------- test/unit/webgl/p5.Shader.js | 104 ++++++++++++++++++ 2 files changed, 178 insertions(+), 94 deletions(-) diff --git a/src/strands/strands_transpiler.js b/src/strands/strands_transpiler.js index 617fae5909..971e914811 100644 --- a/src/strands/strands_transpiler.js +++ b/src/strands/strands_transpiler.js @@ -442,6 +442,29 @@ function addCopyingAndReturn(functionBody, varsToReturn, sourcePrefix = null) { }); } +/** + * Rewrites `node` in place into the call expression + * `object.methodName(...args)`. + * + * The rewritten node stays an *expression*, so it is still valid wherever the + * original one was: inside a comma (sequence) expression, a ternary, or a call + * argument. Turning it into an ExpressionStatement instead would make + * escodegen emit a `;` in those positions, producing invalid JavaScript. + */ +function replaceWithMethodCall(node, object, methodName, args) { + delete node.operator; + delete node.left; + delete node.right; + node.type = 'CallExpression'; + node.callee = { + type: 'MemberExpression', + computed: false, + object, + property: { type: 'Identifier', name: methodName } + }; + node.arguments = args; +} + const ASTCallbacks = { UnaryExpression(node, state, ancestors) { if ( @@ -704,22 +727,12 @@ const ASTCallbacks = { } // Handle direct varying variable assignment: myVarying = value if (state.varyings[node.left.name]) { - node.type = 'ExpressionStatement'; - node.expression = { - type: 'CallExpression', - callee: { - type: 'MemberExpression', - object: { - type: 'Identifier', - name: node.left.name - }, - property: { - type: 'Identifier', - name: 'bridge' - } - }, - arguments: [node.right] - }; + replaceWithMethodCall( + node, + { type: 'Identifier', name: node.left.name }, + 'bridge', + [node.right] + ); } // Handle swizzle assignment to varying variable: myVarying.xyz = value // Note: node.left.object might be worldPos.getValue() due to prior Identifier transformation @@ -729,22 +742,9 @@ const ASTCallbacks = { const value = node.right; const callee = source.object; const member = source.property; - node.right = undefined; - node.left = undefined; - node.operator = undefined; - node.callee = { - type: 'MemberExpression', - object: callee, - property: { - type: 'Identifier', - name: 'set' - } - }; - node.arguments = [member, value]; - node.type = 'CallExpression'; + replaceWithMethodCall(node, callee, 'set', [member, value]); return; } - let varyingName = null; // Check if it's a direct identifier: myVarying.xyz @@ -767,28 +767,12 @@ const ASTCallbacks = { if (varyingName) { const swizzlePattern = node.left.property.name; - node.type = 'ExpressionStatement'; - node.expression = { - type: 'CallExpression', - callee: { - type: 'MemberExpression', - object: { - type: 'Identifier', - name: varyingName - }, - property: { - type: 'Identifier', - name: 'bridgeSwizzle' - } - }, - arguments: [ - { - type: 'Literal', - value: swizzlePattern - }, - node.right - ] - }; + replaceWithMethodCall( + node, + { type: 'Identifier', name: varyingName }, + 'bridgeSwizzle', + [{ type: 'Literal', value: swizzlePattern }, node.right] + ); } } }, @@ -1607,7 +1591,28 @@ function functionHasSetInControlFlow(functionNode) { return hasSetInControlFlow; } - +/** + * Does this statement contain a `.()` call? + * + * Also looks inside comma (sequence) expressions, so that minified-style code + * such as `hook.begin(), doSomething();` is still recognised. + */ +function statementCallsHookMethod(stmt, methodName, exprString) { + if (stmt.type !== 'ExpressionStatement') { + return false; + } + const expressions = + stmt.expression?.type === 'SequenceExpression' + ? stmt.expression.expressions + : [stmt.expression]; + return expressions.some( + expr => + expr?.type === 'CallExpression' && + expr.callee?.type === 'MemberExpression' && + expr.callee?.property?.name === methodName && + escodegen.generate(expr.callee.object) === exprString + ); +} // Transform a function to use __setValue pattern instead of .set() calls in branches/loops function transformFunctionSetCalls(functionNode) { if (!functionNode.body || functionNode.body.type !== 'BlockStatement') { @@ -1662,20 +1667,11 @@ function transformFunctionSetCalls(functionNode) { let beginCallIndex = -1; for (let i = 0; i < functionNode.body.body.length; i++) { - const stmt = functionNode.body.body[i]; if ( - stmt.type === 'ExpressionStatement' && - stmt.expression?.type === 'CallExpression' && - stmt.expression?.callee?.type === 'MemberExpression' && - stmt.expression?.callee?.property?.name === 'begin' + statementCallsHookMethod(functionNode.body.body[i], 'begin', exprString) ) { - const beginExprString = escodegen.generate( - stmt.expression.callee.object - ); - if (beginExprString === exprString) { - beginCallIndex = i; - break; - } + beginCallIndex = i; + break; } } @@ -1697,25 +1693,16 @@ function transformFunctionSetCalls(functionNode) { ) { const currentExprString = escodegen.generate(node.callee.object); if (currentExprString === exprString && node.arguments.length > 0) { - // Find the parent statement - let parentStmt = null; - for (let i = ancestors.length - 1; i >= 0; i--) { - if (ancestors[i].type === 'ExpressionStatement') { - parentStmt = ancestors[i]; - break; - } - } - - if (parentStmt) { - // Replace the .set() call with an assignment - parentStmt.type = 'ExpressionStatement'; - parentStmt.expression = { - type: 'AssignmentExpression', - operator: '=', - left: { type: 'Identifier', name: intermediateVarName }, - right: node.arguments[0] - }; - } + // Replace the .set() call itself with an assignment, in place. + // Replacing the enclosing statement instead would discard any + // sibling expressions when the call is part of a comma expression. + const value = node.arguments[0]; + delete node.callee; + delete node.arguments; + node.type = 'AssignmentExpression'; + node.operator = '='; + node.left = { type: 'Identifier', name: intermediateVarName }; + node.right = value; } } } @@ -1741,18 +1728,11 @@ function transformFunctionSetCalls(functionNode) { // Find the .end() call for this hook let endCallIndex = -1; for (let i = 0; i < functionNode.body.body.length; i++) { - const stmt = functionNode.body.body[i]; if ( - stmt.type === 'ExpressionStatement' && - stmt.expression?.type === 'CallExpression' && - stmt.expression?.callee?.type === 'MemberExpression' && - stmt.expression?.callee?.property?.name === 'end' + statementCallsHookMethod(functionNode.body.body[i], 'end', exprString) ) { - const endExprString = escodegen.generate(stmt.expression.callee.object); - if (endExprString === exprString) { - endCallIndex = i; - break; - } + endCallIndex = i; + break; } } diff --git a/test/unit/webgl/p5.Shader.js b/test/unit/webgl/p5.Shader.js index 06791151af..78966d3222 100644 --- a/test/unit/webgl/p5.Shader.js +++ b/test/unit/webgl/p5.Shader.js @@ -3357,6 +3357,110 @@ suite('p5.Shader', function () { assert.approximately(pixelColor[2], 0, 5); }); + suite('comma operator (#9178)', () => { + test('handles comma-joined hook calls with a shared variable', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + myp5.pixelDensity(1); + + const testShader = myp5.baseMaterialShader().modify( + () => { + let processedNormal = myp5.sharedVec3(); + myp5.objectInputs.begin(); + myp5.objectInputs.position += [0, 0, 0]; + myp5.objectInputs.end(); + + // Comma-joined, as a JavaScript minifier would emit it + myp5.pixelInputs.begin(), + (processedNormal = myp5.normalize(myp5.pixelInputs.normal)), + myp5.pixelInputs.end(); + + myp5.finalColor.begin(); + myp5.finalColor.set([myp5.abs(processedNormal), 1]); + myp5.finalColor.end(); + }, + { myp5 } + ); + + myp5.background(255, 0, 0); + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const centerColor = myp5.get(25, 25); + assert.approximately(centerColor[0], 0, 5); + assert.approximately(centerColor[1], 0, 5); + assert.approximately(centerColor[2], 255, 5); + }); + + test('handles a comma-joined swizzle assignment', () => { + myp5.createCanvas(5, 5, myp5.WEBGL); + + expect(() => { + myp5.baseMaterialShader().modify( + () => { + // The shared variable is consumed by the p5.strands transpiler, not by JS. + /* oxlint-disable-next-line no-unused-vars */ + let processedNormal = myp5.sharedVec3(); + myp5.pixelInputs.begin(), + (processedNormal.xy = myp5.pixelInputs.normal.xy), + myp5.pixelInputs.end(); + }, + { myp5 } + ); + }).not.toThrow(); + }); + + test('keeps sibling expressions around a .set() call in control flow', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const testShader = myp5.baseFilterShader().modify( + () => { + myp5.filterColor.begin(); + let value = 1; + let c = [0, 1, 0, 1]; + if (value > 0.5) { + (c = [1, 0, 0, 1]), myp5.filterColor.set(c); + } + myp5.filterColor.end(); + }, + { myp5 } + ); + + myp5.background(255, 255, 255); + myp5.filter(testShader); + + // Red only if the `c = [1, 0, 0, 1]` sibling survived transpilation + const pixelColor = myp5.get(25, 25); + assert.approximately(pixelColor[0], 255, 5); + assert.approximately(pixelColor[1], 0, 5); + assert.approximately(pixelColor[2], 0, 5); + }); + + test('finds .begin() and .end() inside comma expressions', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const testShader = myp5.baseFilterShader().modify( + () => { + let step = 0; + myp5.filterColor.begin(), (step = 1); + if (step > 0.5) { + myp5.filterColor.set([1, 0, 0, 1]); + } + myp5.filterColor.end(), (step = 2); + }, + { myp5 } + ); + + myp5.background(255, 255, 255); + myp5.filter(testShader); + + const pixelColor = myp5.get(25, 25); + assert.approximately(pixelColor[0], 255, 5); + assert.approximately(pixelColor[1], 0, 5); + assert.approximately(pixelColor[2], 0, 5); + }); + }); + test('handle .set() in for loop with flat API', () => { myp5.createCanvas(50, 50, myp5.WEBGL); From 84645dbc39f9f4d55a6d2672023fc2a5275e371a Mon Sep 17 00:00:00 2001 From: harshiltewari2004 Date: Tue, 22 Sep 2026 19:47:51 +0530 Subject: [PATCH 2/3] Declare hook intermediate variable before the .begin() statement, fix review nits --- src/strands/strands_transpiler.js | 11 ++++++++--- test/unit/webgl/p5.Shader.js | 26 +++++++++++++++++++++++++- 2 files changed, 33 insertions(+), 4 deletions(-) diff --git a/src/strands/strands_transpiler.js b/src/strands/strands_transpiler.js index 971e914811..cd71490d57 100644 --- a/src/strands/strands_transpiler.js +++ b/src/strands/strands_transpiler.js @@ -1675,9 +1675,14 @@ function transformFunctionSetCalls(functionNode) { } } - // Insert intermediate variable after .begin() if found, otherwise at the start + // Declare the intermediate variable *before* the statement containing + // .begin(), not after it. Minified code can put .begin() and a .set() in + // the same comma expression (`hook.begin(), hook.set(x)`). Every .set() on + // this hook is rewritten below to assign to this variable, so declaring it + // after that statement would use it before its `let` runs. (This pass only + // runs for functions with a .set() inside if/for; see #8576.) if (beginCallIndex !== -1) { - functionNode.body.body.splice(beginCallIndex + 1, 0, intermediateVarDecl); + functionNode.body.body.splice(beginCallIndex, 0, intermediateVarDecl); } else { functionNode.body.body.unshift(intermediateVarDecl); } @@ -1693,7 +1698,7 @@ function transformFunctionSetCalls(functionNode) { ) { const currentExprString = escodegen.generate(node.callee.object); if (currentExprString === exprString && node.arguments.length > 0) { - // Replace the .set() call itself with an assignment, in place. + // Replace the .set() call itself with an assignment, in place. // Replacing the enclosing statement instead would discard any // sibling expressions when the call is part of a comma expression. const value = node.arguments[0]; diff --git a/test/unit/webgl/p5.Shader.js b/test/unit/webgl/p5.Shader.js index 78966d3222..906a4e17cf 100644 --- a/test/unit/webgl/p5.Shader.js +++ b/test/unit/webgl/p5.Shader.js @@ -3357,7 +3357,7 @@ suite('p5.Shader', function () { assert.approximately(pixelColor[2], 0, 5); }); - suite('comma operator (#9178)', () => { + suite('comma operator (#9178)', () => { test('handles comma-joined hook calls with a shared variable', () => { myp5.createCanvas(50, 50, myp5.WEBGL); myp5.pixelDensity(1); @@ -3459,6 +3459,30 @@ suite('p5.Shader', function () { assert.approximately(pixelColor[1], 0, 5); assert.approximately(pixelColor[2], 0, 5); }); + + test('handles .set() right after .begin() in the same comma expression', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const testShader = myp5.baseFilterShader().modify( + () => { + myp5.filterColor.begin(), myp5.filterColor.set([0, 1, 0, 1]); + let value = 1; + if (value > 0.5) { + myp5.filterColor.set([1, 0, 0, 1]); + } + myp5.filterColor.end(); + }, + { myp5 } + ); + + myp5.background(255, 255, 255); + myp5.filter(testShader); + + const pixelColor = myp5.get(25, 25); + assert.approximately(pixelColor[0], 255, 5); + assert.approximately(pixelColor[1], 0, 5); + assert.approximately(pixelColor[2], 0, 5); + }); }); test('handle .set() in for loop with flat API', () => { From ed81af814bea6225535090ff63b3f64571c9919e Mon Sep 17 00:00:00 2001 From: harshiltewari2004 Date: Wed, 23 Sep 2026 00:10:38 +0530 Subject: [PATCH 3/3] Use regular block comments for internal transpiler helpers --- src/strands/strands_transpiler.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/strands/strands_transpiler.js b/src/strands/strands_transpiler.js index cd71490d57..dfb30c9531 100644 --- a/src/strands/strands_transpiler.js +++ b/src/strands/strands_transpiler.js @@ -442,7 +442,7 @@ function addCopyingAndReturn(functionBody, varsToReturn, sourcePrefix = null) { }); } -/** +/* * Rewrites `node` in place into the call expression * `object.methodName(...args)`. * @@ -1591,7 +1591,7 @@ function functionHasSetInControlFlow(functionNode) { return hasSetInControlFlow; } -/** +/* * Does this statement contain a `.()` call? * * Also looks inside comma (sequence) expressions, so that minified-style code