Fix p5.strands filters not working on minified code #8367
Merged
+156
−84
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #8364
Changes
So this stems from the fact that p5.strands code gets transpiled, and when we run the result, it's not in the same runtime context as before. It has no access to outside variables. So we had previously made a way to inject those variables: pass them in as an object in a second parameter after your function, and we'll automatically feed them in as parameters to your transpiled function with the original names so that you have access again:
The problem was that minified js and ts code also change the variable names, but not the keys of that object you pass in. Previously it worked because those were always the same, but minification makes that no longer be the case. The above code could turn into something like this:
To address that, there are two possible routes:
The first option is easier (don't write a function, write a source code string), and that would work for our library, but would make js/ts builds a lot more cumbersome. No one really likes writing code in a string. So I moved on to the second option.
There's already a solution along the lines of the second option in the wild. In Puppeteer, when you write a function in nodejs to run in a Chrome window, it's also recreating a new function. The way it handles it is that it doesn't automatically inject variables for you, you have to manually grab them from function arguments. So that's what I've opted for here:
This is more verbose than before, but I think it's the only way we can avoid this issue.
I also noticed that sometimes the minifier turns for loops like this:
...into something like this for compactness, with comma separated statements:
This wasn't getting handled by our transpiler because we were trying to whitelist the statements we need to traverse into to find variable updates in a loop. I've updated that code to now use the default Acord tree traversal, and manually skip the ones we don't want to traverse into.
PR Checklist
npm run lintpasses