Keep p5.strands transpiler output valid for comma-joined expressions

[harshiltewari2004]

Resolves #9178

Changes

The transpiler assumed each call or assignment is its own statement. Comma-joined
code (as minifiers produce) breaks that assumption in three places, all in
src/strands/strands_transpiler.js:

1. Assignments were rewritten into statements. AssignmentExpression turned
the node into an ExpressionStatement for the bridge and bridgeSwizzle
paths, so escodegen emitted a ; mid-expression (;,) and the callback failed
to build. They are now rewritten as call expressions in place, via a new
replaceWithMethodCall() helper that the existing computed-member branch also
uses. This covers ternaries, call arguments and x++ (which routes through the
same handler). In ordinary statement position it also removes a stray empty
statement that was previously emitted.

2. .set() in control flow dropped sibling expressions. The rewrite replaced
the whole enclosing statement, so in a(), hook.set(v), b() the calls to a()
and b() disappeared from the output with no error. Only the .set() call is
replaced now.

3. .begin() / .end() were only found as standalone statements. With
hook.end(), b(); the generated hook.set(...) was inserted after .end(),
which produces a hook function that never assigns its result — a GLSL compile
error ('HOOK_getColor': Function does not return a value). A new
statementCallsHookMethod() helper, shared by both lookups, also looks inside
comma expressions.

Tests

Four tests in a new comma operator (#9178) suite in
test/unit/webgl/p5.Shader.js, one per failure mode. Verified by stashing the
source change: all four fail without it, each for its own reason, and pass with
it. The full WebGL and WebGPU shader suites pass.

PR Checklist

• npm run lint passes
• [Inline reference] is included / updated
• [Unit tests] are included / updated

[p5-bot]

Continuous Release

CDN link

• https://raw.esm.sh/pr/p5@195e6a6/lib/p5.min.js

Published Packages

• https://pkg.pr.new/p5@195e6a6

Commit hash: 195e6a6

Previous deployments

---

This is an automated message.

[Ayush4958]

LGTM, In place AST mutations correctly preserve minified SequenceExpression struct without dropping sibling node & acorn walk post order traversal ensures the node property deletions are entirely safe.

cc: @davepagurek

[davepagurek]

The approach looks good! I left a comment about what looks like it might be an edge case (but maybe not, worth testing to see), let me know what you think!

[davepagurek]

Pretty minor but it looks like indentation is off here

[harshiltewari2004]

Fixed, thankss!

[davepagurek]

minor, there's an extra space here

[harshiltewari2004]

Fixed.

[davepagurek]

I believe this is still finding the index of the statement that contains a hook .begin(). If we are trying to splice in intermediateVarDecl, so I was wondering if there is a chance that now we're splicing it after it gets used if it happens to get used in the same statement in a different comma-separated expression. Looks like this was only added to handle loops and branches (see #8576), but does this run on every set call? e.g. would this affect something that calls set right after a begin in a comma-separated expression currently? If not I'm curious what the flow is, would be good to add in a comment to help future contributors understand.

[harshiltewari2004]

Good catch, this was a real bug. With hook.begin(), hook.set(x) in one
statement, the .set() becomes an assignment to the intermediate variable,
but the let was inserted after that statement, so it threw a
ReferenceError. Before this PR it didn't happen only because .begin()
wasn't found inside comma expressions, so the declaration went to the top.

On your question: the pass only runs for functions with a .set() inside
if/for, but once it runs it rewrites every .set() on that hook, including
one next to .begin().

I now insert the declaration before the .begin() statement instead of
after, added a comment explaining the flow, and added a regression test for
this case.