Version 5.0.0-rc.0

[jamesrweb]

Proposed Changes

• Migrate to vitest 1.x.x, update tests and coverage reporting for this.
• Utilize lazy component loading strategies to create smaller bundle sizes.
• Move p5 to a peer dependency to allow users to install their preferred version while drastically reducing our bundle size even further in the process.
• Implement an error boundary and expose ways for the consumer to handle error, loading and success cases if they want to. Provide simple defaults if they don't.
• Brings test coverage up to effectively 100% when the it.skip tests are eventually unskipped, for now it is 98.x% covered which is much higher than previous versions, even with the it.skip cases.
• Adds the React v19 compiler for development use

Additional Notes (optional)

This is a WIP and will probably take me some time to work through some ideas I have but this is the initial foundation of that process. That said, bundle size alone is massively reduced so far:

Version 4:

• 1 MB minified
• 237.8 KB minified + gzipped

Version 5 (latest update):

• 6.24 KB minified (99.376% decrease)
• 3.14 KB minified + gzipped (98.6796% decrease)

[jamesrweb]

@yevdyko this is basically ready to go pending your review but I want to leave it as a draft since the README will need updated still and maybe you or I have other ideas to add to this foundation. I won't push more changes until you review the current state, thanks in advance! 🚀

[yevdyko]

Thanks @jamesrweb! This looks like really good improvements. Much appreciate your efforts 🥇

Since this will be the new major version, maybe we can rename the main component. For example, from ReactP5Wrapper to P5Canvas. The idea is that we can be more particular in naming and specify what it is instead of just declaring that we wrap p5 in a component using React, which is obvious if you're building a React application. WDYT @jamesrweb ?

[jamesrweb]

Thanks @jamesrweb! This looks like really good improvements. Much appreciate your efforts 🥇

Since this will be the new major version, maybe we can rename the main component. For example, from ReactP5Wrapper to P5Canvas. The idea is that we can be more particular in naming and specify what it is instead of just declaring that we wrap p5 in a component using React, which is obvious if you're building a React application. WDYT @jamesrweb ?

I don't know, the component has been named this for 2 major versions if I am not mistaken and I think it is a "recognised" marker of the project. I don't know, I will think about it though!

[jamesrweb]

@yevdyko version 5 is basically ready now, please review and check the tests, documentation and see if I missed anything please! ❤️

[yevdyko]

@jamesrweb Thanks for the updates. I'll try to review the changes over the weekend

[yevdyko]

LGTM 🚀

Thanks for the new features and improving test coverage 💪

[yevdyko]

MINOR:

It would be nice to add a new line to the end of the file

[jamesrweb]

Pushed with a new line, mind if I ask why though? 😛

[jamesrweb]

Did you manage to test version 5 with any sketches of your own or any others? I would appreciate the feedback if you did, I tried with some of mine but simple use cases only so far.

[yevdyko]

As for the new line, it's specific to git, how the diff utility is implemented. That's why github highlighted it. Read more: https://unix.stackexchange.com/a/18744

We can add an editorconfig file to the repository to avoid this in the future: https://editorconfig.org/#example-file

[yevdyko]

I haven't tested it locally, but I may do so today / tomorrow

[jamesrweb]

Okay, I'll wait for you to test and give feedback then before merging.

[jamesrweb]

I think editorconfig is good but prettier would be better, maybe we can automate that somehow, a custom rule or simple plugin for example.

[yevdyko]

@jamesrweb I tried to test the latest changes in my demo app and ran into the issue when importing the component:

Compiled with problems:
×
ERROR in ./src/App.js 28:30-44
export 'ReactP5Wrapper' (imported as 'ReactP5Wrapper') was not found in '@p5-wrapper/react' (module has no exports)

I installed the new version of the package from git and built it inside node_modules, so maybe there is something wrong with the changes themselves or I'm not importing it correctly. So I'll try to figure out what's wrong in my case again tomorrow, but maybe you have some ideas.

[jamesrweb]

@jamesrweb I tried to test the latest changes in my demo app and ran into the issue when importing the component:

Compiled with problems:
×
ERROR in ./src/App.js 28:30-44
export 'ReactP5Wrapper' (imported as 'ReactP5Wrapper') was not found in '@p5-wrapper/react' (module has no exports)

I installed the new version of the package from git and built it inside node_modules, so maybe there is something wrong with the changes themselves or I'm not importing it correctly. So I'll try to figure out what's wrong in my case again tomorrow, but maybe you have some ideas.

