[p5.js 2.0 Beta Bug Report]: Error in FES prevents message from being shown #7678
Comments
|
Hi, I am wondering if anyone is working on this issue, and if i could be assigned to it. Fair warning this is my first contribution. I would like to contribute as part of an assignment for coursework, but could not find any good first issues that were not either, trivial frontend code or already assigned. |
|
when we do in for (const prop of Object.keys(context)) {
const lowercase = prop.toLowerCase();
if (
fxns[lowercase] &&
!context[fxns[lowercase]] &&
typeof context[prop] === 'function'
) {
to: if (
fxns.hasOwnProperty(lowercase) &&
!context[fxns[lowercase]] &&
typeof context[prop] === 'function'
)would using hasOwnProperty work in this case so that our check would only return true if the specified property is a direct property of this way we could ignore please let me know any feedback about this approach. Thanks |
|
Good but also I think p5 shouldn't override window.constructor |
|
It seems like this is where the const bindGlobal = (property) => {
Object.defineProperty(window, property, {
configurable: true,
enumerable: true,
get: () => {
if(typeof this[property] === 'function'){
return this[property].bind(this);
}else{
return this[property];
}
},
set: (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.`);
}
}
})
};Would simply excluding constructor from this function or the later loops that call it be enough to fix this issue? That doesn't seem like the smartest way of fixing this issue. Additionally are there any other default |
|
Thanks @HughJacks! If you add a conditional breakpoint in there or something to catch when
Once we see which loop it's coming from, we could maybe log all the things being bound and see if anything seems suspicious. |
|
okay after trying this out I found that we are overriding in this loop for (const p of Object.getOwnPropertyNames(p5.prototype)) {
if(p[0] === '_') continue;
bindGlobal(p);
}Just to check for other overrides I downloaded a text file with the output of this code which should have all the default window properties function getAllProperties(obj) {
const props = new Set();
let current = obj;
while (current) {
Object.getOwnPropertyNames(current).forEach(prop => props.add(prop));
current = Object.getPrototypeOf(current);
}
return Array.from(props).sort();
}
const windowProperties = getAllProperties(window);then I compared it to a list i collect from and found that we are potentially overriding
I guess a potential solution would just be to create a set of "protected" properties in bind global and skip them. |
|
Im going to move forward with my protected properties solution, as well as the previous if (
fxns.hasOwnProperty(lowercase) &&
!context[fxns[lowercase]] &&
typeof context[prop] === 'function'
)solution. I'll make sure to test that we aren't messing anything else up. Again I am a new contributor so any advice would be welcome. |
const protectedProperties = ['constructor', 'length', 'print'];
// Attach its properties to the window
for (const p in this) {
if (this.hasOwnProperty(p)) {
if(p[0] === '_' || protectedProperties.includes(p)) continue;
bindGlobal(p);
}
} |
|
print is an existing function but one that p5 intentionally overrides |
and others
Most appropriate sub-area of p5.js?
p5.js version
v2 beta 5
Web browser and version
134.0.6998.166 (Official Build) (arm64)
Operating system
macOS Version 15.3.2 (Build 24D81)
Steps to reproduce this
To reproduce run
new p5().Error log:
This error happens here in p5.js because
contextis thewindowand p5 overrideswindow.constructorp5's class constructor, which is why it's enumerated byObject.keyswhen it wouldn't otherwise.fxnshas aconstructorproperty too because all JS Objects do, but ofc it doesn't store a string. Hence sincefuncis not a string it lacks string methods.The text was updated successfully, but these errors were encountered: