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

Callbacks inconsistencies. #396

Open
T4rk1n opened this issue Sep 19, 2018 · 3 comments
Open

Callbacks inconsistencies. #396

T4rk1n opened this issue Sep 19, 2018 · 3 comments

Comments

@T4rk1n
Copy link
Contributor

T4rk1n commented Sep 19, 2018

I have been having strange issues when developing new components and dash apps and I'd like to share them.

Sometimes callbacks are not called during initialization, sometimes the default prop never get applied. There are some inconsistencies with the callback system.

If a prop doesn't have a defaultProps entry, it never get an initial callback.

Also true if the default props is nil. Discovered while working on the Storage component, I initially thought that setProps didn't work in componentWillMount, when I added a timestamp with a default of -1, I was getting the callback.

Example to get the data props from a storage component using the timestamp on initial load:

import dash

from dash.dependencies import Output, Input, State

import dash_html_components as html
import dash_core_components as dcc
from dash.exceptions import PreventUpdate

app = dash.Dash(__name__)
app.scripts.config.serve_locally = True

app.layout = html.Div([
    dcc.Storage(id='storage'),
    html.Button('set storage', id='set-storage'),
    html.Div(id='output')
])


@app.callback(Output('storage', 'data'), [Input('set-storage', 'n_clicks')])
def on_click(n_clicks):
    if n_clicks is None:
        raise PreventUpdate
    return 'initial'


@app.callback(Output('output', 'children'),
              [Input('storage', 'modified_timestamp')],
              [State('storage', 'data')])
def on_init(ts, data):
    return data


if __name__ == '__main__':
    app.run_server(port=30600, debug=True)

If you click the button, then reload the page, it should show initial below the button.

If you remove the modified_timestamp input:

@app.callback(Output('output', 'children'),
              [Input('storage', 'data')])
def on_init(data):
    return data

It won't be called on initial load.

Add a second Input to an input list: the default props never get applied to the Inputs it stays at None.

import dash

from dash.dependencies import Output, Input
from dash.exceptions import PreventUpdate

import dash_html_components as html

app = dash.Dash(__name__)
app.scripts.config.serve_locally = True

app.layout = html.Div([
    html.Button('btn1', id='btn1'),
    html.Button('btn2', id='btn2'),
    html.Div(id='output')
])


@app.callback(Output('output', 'children'), [Input('btn1', 'n_clicks'), Input('btn2', 'n_clicks')])
def on_click(n1, n2):
    if n1 is None:
        raise PreventUpdate
    print(n1)
    print(n2)
    return 'Hello'


if __name__ == '__main__':
    app.run_server(port=30500, debug=True)

If you only click btn2, you will always get a PreventUpdate, it also ends up missing the second initial callback.

There is two initial callbacks that can pop up, only the second is useful since the first one only have None as arguments.

I think we should remove the first callback with all None arguments. It has no use and only create bloat IMO because I always have a check if None then raise PreventUpdate. The only time it is useful is if you want to act on the initial layout, but if we fix the initial callback for non default props it's a non-issue.

Also could be useful to have the initial values if we implement state saving for the dash-renderer redux store.

cc @plotly/dash

@T4rk1n
Copy link
Contributor Author

T4rk1n commented Sep 19, 2018

Found the underlying issue with the first problem, it is not related to defaultProps but the way the renderer asks if the controllersInFutureQueue. The callback which output to the storage is blocking the one which output to the output children div since that one requires the storage.data.

The behavior is explained in dash-renderer:
https://github.com/plotly/dash-renderer/blob/b712db585d685467d7fafed2655d8f6939627c28/src/actions/index.js#L285-L301

I think the logic there is sound, but the unexpected behavior comes from the None calls that are undesired. If a component do setProps in the componentWillMount on the initial layout, it shouldn't be blocked by a callback that shouldn't even fire in the first place.

@T4rk1n
Copy link
Contributor Author

T4rk1n commented Sep 20, 2018

I've narrowed down the culprit for the None callbacks in dash-renderer:

https://github.com/plotly/dash-renderer/blob/b712db585d685467d7fafed2655d8f6939627c28/src/actions/index.js#L83-L87

By debugging, I can tell the only times it is getting called is to make those initial layout callbacks. I removed those lines and the None callbacks were gone but also those that are set by layout (eg Input('div','id')). So instead, I added a isNil check and we don't get the None callbacks while still preserving the good one. It's basically doing the is None: PreventUpdate part on the client side.

@SterlingButters
Copy link

SterlingButters commented Mar 6, 2019

Didn't bother investigating the nitty gritty of your debugging but can confirm weird things happing with Store component. In two separate instances the data in Store (of type session) is consistently wiped on a refresh. Additionally, it seems that the odd and inconsistent behavior (e.g. output to children in completely separate callback not showing up but printing in console (and only once), dropdown values not clearing (somewhat related callback), etc) is propagating throughout my code making it hard to diagnose the problem. I am hoping this is my issue.

I would be happy to share my code causing the problem if it would help though its few hundred lines so not a prime minimal example (but its hard for me to locate the problem like I said).

HammadTheOne pushed a commit to HammadTheOne/dash that referenced this issue May 28, 2021
HammadTheOne pushed a commit that referenced this issue Jul 23, 2021
AnnMarieW pushed a commit to AnnMarieW/dash that referenced this issue Jan 6, 2022
Add Renovate Bot configuration to repo
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants