-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[BUG] Concurrent callbacks broken in Dash 1.13+? #1350
Comments
I encountered the same issue with new versions of Dash (1.12 onward). |
@alexcjohnson This is rather interesting, running this against Dash v1.5.0 and above, I get the following callbacks on click for all versions but Dash v1.11 and Dash v1.12: Clicking on the
Clicking on the
For Dash v1.11 and v1.12, I get a slightly different: Clicking on the
Clicking on the
v1.11 and v1.12 are the only versions for which the code above can work. Interestingly enough since React does not prevent event propagation automatically when an event is handled, it's normal to see both the I wonder if the correct approach for html components would be to by-default (or opt-in) stop event propagation when it triggers a callback. Allowing for the use case described here to actually work. |
I don't think it's related as both the |
Weird that 1.11 and 1.12 trigger with If I had thought about this situation before seeing what we did, I'd say it should trigger the callback once with both props triggering. It's only a single action after all, so it should appear as a single action. That looks like it would work for this use case, and even without changing the code. If we later add an ability to stop propagation then fine, but that will need to be opt-in regardless. @Marc-Andre-Rivet perhaps there's a way we can make what I'm describing happen by somehow waiting to fire callbacks until propagation has had a chance to complete? |
@alexcjohnson I think the best candidate might be introducing an artificial async/wait delay here, just before getting the variables: https://github.com/plotly/dash/blob/dev/dash-renderer/src/observers/requestedCallbacks.ts#L56 - processing of |
@alexcjohnson Adding a delay does cause the Seems that instead of simply dropping the older callbacks we would now want to merge them, giving priority to props of newer callbacks. |
good point! It doesn't seem to me as though we ever want to simply drop the older copy, and there's no ambiguity about prop values, the only question is what to do with triggers. I think merging them is correct, just trying to think whether there are any edge cases we need to consider. OK here's one. I don't know why you'd do this but see if this makes sense:
Anyway merging the triggers would still be the correct thing to do, this is just a second-order issue to look out for after that. Edit: we might already handle this case correctly, based on this logic: dash/dash-renderer/src/actions/dependencies.js Lines 929 to 933 in d9a8d9c
|
Describe your context
The MWE code below does work in this environment:
The MWE code below does not work in this environment:
Describe the bug
When there are nested elements with callbacks (e.g. a button inside a div and both have conflicting callbacks), the documented method for differentiating between them (checking
ctx.triggered[0]['prop_id'].split('.')[0]
) does not work for Dash 1.13+ (it works with 1.12, though). I am not completely sure whether this was a deliberate change and an update to the documentation is needed or this is a bug.I have also asked in the community forum first.
Minimal working example code
Expected behavior
When the button is pressed, the info box should be displayed. When the background is clicked, the infobox should be hidden.
This code produces the expected behaviour in Dash 1.12. There, the callback is executed once only and
prop_ids
is a list of all callbacks ([“button”, “div”]
when clicking on the button and[“div”]
when clicking on the div).Actual behavior
Since version 1.13, dash behaves differently. The callback function is now executed twice, one time for the button callback and one time for the div. This makes it impossible to figure out which one was actually pressed.
Example output (Dash 1.13) for the above code after clicking one time on the button (the None callback is just the initial callback on load an can be ignored):
So, the button callback is executed, the infobox is displayed, but then the div callback immediately hides the infobox again.
The text was updated successfully, but these errors were encountered: