Update sketch verifier to check for redefinitions and print friendly messages #7326
Conversation
| // reference on the p5.js website. | ||
| function generateFriendlyError(errorType, name, line) { | ||
| const url = `https://p5js.org/reference/#/p5/${name}`; | ||
| const message = `${errorType} "${name}" on line ${line} is being redeclared and conflicts with a p5.js ${errorType.toLowerCase()}. JavaScript does not declaring a ${errorType.toLowerCase()} more than once. p5.js reference: ${url}.`; |
There was a problem hiding this comment.
Looks like we maybe lost a verb in here:
| const message = `${errorType} "${name}" on line ${line} is being redeclared and conflicts with a p5.js ${errorType.toLowerCase()}. JavaScript does not declaring a ${errorType.toLowerCase()} more than once. p5.js reference: ${url}.`; | |
| const message = `${errorType} "${name}" on line ${line} is being redeclared and conflicts with a p5.js ${errorType.toLowerCase()}. JavaScript does not support declaring a ${errorType.toLowerCase()} more than once. p5.js reference: ${url}.`; |
There was a problem hiding this comment.
Also, I guess JavaScript does technically allow it in different scopes? We could consider saying "your code might not behave correctly" or "your code might behave unexpectedly" or something like that.
| 'toString', | ||
| 'print', | ||
| 'stop', | ||
| 'onended' |
There was a problem hiding this comment.
The functions meant to be redefined by the user make sense. Out of curiosity where do the other false positive entries come from? Is that from looking at classesWithGlobalFunctions?
| // in the p5.js library, either as a constant or as a function. | ||
| function checkForRedefinition(name, libValue, line, type) { | ||
| try { | ||
| const userValue = eval("name"); |
There was a problem hiding this comment.
I'm not sure what this line is meant to be doing but we want to avoid eval() as the bundler generally does work well with it.
|
I've also just checked the dependency sizes, it seems acorn and to a slightly lesser extend zod adds quite a bit to the bundle size. Are they importable as treeshakable modules so we don't include code we aren't using? |
|
@sproutleaf I kinda forgot about this one but is the changes in here still relevant for merge? |
Addresses #7269
Changes:
checkForConstsAndFuncs()that checks for redefinitions and prints friendly error messagesPR Checklist