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

P5-wrapper size #282

Closed
JeffML opened this issue Oct 29, 2023 · 5 comments
Closed

P5-wrapper size #282

JeffML opened this issue Oct 29, 2023 · 5 comments
Assignees

Comments

@JeffML
Copy link

JeffML commented Oct 29, 2023

          Linked to this I think, not an issue with the wrapper.

processing/p5.js#1734 (comment)

Originally posted by @subodhpareek18 in #26 (comment)

I reviewed the above issue in P5 proper, and still have some questions:

  1. is p5-wrapper using the minified version of the p5 library as a dependency?
  2. if so, is it also including p5-sound as a dependency?
    1. this library has a problem, in that it includes the unminified P5 library (see issue link)
  3. if 1 & 2 are true, then can't p5-wrapper be split into (a) p5-wrapper; (b) p5-sound-wrapper, such that including (a) doesn't bloat the dependency if (b) isn't included as well?
@jamesrweb
Copy link
Collaborator

jamesrweb commented Nov 2, 2023

@yevdyko can you take a look into this, maybe the imported values should be updated.

Thanks @JeffML for the topic. If it's something we can change or brings benefits, we will! Appreciate it.

@yevdyko
Copy link
Collaborator

yevdyko commented Nov 2, 2023

@jamesrweb sure thing, I will check the bundle sizes to see if it can apply to us, and thanks for pointing that out @JeffML

@jamesrweb
Copy link
Collaborator

@yevdyko any updates here?

@jamesrweb
Copy link
Collaborator

jamesrweb commented Mar 2, 2024

I looked into this issue and the outcome is:

  1. p5 does use the minified version by default in the current version at least (didn't check others)
  2. The minified version is 1,029.38 kB by default and 260.21 kB when gzipped
  3. We could reduce upfront load by using React.lazy and more aggressive code-splitting in the vite build but in my testing this only saved less than 0.5% of the current bundle size and that took quite a bit of configuring to do. Not sure if it is worth the future administration for such little gain. I will consider it.

So to answer the original questions from @JeffML:

  1. Yes, by default from p5 themselves, in the upstream dependency.
  2. Not unless you import it yourself and there is also a p5/lib/addons/p5.sound.min file available for import also FYI.
  3. No since we rely on the upstream p5 dependency and since they use p5/lib/p5.min by default as the main entrypoint, we are already using the minified version. We don't use sound or other plugins and so optimization routes for us are either further code-splitting of the internal components (but these are already extremely small) or applying fixes upstream which depends on the p5 maintainers.

Anyway, closing this issue for now. Thanks for raising the issue, it was interesting to look into 😄.

@yevdyko
Copy link
Collaborator

yevdyko commented Mar 3, 2024

@jamesrweb Moreover, after releasing a new version of the package in #325, where p5 moved to a peer dependency, this becomes irrelevant to the package itself. Which seems like the right solution to me.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants