Skip to content

Commit b42960e

Browse files
sproutleafsproutleaf
committed
update
1 parent 8c13dde commit b42960e

File tree

3 files changed

+94
-67
lines changed

3 files changed

+94
-67
lines changed

‎src/core/friendly_errors/friendly_errors_utils.js

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
/**
2+
* Loads and returns a mapping of p5 constructor names to their corresponding
3+
* functions. This function iterates through all properties of the p5 object
4+
* identifying constructor functions (those starting with a capital letter) and
5+
* storing them in an object.
6+
* @returns {Object} An object mapping p5 constructor names to their functions.
7+
*/
8+
function loadP5Constructors(p5) {
9+
// Mapping names of p5 types to their constructor functions.
10+
// p5Constructors:
11+
// - Color: f()
12+
// - Graphics: f()
13+
// - Vector: f()
14+
// and so on.
15+
const p5Constructors = {};
16+
17+
// Make a list of all p5 classes to be used for argument validation
18+
// This must be done only when everything has loaded otherwise we get
19+
// an empty array
20+
for (let key of Object.keys(p5)) {
21+
// Get a list of all constructors in p5. They are functions whose names
22+
// start with a capital letter
23+
if (typeof p5[key] === 'function' && key[0] !== key[0].toLowerCase()) {
24+
p5Constructors[key] = p5[key];
25+
}
26+
}
27+
28+
return p5Constructors;
29+
}
30+
31+
export { loadP5Constructors };

‎src/core/friendly_errors/sketch_verifier.js

Lines changed: 24 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import * as acorn from 'acorn';
2-
import * as walk from 'acorn-walk';
1+
import { parse } from 'acorn';
2+
import { simple as walk } from 'acorn-walk';
33
import * as constants from '../constants';
4+
import { loadP5Constructors } from './friendly_errors_utils';
45

