Skip to content

Re-bind globals when assigned in addons #8099

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Parent: Vector toString() consistency fix
merged 10 commits into from
Oct 4, 2025
Merged

Re-bind globals when assigned in addons #8099

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
b168dc1
Re-bind globals when assigned in addons
davepagurek Sep 19, 2025
792888c
Implement decorator helper function and have FES use it
limzykenneth Sep 27, 2025
9145f6b
Merge branch 'dev-2.0' into addon-fn-proxy
limzykenneth Sep 27, 2025
ba4c808
Merge branch 'dev-2.0' into addon-fn-proxy
davepagurek Sep 28, 2025
ad206b7
Avoid double-binding prototype methods
davepagurek Sep 28, 2025
419574a
Add decorators to a subclass instead
davepagurek Sep 28, 2025
b7ecbac
Apply function decoration just before class contruction
limzykenneth Oct 3, 2025
8dc859d
Basic conformance to TC39 decorator proposal
limzykenneth Oct 3, 2025
1deae41
Remove accidentally committed file
limzykenneth Oct 4, 2025
fd9df8c
Move decoration application to constructor
limzykenneth Oct 4, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 20 additions & 23 deletions src/core/friendly_errors/param_validator.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ function validateParams(p5, fn, lifecycles) {
* @param {String} func - Name of the function. Expect global functions like `sin` and class methods like `p5.Vector.add`
* @returns {z.ZodSchema} Zod schema
*/
fn.generateZodSchemasForFunc = function (func) {
const generateZodSchemasForFunc = function (func) {
const { funcName, funcClass } = extractFuncNameAndClass(func);
let funcInfo = dataDoc[funcClass][funcName];

Expand Down Expand Up @@ -308,7 +308,7 @@ function validateParams(p5, fn, lifecycles) {
* @param {Array} args - User input arguments.
* @returns {z.ZodSchema} Closest schema matching the input arguments.
*/
fn.findClosestSchema = function (schema, args) {
const findClosestSchema = function (schema, args) {
if (!(schema instanceof z.ZodUnion)) {
return schema;
}
Expand Down Expand Up @@ -389,7 +389,7 @@ function validateParams(p5, fn, lifecycles) {
* @param {String} func - Name of the function. Expect global functions like `sin` and class methods like `p5.Vector.add`
* @returns {String} The friendly error message.
*/
fn.friendlyParamError = function (zodErrorObj, func, args) {
const friendlyParamError = function (zodErrorObj, func, args) {
let message = '🌸 p5.js says: ';
let isVersionError = false;
// The `zodErrorObj` might contain multiple errors of equal importance
Expand Down Expand Up @@ -520,7 +520,7 @@ function validateParams(p5, fn, lifecycles) {
* @returns {any} [result.data] - The parsed data if validation was successful.
* @returns {String} [result.error] - The validation error message if validation has failed.
*/
fn.validate = function (func, args) {
const validate = function (func, args) {
if (p5.disableFriendlyErrors) {
return; // skip FES
}
Expand Down Expand Up @@ -548,7 +548,7 @@ function validateParams(p5, fn, lifecycles) {

let funcSchemas = schemaRegistry.get(func);
if (!funcSchemas) {
funcSchemas = fn.generateZodSchemasForFunc(func);
funcSchemas = generateZodSchemasForFunc(func);
if (!funcSchemas) return;
schemaRegistry.set(func, funcSchemas);
}
Expand All @@ -559,9 +559,9 @@ function validateParams(p5, fn, lifecycles) {
data: funcSchemas.parse(args)
};
} catch (error) {
const closestSchema = fn.findClosestSchema(funcSchemas, args);
const closestSchema = findClosestSchema(funcSchemas, args);
const zodError = closestSchema.safeParse(args).error;
const errorMessage = fn.friendlyParamError(zodError, func, args);
const errorMessage = friendlyParamError(zodError, func, args);

return {
success: false,
Expand All @@ -570,25 +570,22 @@ function validateParams(p5, fn, lifecycles) {
}
};

lifecycles.presetup = function(){
loadP5Constructors();
fn._validate = validate; // TEMP: For unit tests

if(
p5.disableParameterValidator !== true &&
p5.disableFriendlyErrors !== true
){
const excludes = ['validate'];
for(const f in this){
if(!excludes.includes(f) && !f.startsWith('_') && typeof this[f] === 'function'){
const copy = this[f];

this[f] = function(...args) {
this.validate(f, args);
return copy.call(this, ...args);
};
p5.decorateHelper(
/^(?!_).+$/,
function(target, { name }){
return function(...args){
if (!p5.disableFriendlyErrors && !p5.disableParameterValidator) {
validate(name, args);
}
}
return target.call(this, ...args);
};
}
);

lifecycles.presetup = function(){
loadP5Constructors();
};
}

Expand Down
253 changes: 134 additions & 119 deletions src/core/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,122 +77,7 @@ class p5 {
// ensure correct reporting of window dimensions
this._updateWindowSize();

const bindGlobal = property => {
if (property === 'constructor') return;

// Common setter for all property types
const createSetter = () => newValue => {
Object.defineProperty(window, property, {
configurable: true,
enumerable: true,
value: newValue,
writable: true
});
if (!p5.disableFriendlyErrors) {
console.log(`You just changed the value of "${property}", which was a p5 global value. This could cause problems later if you're not careful.`);
}
};

// Check if this property has a getter on the instance or prototype
const instanceDescriptor = Object.getOwnPropertyDescriptor(this, property);
const prototypeDescriptor = Object.getOwnPropertyDescriptor(p5.prototype, property);
const hasGetter = (instanceDescriptor && instanceDescriptor.get) ||
(prototypeDescriptor && prototypeDescriptor.get);

// Only check if it's a function if it doesn't have a getter
// to avoid actually evaluating getters before things like the
// renderer are fully constructed
let isPrototypeFunction = false;
let isConstant = false;
let constantValue;

if (!hasGetter) {
const prototypeValue = p5.prototype[property];
isPrototypeFunction = typeof prototypeValue === 'function';

// Check if this is a true constant from the constants module
if (!isPrototypeFunction && constants[property] !== undefined) {
isConstant = true;
constantValue = prototypeValue;
}
}

if (isPrototypeFunction) {
// For regular functions, cache the bound function
const boundFunction = p5.prototype[property].bind(this);
if (p5.disableFriendlyErrors) {
Object.defineProperty(window, property, {
configurable: true,
enumerable: true,
value: boundFunction,
});
} else {
Object.defineProperty(window, property, {
configurable: true,
enumerable: true,
get() {
return boundFunction;
},
set: createSetter()
});
}
} else if (isConstant) {
// For constants, cache the value directly
if (p5.disableFriendlyErrors) {
Object.defineProperty(window, property, {
configurable: true,
enumerable: true,
value: constantValue,
});
} else {
Object.defineProperty(window, property, {
configurable: true,
enumerable: true,
get() {
return constantValue;
},
set: createSetter()
});
}
} else if (hasGetter || !isPrototypeFunction) {
// For properties with getters or non-function properties, use lazy optimization
// On first access, determine the type and optimize subsequent accesses
let lastFunction = null;
let boundFunction = null;
let isFunction = null; // null = unknown, true = function, false = not function

Object.defineProperty(window, property, {
configurable: true,
enumerable: true,
get: () => {
const currentValue = this[property];

if (isFunction === null) {
// First access - determine type and optimize
isFunction = typeof currentValue === 'function';
if (isFunction) {
lastFunction = currentValue;
boundFunction = currentValue.bind(this);
return boundFunction;
} else {
return currentValue;
}
} else if (isFunction) {
// Optimized function path - only rebind if function changed
if (currentValue !== lastFunction) {
lastFunction = currentValue;
boundFunction = currentValue.bind(this);
}
return boundFunction;
} else {
// Optimized non-function path
return currentValue;
}
},
set: createSetter()
});
}
};
const bindGlobal = createBindGlobal(this);
// If the user has created a global setup or draw function,
// assume "global" mode and make everything global (i.e. on the window)
if (!sketch) {
Expand Down Expand Up @@ -259,6 +144,7 @@ class p5 {

static registerAddon(addon) {
const lifecycles = {};

addon(p5, p5.prototype, lifecycles);

const validLifecycles = Object.keys(p5.lifecycleHooks);
Expand All @@ -269,6 +155,13 @@ class p5 {
}
}

static decorations = new Map();
static decorateHelper(pattern, decoration){
let patternArray = pattern;
if (!Array.isArray(pattern)) patternArray = [pattern];
p5.decorations.set(patternArray, decoration);
}

#customActions = {};
_customActions = new Proxy({}, {
get: (target, prop) => {
Expand Down Expand Up @@ -511,6 +404,96 @@ class p5 {
}
}

// Global helper function for binding properties to window in global mode
function createBindGlobal(instance) {
return function bindGlobal(property) {
if (property === 'constructor') return;

// Check if this property has a getter on the instance or prototype
const instanceDescriptor = Object.getOwnPropertyDescriptor(
instance,
property
);
const prototypeDescriptor = Object.getOwnPropertyDescriptor(
p5.prototype,
property
);
const hasGetter = (instanceDescriptor && instanceDescriptor.get) ||
(prototypeDescriptor && prototypeDescriptor.get);

// Only check if it's a function if it doesn't have a getter
// to avoid actually evaluating getters before things like the
// renderer are fully constructed
let isPrototypeFunction = false;
let isConstant = false;
let constantValue;

if (!hasGetter) {
const prototypeValue = p5.prototype[property];
isPrototypeFunction = typeof prototypeValue === 'function';

// Check if this is a true constant from the constants module
if (!isPrototypeFunction && constants[property] !== undefined) {
isConstant = true;
constantValue = prototypeValue;
}
}

if (isPrototypeFunction) {
// For regular functions, cache the bound function
const boundFunction = p5.prototype[property].bind(instance);
Object.defineProperty(window, property, {
configurable: true,
enumerable: true,
value: boundFunction
});
} else if (isConstant) {
// For constants, cache the value directly
Object.defineProperty(window, property, {
configurable: true,
enumerable: true,
value: constantValue
});
} else if (hasGetter || !isPrototypeFunction) {
// For properties with getters or non-function properties, use lazy optimization
// On first access, determine the type and optimize subsequent accesses
let lastFunction = null;
let boundFunction = null;
let isFunction = null; // null = unknown, true = function, false = not function

Object.defineProperty(window, property, {
configurable: true,
enumerable: true,
get: () => {
const currentValue = instance[property];

if (isFunction === null) {
// First access - determine type and optimize
isFunction = typeof currentValue === 'function';
if (isFunction) {
lastFunction = currentValue;
boundFunction = currentValue.bind(instance);
return boundFunction;
} else {
return currentValue;
}
} else if (isFunction) {
// Optimized function path - only rebind if function changed
if (currentValue !== lastFunction) {
lastFunction = currentValue;
boundFunction = currentValue.bind(instance);
}
return boundFunction;
} else {
// Optimized non-function path
return currentValue;
}
}
});
}
};
}

// Attach constants to p5 prototype
for (const k in constants) {
p5.prototype[k] = constants[k];
Expand Down Expand Up @@ -745,8 +728,6 @@ for (const k in constants) {
* </code>
* </div>
*/
p5.disableFriendlyErrors = false;

import transform from './transform';
import structure from './structure';
import environment from './environment';
Expand All @@ -763,4 +744,38 @@ p5.registerAddon(renderer);
p5.registerAddon(renderer2D);
p5.registerAddon(graphics);

export default p5;
const p5Proxy = new Proxy(p5, {
construct(target, args){
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cool, I'll just move it to the very start of the constructor and not use Proxy.

if(p5.decorations.size > 0){
// Apply addon defined decorations
for (const [patternArray, decoration] of p5.decorations) {
for(const member in p5.prototype) {
// Member must be a function
if (typeof p5.prototype[member] !== 'function') continue;

if (!patternArray.some(pattern => {
if (typeof pattern === 'string') {
return pattern === member;
} else if (pattern instanceof RegExp) {
return pattern.test(member);
}
})) continue;

p5.prototype[member] = decoration(p5.prototype[member], {
kind: 'method',
name: member,
access: {},
static: false,
private: false,
addInitializer(initializer){}
});
}
}

p5.decorations.clear();
}
return new target(...args);
}
});

export default p5Proxy;
Loading
Loading