[p5.js 2.0 Beta Bug Report]: Output a friendly error when p5 is loaded twice on a page

[davepagurek]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL
• Build process
• Unit testing
• Internationalization
• Friendly errors
• Other (specify if possible)

p5.js version

2.0 beta 5

Web browser and version

Firefox

Operating system

MacOS

Steps to reproduce this

Steps:

1. Go to the p5 website repo
2. Switch the 2.0 branch
3. Run npm run dev
4. Open the reference for e.g. circle at http://localhost:4321/reference/p5/circle/

You see this error in the console

Not sure what the cause is just yet. Doesn't seem to affect the examples though.

[VANSH3104]

@davepagurek can i give it a try

[davepagurek]

Sure thing @VANSH3104, even without a fix yet, let me know in a comment here if you can figure out what the cause is! Now that we've started testing out a beta site for 2.0, you can see this happening for yourself at e.g. https://beta.p5js.org/reference/p5/arc/ and looking in your browser console.

[VANSH3104]

@davepagurek I checked it out in the browser console, and it seems like the issue is related to how width is defined. The error Uncaught TypeError: can't redefine non-configurable property "width" suggests that width might already be locked, possibly by another script or browser restrictions. I also noticed multiple instances of p5.js being loaded, which I think could be causing conflicts.
whats your thought ?

[davepagurek]

Thanks @VANSH3104! I think you're right, the script tag is getting included multiple times on the reference page, which I can see in the browser's element inspector dev tools:

So I think there are actually two problems going on here:

1. (For this repo) When there is already a p5 instance running, rather than outputting a friendly error indicating that p5 is loaded twice, we're getting a cryptic error about property redefinition. Maybe we can detect some other way when p5 is loaded twice (e.g. window.p5 exists already?) and halt initialization with a descriptive message?

2. (For the p5.js-website repo) Stop p5 from being loaded multiple times on reference pages.

• Each example in the p5 reference loaded in an <iframe> tag with the following source: https://github.com/processing/p5.js-website/blob/8e571e8ca5b46cde3b4bb7a149307b705d7758c8/src/components/CodeEmbed/frame.tsx#L31

• It might be because of this: https://github.com/processing/p5.js-website/blob/8e571e8ca5b46cde3b4bb7a149307b705d7758c8/src/components/CodeEmbed/index.jsx#L70

  • To make sure the p5 script is cached when multiple examples use it, a <script> tag is created on the main page (this triggers the browser/service worker to cache its contents) and then the contents is fetched (which uses the cache) and sent to each example here: https://github.com/processing/p5.js-website/blob/8e571e8ca5b46cde3b4bb7a149307b705d7758c8/src/components/CodeEmbed/frame.tsx#L134
  • It looks like it doesn't check if that script tag already exists before making it, so every example creates a new tag
  • Maybe we just need to check if something with that id already exists on the page and only make a new tag if it's not already there?

[davepagurek]

I renamed this issue and made an issue for the website repo here: processing/p5.js-website#770

[davepagurek]

More context on the issue here:

• We detect double-initialization by checking for window._setupDone:

  p5.js/src/core/init.js

  Line 19 in 3e2d17a

  if (typeof window._setupDone !== 'undefined') {

• We still set it:

  p5.js/src/core/main.js

  Line 268 in 3e2d17a

  this._setupDone = true;

  • ...but we don't put anything onto window that starts with an underscore:

    p5.js/src/core/main.js

    Lines 143 to 144 in 3e2d17a

    	if(p[0] === '_') continue;
    	bindGlobal(p);

  • ...but it this is only set after setup finishes, and we might do the duplicate variable checks before that

So I think we maybe need to update it to look for a different variable that we can set immediately once we know we're in global mode. Maybe we make a window._p5Initialized?

[VANSH3104]

@davepagurek Have you found anything else on the double-initialization issue? The changes window._p5Initialized don’t seem to work as expected.

[davepagurek]

@VANSH3104 I haven't tried it out myself, if you've started and want some help debugging feel free to open a draft PR and I can take a look!

[limzykenneth]

The error logged is because bindGlobal attaches the p5 global mode variable to window using Object.defineProperty(), which by default has configurable set to false which means future attempt at trying to do the same thing will fail with that error.

When p5.js is loaded multiple times, the global initialization is called multiple times, ie. fulfilling the error condition above. In 1.x there are checks to prevent multiple initialization I think but I must have accidentally removed it if it existed. We can reimplement it by checking for the existence of window.p5 and halt initialization if the value is already defined. _setupDone() or any checking of full initialization run the risk of being detected too late because of async loading and out of order script tag execution so I think window.p5 is likely more guaranteed to detect previous import of p5.js.

[VANSH3104]

Hi @davepagurek, I’ve created a draft PR and incorporated @limzykenneth’s suggestion. It adds a check to prevent reinitialization if p5 is already loaded, helping to avoid multiple initialization errors. Would appreciate it if both of you could take a look when you get a time. Thanks!