p5.js icon indicating copy to clipboard operation
p5.js copied to clipboard

Feature/detect outside variables strands

Open Aayushdev18 opened this issue 1 month ago • 4 comments

Resolves #8172

Changes

This PR adds detection for outside variable references in p5.strands uniform initializers, providing helpful error messages to users when they reference variables that aren't declared in the strand context.

What was implemented:

  • Created detectOutsideVariableReferences() function in src/strands/strands_transpiler.js that:
    • Uses Acorn AST parsing to analyze strand code
    • Collects all variable declarations within the strand context
    • Finds uniform initializer functions (e.g., uniformFloat('color', () => ...))
    • Extracts variables referenced inside uniform initializers
    • Checks if referenced variables are declared in the strand scope
    • Returns helpful error messages for any undeclared variables
  • Integrated detection to run before transpilation in src/strands/p5.strands.js
  • Added unit tests in test/unit/strands/strands_transpiler.js with test cases for:
    • Detecting undeclared variables (e.g., mouseX, windowWidth)
    • No errors when variables are properly declared
    • Multiple undeclared variables
    • Edge cases

Approach: Following @davepagurek's feedback, this implementation:

  • Runs as a separate analysis phase before transpilation
  • Reuses the transpiler's Acorn-based structure for AST parsing
  • Analyzes variable declarations to build a set of in-scope variables
  • Detects when uniform initializer functions reference variables not in that set
  • Provides specific, actionable error messages to users

This is more robust than checking for hardcoded variable names, as it analyzes the actual code structure and works with any user-defined variables.

Screenshots

N/A (infrastructure change - no visual impact)

PR Checklist

  • [x] Code follows p5.js coding conventions and practices
  • [x] Code is tested and all tests pass
  • [x] Documentation is updated (if applicable)
  • [x] Pull request targets the correct branch (dev-2.0)
  • [x] Changes are minimal and focused on the issue
  • [x] No breaking changes

Aayushdev18 avatar Oct 26 '25 22:10 Aayushdev18

Hey @davepagurek!

I've re-implemented the outside variable detection following your feedback:

  • Uses AST analysis instead of hardcoded variable names
  • Runs as a separate analysis phase before transpilation
  • Properly handles the uniform function signature
  • Includes unit tests Could you take a look when you have time? Let me know if you'd like any adjustments! Thanks!

Aayushdev18 avatar Oct 27 '25 15:10 Aayushdev18

@davepagurek Fixed! Now checking strand code (not uniform callbacks) and using the simpler two-pass approach. Tests updated. Ready for another look when you have time.

Aayushdev18 avatar Oct 27 '25 19:10 Aayushdev18

@davepagurek Thanks for catching those issues! Updated the tests to pass just the function body, and the scope checking should now properly exclude sibling scopes using the ancestor chain. Going to run the integration tests as you suggested - expect that will surface any issues I missed. Thanks for the detailed guidance! 💯

Aayushdev18 avatar Oct 27 '25 20:10 Aayushdev18

Hi @davepagurek , just following up, it’s been over a week since this PR was opened. Could you please approve the pending workflows or review it when you get a chance? Thanks!

Aayushdev18 avatar Nov 03 '25 10:11 Aayushdev18