56
/**
67
* @for p5
@@ -38,27 +39,6 @@ function sketchVerifier(p5, fn) {
3839
'onended'
3940
];
4041

41-
// Mapping names of p5 types to their constructor functions.
42-
// p5Constructors:
43-
// - Color: f()
44-
// - Graphics: f()
45-
// - Vector: f()
46-
// and so on.
47-
const p5Constructors = {};
48-
49-
fn.loadP5Constructors = function () {
50-
// Make a list of all p5 classes to be used for argument validation
51-
// This must be done only when everything has loaded otherwise we get
52-
// an empty array
53-
for (let key of Object.keys(p5)) {
54-
// Get a list of all constructors in p5. They are functions whose names
55-
// start with a capital letter
56-
if (typeof p5[key] === 'function' && key[0] !== key[0].toLowerCase()) {
57-
p5Constructors[key] = p5[key];
58-
}
59-
}
60-
}
61-
6242
/**
6343
* Fetches the contents of a script element in the user's sketch.
6444
*
@@ -119,13 +99,13 @@ function sketchVerifier(p5, fn) {
11999
const lineOffset = -1;
120100

121101
try {
122-
const ast = acorn.parse(code, {
102+
const ast = parse(code, {
123103
ecmaVersion: 2021,
124104
sourceType: 'module',
125105
locations: true // This helps us get the line number.
126106
});
127107

128-
walk.simple(ast, {
108+
walk(ast, {
129109
VariableDeclarator(node) {
130110
if (node.id.type === 'Identifier') {
131111
const category = node.init && ['ArrowFunctionExpression', 'FunctionExpression'].includes(node.init.type)
@@ -181,7 +161,7 @@ function sketchVerifier(p5, fn) {
181161
* @param {Array<{name: string, line: number}>} userDefinitions.functions - Array of user-defined function names and their line numbers.
182162
* @returns {boolean} - Returns true if a conflict is found, false otherwise.
183163
*/
184-
fn.checkForConstsAndFuncs = function (userDefinitions) {
164+
fn.checkForConstsAndFuncs = function (userDefinitions, p5Constructors) {
185165
const allDefinitions = [
186166
...userDefinitions.variables,
187167
...userDefinitions.functions
@@ -193,7 +173,7 @@ function sketchVerifier(p5, fn) {
193173
// reference on the p5.js website.
194174
function generateFriendlyError(errorType, name, line) {
195175
const url = `https://p5js.org/reference/#/p5/${name}`;
196-
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}.`;
176+
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}.`;
197177
return message;
198178
}
199179

@@ -224,36 +204,19 @@ function sketchVerifier(p5, fn) {
224204
}
225205
}
226206

227-
// Get the names of all p5.js functions which are available globally
228-
const classesWithGlobalFns = ['Renderer', 'Renderer2D', 'RendererGL'];
207+
// The new rules for attaching anything to global are (if true for both of
208+
// the following):
209+
// - It is a member of p5.prototype
210+
// - Its name does not start with `_`
229211
const globalFunctions = new Set(
230-
classesWithGlobalFns.flatMap(className =>
231-
Object.keys(p5Constructors[className]?.prototype || {})
232-
)
212+
Object.keys(p5.prototype).filter(key => !key.startsWith('_'))
233213
);
234214

235215
for (let { name, line } of allDefinitions) {
236216
if (!ignoreFunction.includes(name) && globalFunctions.has(name)) {
237-
for (let className of classesWithGlobalFns) {
238-
const prototypeFunc = p5Constructors[className]?.prototype[name];
239-
if (prototypeFunc && checkForRedefinition(name, prototypeFunc, line, "Function")) {
240-
return true;
241-
}
242-
}
243-
}
244-
}
245-
246-
// Additional check for other p5 constructors
247-
const otherP5ConstructorKeys = Object.keys(p5Constructors).filter(
248-
key => !classesWithGlobalFns.includes(key)
249-
);
250-
for (let { name, line } of allDefinitions) {
251-
for (let key of otherP5ConstructorKeys) {
252-
if (p5Constructors[key].prototype[name] !== undefined) {
253-
const prototypeFunc = p5Constructors[key].prototype[name];
254-
if (prototypeFunc && checkForRedefinition(name, prototypeFunc, line, "Function")) {
255-
return true;
256-
}
217+
const prototypeFunc = p5.prototype[name];
218+
if (prototypeFunc && checkForRedefinition(name, prototypeFunc, line, "Function")) {
219+
return true;
257220
}
258221
}
259222
}
@@ -262,10 +225,18 @@ function sketchVerifier(p5, fn) {
262225
}
263226

264227
fn.run = async function () {
228+
const p5Constructors = await new Promise(resolve => {
229+
if (document.readyState === 'complete') {
230+
resolve(loadP5Constructors(p5));
231+
} else {
232+
window.addEventListener('load', () => resolve(loadP5Constructors(p5)));
233+
}
234+
});
235+
265236
const userCode = await fn.getUserCode();
266237
const userDefinedVariablesAndFuncs = fn.extractUserDefinedVariablesAndFuncs(userCode);
267238

268-
if (fn.checkForConstsAndFuncs(userDefinedVariablesAndFuncs)) {
239+
if (fn.checkForConstsAndFuncs(userDefinedVariablesAndFuncs, p5Constructors)) {
269240
return;
270241
}
271242
}
@@ -275,5 +246,4 @@ export default sketchVerifier;
275246

276247
if (typeof p5 !== 'undefined') {
277248
sketchVerifier(p5, p5.prototype);
278-
p5.prototype.loadP5Constructors();
279249
}

‎test/unit/core/sketch_overrides.js

Lines changed: 39 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,22 @@
11
import sketchVerifier from '../../../src/core/friendly_errors/sketch_verifier.js';
2+
import { loadP5Constructors } from '../../../src/core/friendly_errors/friendly_errors_utils.js';
23

34
suite('Sketch Verifier', function () {
45
const mockP5 = {
56
_validateParameters: vi.fn(),
6-
Renderer: function () { },
7+
Color: function () { },
8+
Vector: function () { },
9+
prototype: {
10+
rect: function () { },
11+
ellipse: function () { },
12+
}
713
};
8-
mockP5.Renderer.prototype.rect = vi.fn();
914
const mockP5Prototype = {};
15+
let p5Constructors;
1016

1117
beforeAll(function () {
18+
p5Constructors = loadP5Constructors(mockP5);
1219
sketchVerifier(mockP5, mockP5Prototype);
13-
mockP5Prototype.loadP5Constructors();
1420
});
1521

1622
afterAll(function () {
@@ -199,7 +205,7 @@ suite('Sketch Verifier', function () {
199205
variables: [{ name: 'PI', line: 1 }],
200206
functions: []
201207
};
202-
const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions);
208+
const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions, p5Constructors);
203209

204210
expect(result).toBe(true);
205211
expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Constant "PI" on line 1 is being redeclared and conflicts with a p5.js constant'));
@@ -210,13 +216,13 @@ suite('Sketch Verifier', function () {
210216
variables: [],
211217
functions: [{ name: 'rect', line: 2 }]
212218
};
213-
const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions);
219+
const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions, p5Constructors);
214220

215221
expect(result).toBe(true);
216222
expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Function "rect" on line 2 is being redeclared and conflicts with a p5.js function'));
217223
});
218224

219-
test('allows redefinition of whitelisted functions', function () {
225+
test('Allows redefinition of whitelisted functions', function () {
220226
const userDefinitions = {
221227
variables: [],
222228
functions: [
@@ -226,7 +232,7 @@ suite('Sketch Verifier', function () {
226232
]
227233
};
228234

229-
const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions);
235+
const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions, p5Constructors);
230236

231237
expect(result).toBe(false);
232238
expect(consoleSpy).not.toHaveBeenCalled();
@@ -238,14 +244,27 @@ suite('Sketch Verifier', function () {
238244
functions: [{ name: 'cut', line: 2 }]
239245
};
240246

241-
const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions);
247+
const result = mockP5Prototype.checkForConstsAndFuncs(userDefinitions, p5Constructors);
242248

243249
expect(result).toBe(false);
244250
});
245251
});
246252

247253
suite('run()', function () {
254+
let originalReadyState;
255+
256+
beforeEach(function () {
257+
originalReadyState = document.readyState;
258+
vi.spyOn(window, 'addEventListener');
259+
});
260+
261+
afterEach(function () {
262+
Object.defineProperty(document, 'readyState', { value: originalReadyState });
263+
vi.restoreAllMocks();
264+
});
265+
248266
test('Extracts user-defined variables and functions and checks for conflicts', async function () {
267+
Object.defineProperty(document, 'readyState', { value: 'complete' });
249268
const mockScript = `
250269
let x = 5;
251270
const y = 10;
@@ -281,10 +300,15 @@ suite('Sketch Verifier', function () {
281300
{ name: 'foo', line: 3 },
282301
{ name: 'bar', line: 4 }
283302
]
284-
});
303+
},
304+
expect.any(Object) // This is the p5Constructors object
305+
);
306+
expect(window.addEventListener).not.toHaveBeenCalled();
285307
});
286308

287309
test('Stops execution when a conflict is found', async function () {
310+
Object.defineProperty(document, 'readyState', { value: 'complete' });
311+
288312
const mockScript = `
289313
let PI = 3.14;
290314
function setup() {}
@@ -300,10 +324,12 @@ suite('Sketch Verifier', function () {
300324

301325
expect(mockP5Prototype.getUserCode).toHaveBeenCalledTimes(1);
302326
expect(mockP5Prototype.extractUserDefinedVariablesAndFuncs).toHaveBeenCalledWith(mockScript);
303-
expect(mockP5Prototype.checkForConstsAndFuncs).toHaveBeenCalledWith({
304-
variables: [{ name: 'PI', line: 1 }],
305-
functions: [{ name: 'setup', line: 2 }]
306-
});
327+
expect(mockP5Prototype.checkForConstsAndFuncs).toHaveBeenCalledWith(
328+
{
329+
variables: [{ name: 'PI', line: 1 }],
330+
functions: [{ name: 'setup', line: 2 }]
331+
},
332+
expect.any(Object)); // This is the p5Constructors object
307333
expect(result).toBeUndefined();
308334
});
309335
});

0 commit comments

Comments
 (0)