It definitely builds it into the export list as shown in the image above. I wonder if you need to re-install your dependencies or something because the package.json values for main, module and exports changed in version 5 as shown here:

Also we have more files in the library due to the Suspense and lazy boundaries for lazy loading components on demand, etc, as shown here:

[yevdyko]

@jamesrweb I have updated all dependencies and cleaned up all modules before installing the new version. Have you tried using the new version in a standalone React app rather than a demo where the component is used directly from a folder nearby rather than the build?

[jamesrweb]

@jamesrweb I have updated all dependencies and cleaned up all modules before installing the new version. Have you tried using the new version in a standalone React app rather than a demo where the component is used directly from a folder nearby rather than the build?

The CJS version was overwriting main.js (since both used the same file name 😅), I fixed the builder and retested the demo and it works with the es version imported now. Please try again 😄

[yevdyko]

@jamesrweb Yeah, that solved the import issue. Thanks for the fix!

There are a few runtime errors related to the package and possibly to the changes:

[jamesrweb]

@jamesrweb Yeah, that solved the import issue. Thanks for the fix!

There are a few runtime errors related to the package and possibly to the changes:

I don't get this in the demo app for any of the sketches 🤔... you're sure that react, react-dom and p5 (since they're peer deps) are installed in your project?

The only refs being used in that file are to the canvas instance and the wrapper div. The null cases are handled in the effects and the refs mount the values when the component renders.

Not sure why this could be...

• What's your repo / setup like? Can you share?
• Is it in next or raw react?
• Did you check the DOM to see the render tree is correct?

[yevdyko]

I don't get this in the demo app for any of the sketches 🤔... you're sure that react, react-dom and p5 (since they're peer deps) are installed in your project?

The only refs being used in that file are to the canvas instance and the wrapper div. The null cases are handled in the effects and the refs mount the values when the component renders.

Not sure why this could be...

• What's your repo / setup like? Can you share?
• Is it in next or raw react?
• Did you check the DOM to see the render tree is correct?

@jamesrweb The dependencies are installed correctly. This is most likely due to your changes related to lazy loading. Unfortunately, this will not be reproducible in the demo from the package repository, as it does not use the built version of the package. Here is the repo with my test React app: https://github.com/yevdyko/react-p5-wrapper-demo/tree/test-version-5

Could you check this using my demo app or create your own with the latest version of the package installed in it?

[jamesrweb]

I don't get this in the demo app for any of the sketches 🤔... you're sure that react, react-dom and p5 (since they're peer deps) are installed in your project?

The only refs being used in that file are to the canvas instance and the wrapper div. The null cases are handled in the effects and the refs mount the values when the component renders.

Not sure why this could be...

• What's your repo / setup like? Can you share?
• Is it in next or raw react?
• Did you check the DOM to see the render tree is correct?

@jamesrweb The dependencies are installed correctly. This is most likely due to your changes related to lazy loading. Unfortunately, this will not be reproducible in the demo from the package repository, as it does not use the built version of the package. Here is the repo with my test React app: https://github.com/yevdyko/react-p5-wrapper-demo/tree/test-version-5

Could you check this using my demo app or create your own with the latest version of the package installed in it?

I did use the built script from the demo by manually changing the path to use the script from dist and it was fine. 🤷🏻‍♂️

Note: I accidentally edited your comment earlier instead of replying, no idea how that happened but sorry 😂😂

[jamesrweb]

@yevdyko I just added the react v19 compiler for development into this version, to ensure we are adhering to the rules of hooks, rules of react and are prepared for the upcoming version 19 changes.

[yevdyko]

@jamesrweb I have reinstalled the packages in my demo application. There seems to be some runtime errors related to the changes in ReactP5WrapperGuard:

[jamesrweb]

Try installing react@beta and react-dom@beta and let me know if it works then? If it does I might need to decide to either:

• A: drop react 19 support in the build
• B: only support react 19+ for version 5

React 19 is just round the corner so this might not be such a bad thing and from my own testing, yes it's beta in the meantime but it's very stable so far in other projects of mine so I'd be okay with that, WDYT? 🤷🏻‍♂️

[yevdyko]

@jamesrweb After installing react@beta, there are still runtime errors related to useRef:

I assume this is not related to changes in react 19 support, but it may be relevant to the ReactP5WrapperGuard component as I mentioned above.

It would be nice if you tested your branch with an external React app, otherwise you can release as is, but there is a good chance the package won't work I think. So it's up to you.