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

Stale event handlers with frequent state updates #330

Closed
brentbaum opened this issue Mar 18, 2021 · 8 comments · Fixed by #345
Closed

Stale event handlers with frequent state updates #330

brentbaum opened this issue Mar 18, 2021 · 8 comments · Fixed by #345
Assignees
Labels
type-bug About something that isn't working

Comments

@brentbaum
Copy link

Hello, I believe I'm running into the worst-case scenario for #19. I'm using idom for an industrial control panel that updates every 100ms. At that refresh rate, factoring the latency of a decent internet connection, about 1/3 of event handlers are ignored from the client (see logs below).

2021-03-17T17:39:12+0000 | INFO | idom.core.layout | Ignored event - handler '3021121024' does not exist or its component unmounted
2021-03-17T17:39:13+0000 | INFO | idom.core.layout | Ignored event - handler '3021515056' does not exist or its component unmounted
2021-03-17T17:39:21+0000 | INFO | idom.core.layout | Ignored event - handler '2986229456' does not exist or its component unmounted

Do you have any ideas on ways to mitigate this issue? We (https://www.beanstalk.farm/) are loving idom would be happy to contribute to the project or potentially pay for time to improve this.

@brentbaum brentbaum added the type-bug About something that isn't working label Mar 18, 2021
@rmorshea
Copy link
Collaborator

rmorshea commented Mar 18, 2021

Is there a way to pull the event handler outside the section of the UI that is being updated with that 100ms frequency? For example, in the Simple Dashboard from the docs, the control panel is separate from the section of the UI that is being regularly updated.

If not, my short term suggestion is to drop into Javascript for the highly updated section of your UI. There's an example that shows how you can do this with just a single .js file. Or you can create a full-blown React component from from this template repository.Let me know if you have questions about either JS option.

@rmorshea
Copy link
Collaborator

It's also really cool to hear how IDOM is being used in the wild! I do just want to emphasize that the project is under heavy development and that you may experience breaking changes if you don't have your dependencies pinned.

In any case though, I'd be interested to learn more about how you're using it at Beanstalk. If you're able, I've created a weekly set of "office hours" that you can just book a slot in: #312

@brentbaum
Copy link
Author

I'll go with the approach in the Simple Dashboard example. My use case is to disable buttons for jogging the machine when an incompatible move is in progress. I can handle that in another way to start but operator feedback RE what's possible is important.

I just signed up for a meeting with you this Wednesday. Looking forward to it!

@rmorshea
Copy link
Collaborator

rmorshea commented Mar 22, 2021

Ok, there's a solution to this, but it could take a while to fix. Basically the issue is that the layout throws away elements (and thus event handlers) on each render, but in reality those elements haven't really changed or they've just been modified and should actually have been preserved. This same problem is solved by React with its concept of keys.

@rmorshea
Copy link
Collaborator

Another sub-optimal way to work around this is to hard-code the event handler target ID:

@idom.component
def MyComponent():
    events = idom.Events()

    @events.on("click", target_id="a-globally-unique-id")
    def handler(event):
        ...

    return idom.html.button({"onClick": handler}, "click me")

This handler should respond to events regardless of how frequently any parent components are being updated because its target ID is fixed. The unfortunately thing is that the ID must be globally unique.

NOTES: this target_id param was originally intended to be used in testing, so IMO this is kind of an exploit of its behavior.

@brentbaum
Copy link
Author

Ok. I'll pass on that param - not sure if that is something you envisioning as part of the public api moving forward. We are in a fine place after moving the frequently updated state into a child component.

I've been thinking about how this might be solved without fully implementing the key rework you mentioned above... I'm hoping to test those ideas sometime soon and report back.

@rmorshea
Copy link
Collaborator

rmorshea commented Apr 2, 2021

@brentbaum any progress on that idea to avoid implementing keys?

@rmorshea
Copy link
Collaborator

rmorshea commented Apr 3, 2021

@brentbaum it's gonna be a bit, but I've started work to resolve this here: https://github.com/idom-team/idom/pulls

Still interested to hear if you've thought of an alternative though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type-bug About something that isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants