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

Fixing deprecated zustand imports & implementation, subsequently storing events in props.events #10

Open
wants to merge 1 commit into
base: develop
Choose a base branch
from

Conversation

Mozoloa
Copy link

@Mozoloa Mozoloa commented Jan 19, 2024

My ocd tickled me when I was seeing all the deprecation warnings so I fixed the zustand imports & implementations.

I also passed the events into the props the same way errors are passed. To make it niftier I made it a map with keys as event sources (names), so you can el.meter({ name: "leftInput}, some signal) and you'll recieve it via props.events.leftInput

Copy link
Contributor

@nick-thompson nick-thompson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @Mozoloa! Sorry for the delay here.

I'll definitely take the Zustand imports fix, but actually I think I'd prefer not to take the events store approach upstream. Would you mind updating the PR for just the imports fix?

The reason I say that is because these events can come in extremely fast. As written right now on the develop branch, events come at 60Hz (which is intended, so that you have data available for an accurate 60fps rendering of the UI for things like scopes and meters, though of course you could tune it differently for your own needs). Updating a zustand store at 60Hz in itself doesn't seem troubling, but hooking a React component to that store does, because running React's diff at 60Hz could cause a ton of overhead.

The approach I generally prefer for this kind of event handling would look more like updating that zustand store at 60Hz, but making sure there are no subscribers to that store (in which case, you might as well just use your own plain old data instead of zustand, but either way). Then, for drawing meters and scopes and the like, I create a canvas somewhere in the DOM (which is managed by React), and set up a requestAnimationFrame paint loop (which isnt managed by React), during which it reads the latest value from the store and paints to the canvas.

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

Successfully merging this pull request may close these issues.

2 participants