Re-bind globals when assigned in addons #8099
Conversation
|
I'll try to review this tomorrow |
|
I've figured out a way to do function decoration for FES, will see if we fully need these. |
|
@davepagurek Hopefully we are there now. I've changed up the implementation a bit from what we talked about, instead of a boolean flag to track whether the decoration has been applied, I've changed it to see if the decorators Map is empty or not and empty it on each construction. This mean it is possible to dynamically decorate the p5 class as you go, a bit of a niche use case but I'll leave it just in case unless there is any obvious problem with this approach that I missed. The decorator signature also roughly matches TC39 decorator proposal now although there are still room to implement further. |
src/friendly_errors/engine.js
Outdated
| * @returns {any} [result.data] - The parsed data if validation was successful. | ||
| * @returns {String} [result.error] - The validation error message if validation has failed. | ||
| */ | ||
| function validate(func, args) { |
There was a problem hiding this comment.
Do we use the versions of the functions in this file or do we still use the param_validator.js versions? Might want to remove one of them
There was a problem hiding this comment.
Ah, accidentally committed this file. It is meant to be the WIP for the FES engine idea but it is not connected anywhere yet. I'll remove this file for this PR to avoid confusion.
src/core/main.js
Outdated
|
|
||
| export default p5; | ||
| const p5Proxy = new Proxy(p5, { | ||
| construct(target, args){ |
There was a problem hiding this comment.
Ah yeah I see what you mean about the proxy no longer being necessary, we probably could cut and paste this into the regular constructor now. Maybe worth doing to see if tests pass, just to avoid any other possible complications that come from having a proxy
There was a problem hiding this comment.
Cool, I'll just move it to the very start of the constructor and not use Proxy.
2 participants
This is another attempt at what #8098 is trying to do. In #8094 (comment) @limzykenneth noticed that the updates to global mode function binding were clashing with how FES parameter validation currently works: methods on global mode now cache the functions they're wrapping for efficiency, but they were caching them before FES got added.
This PR tries to do this by passing proxies into addon registration and lifecycle methods. If you assign something to a p5 instance or the p5 prototype, this will now re-bind the global to the window.
There's a chance that in a lifecycle method like
presetup, an addon will define a function (like FES does) that it adds to the instance, but that additionally captures and usesthis, which will now be a proxy to the instance. Whenthisis used within that function, it could be assigning and reassigning to state properties. We don't want the overhead of making those global every time, so I've added aninLifecyclevariable that tracks when we're inside e.g.presetupand then turns false afterwards, letting us check that and stop binding new globals once we're out of that lifecycle.Test sketch: https://editor.p5js.org/davepagurek/sketches/tlH_eIBUn