Skip to content
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.

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

Reference pages with multiple examples load p5 multiple times #770

Closed
davepagurek opened this issue Apr 3, 2025 · 6 comments · Fixed by #774
Closed

Reference pages with multiple examples load p5 multiple times #770

davepagurek opened this issue Apr 3, 2025 · 6 comments · Fixed by #774
Labels
Bug Something isn't working Help Wanted Extra attention is needed

Comments

@davepagurek
Copy link
Collaborator

Most appropriate sections of the p5.js website?

Reference

What is your operating system?

Mac OS

Web browser and version

Firefox

Actual Behavior

If you open https://beta.p5js.org/reference/p5/arc/ and inspect element, the head tag has multiple p5 scripts:

Image

  • Each example in the p5 reference loaded in an <iframe> tag with the following source:

    const wrapInMarkup = (code: CodeBundle) =>

  • It might be because of this:

    const p5ScriptElement = document.createElement("script");

    • 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:
      const p5ScriptText = await fetch(p5ScriptTag.src).then((res) =>
    • 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?
    • Alternatively, we load and cache it without using a script tag at all (will a fetch() cache it?)

Expected Behavior

  • Ideally, p5 is not loaded at all on the main page, and is just loaded in the example iframes.
  • Whatever we do, in the browser dev tools network tab, we should only see the script being loaded over the internet once for the whole reference page -- it should be cached when subsequent examples use it.

Steps to reproduce

Open https://beta.p5js.org/reference/p5/arc/ and inspect element, then look at the head tag

Would you like to work on the issue?

Someone else can take this!

@webermayank
Copy link
Contributor

webermayank commented Apr 3, 2025

Hey @davepagurek , i tried adding a simple check if (!document.getElementById("p5ScriptTag"))

https://github.com/processing/p5.js-website/blob/8e571e8ca5b46cde3b4bb7a149307b705d7758c8/src/components/CodeEmbed/index.jsx#L70C1-L74C10

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?

it did solve the issue

Image

is it okay , or it is forcefully checking the script tag ?
i thought it would be better to ask before PR

@davepagurek
Copy link
Collaborator Author

Sounds good @webermayank! I think it's still true that we don't need the script tag at all on the main page since we don't actually use it there, just for caching, but this fix seems straightforward and fixes the main problem. Feel free to make a PR!

@webermayank
Copy link
Contributor

@davepagurek , should i make PR to main branch or 2.0 branch

@davepagurek
Copy link
Collaborator Author

If you go to the live non-beta site and open the dev tools element inspector, do you see multiple script tags there too? if so, we can make the change on main, and I can copy the change to the 2.0 branch as well once it's merged.

@webermayank
Copy link
Contributor

Yes https://p5js.org/reference/p5/arc/ also have multiple script tag , so i will make PR to main branch.

@VANSH3104
Copy link

@webermayank The current code change looks good and effectively prevents duplicate <script> tags by checking if the p5ScriptTag already exists nice work!
Minor suggestion for improvement:
To ensure clarity and improve maintainability, it might help to abstract this logic into a helper function.This can make the code a bit cleaner, especially if similar patterns are reused elsewhere. I saw that same things happened for all 2d premetives

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working Help Wanted Extra attention is needed
Projects
None yet
4 participants