some issue with tabs reverting their value?

[chriddyp]

It's being reported in the community forum: https://community.plot.ly/t/problem-with-callbacks-in-different-tabs/13044/13?u=chriddyp

[valentijnnieman]

I'm looking into this but I'm not sure if it's really a Tabs problem, that is, I think it might be an issue in dash-renderer where the updating of props is not happening 100% correctly. Here's a console output of this example taken from the forums:

import dash
import dash_html_components as html
import dash_core_components as dcc
from dash.dependencies import Input, Output, State

app = dash.Dash()
app.config['suppress_callback_exceptions']=True

app.scripts.config.serve_locally = True

tab1_layout = [
    html.Div([
        html.H2("This is the default first tab page.")
    ])
]

tab2_layout = [
    html.Div([
        dcc.RadioItems(
            id='dropdown',
            options=[{'label': i, 'value': i} for i in range(3)],
            value=0,
        ),
        html.H2(id='header')
    ])
]

app.layout = html.Div([
    dcc.Tabs(id="tabs", value=1, children=[
        dcc.Tab(label='1', value=1, children=tab1_layout),
        dcc.Tab(label='2', value=2, children=tab2_layout)
    ])
])

@app.callback(
    Output('header', 'children'),
    [Input('dropdown', 'value')])
def update_header(v):
    return 'The selected number is ' + str(v)

if __name__ == "__main__":
    app.run_server(debug=True)

after clicking on the 2nd tab, the Tab's internal selectHandler fires with an argument of 2 (the value prop of that Tab). render() is being called again, and in render we see that state.selected equals 2, which is correct. Then, if I click on a radio button, the Tabs render() is somehow called again, and it's value is updated to it's default (1, taken from the parent Tabs component) again. Note that no selectHandler() is being called, nor constructor() - the only two places in the Tabs component source code where state.selected is being changed. This makes me believe that somehow dash-renderer is re-rendering the Tabs component and resetting it's props.
@chriddyp what do you think?

[chriddyp]

Yeah, that sounds plausible. This is the first component that has demonstrated an implicit link between children and parents, and so I wouldn't be surprised if there was some surprising behaviour, probably when we're doing a createElement here:
https://github.com/plotly/dash-renderer/blob/b1cfc996563bd0b57f9487b15212dd5b782e5b3a/src/TreeContainer.js#L75-L79

I'd try to keep the Tabs stateless when it's connected to dash-renderer. That is, instead of lines like:

        this.setState({
            selected: value,
        });
        if (this.props.setProps) {
            this.props.setProps({value: value});
}

have:

        if (this.props.setProps) {
            this.props.setProps({value: value});
      } else {
        this.setState({
            selected: value,
        });
      }

That is, if it's connected to dash-renderer (setProps is defined), then let dash-renderer control selected value just through props. If it's not connected to dash-renderer, then just handle the state internally in the component.

[valentijnnieman]

@chriddyp Ah yeah, so that's the same problem as this issue right? I tried to come up with a better solution for that but haven't yet.

That suggestion is interesting and probably a good idea, but in the example above there are no callbacks assigned to the Tabs, so won't that leave setProps empty meaning it will use setState anyway?

[chriddyp]

Yeah, good point. Could you be a little more precise on this statement?

Then, if I click on a radio button, the Tabs render() is somehow called again, and it's value is updated to it's default (1, taken from the parent Tabs component) again.

Do you mean the Tab render is called again, taken from the parent Tabs component? There is only one parent Tabs instance here.

[valentijnnieman]

Sorry, that was confusing, let me rewrite that.

Then, if I click on a radio button, Tabs.render() is called again, and state.value is now 1 again (which is how it was set in the app: dcc.Tabs(value=1 ...). Neither the constructor() nor the selectHandler() methods are being called (the only two places that modify state).

[chriddyp]

• the only two places in the Tabs component source code where state.selected is being changed.

It's also being updated in componentWillReceiveProps right?

dash-core-components/src/components/Tabs.react.js

Lines 153 to 160 in d004b29

	componentWillReceiveProps(newProps) {
	const value = newProps.value;
	if (typeof value !== 'undefined') {
	this.setState({
	selected: value,
	});
	}
	}

[valentijnnieman]

Oh, yes, you're right.

[valentijnnieman]

So I guess it's receiving new props coming from dash-renderer there. Only, it shouldn't - there are no callbacks being fired that involve Tabs.

[chriddyp]

Only, it shouldn't - there are no callbacks being fired that involve Tabs.

Yeah, so you can't assume that dash-renderer will only re-render your component when new properties change. Your component needs to be idempotent: safe to re-render by the parent multiple times.

In this case it seems excessive, but imagine if a callback updated a different property of the Tabs like style: we'd need to call componentWillReceiveProps with that new style property and re-render the table.

[valentijnnieman]

Ah right, I remember now. I think my thinking was this: when the Tab's value get's updated, for example in selectHandler, it updates it in this.state and also fires setProps() to update the value in the Dash, so that Dash knows the value prop has been changed. But because there's no callback, I guess setProps isn't set, so it isn't updating in Dash, which, when it rerenders, it will rerender with what Dash thinks it's value should be.

What's the solution then? If we want users to use the Tabs without a callback, then I don't see a way of updating the value prop in Dash if setProps isn't set. If we don't set the value on componentWillReceiveProps, then callbacks wouldn't work anymore.

We could add a mode prop that if set to auto it will ignore props coming in from componentWillReceiveProps or something, but I'd rather not do that if we can avoid it.

Any other suggestions?

[chriddyp]

What about something like this:

constructor() {
    this.state = this.props;
}

render() {
   let style, selected, ...;
   if (this.props.setProps) {
       {style, selected, ...} = this.props;
   } else {
       {style, selected, ...} = this.state;
   }
}

[valentijnnieman]

Hmm I don't think that would solve anything, unfortunately! The issue is that the value prop is being handled by React's state, so that the Tabs component can handle it's own selection logic (the way of using Tabs without callbacks). But what we see happening here is that when Dash is rerendering, it apparently passes the value prop as Dash knows it back to the Tabs component. So internally, the value as it is in this.state is 2, but because Dash has not been informed (because setProps isn't set, the value prop cannot be sent back to Dash), Dash will push the value prop as 1 (because that's what it is in the beginning, when setting it as dcc.Tabs(value=1, ...). Sorry if I'm missing anything here.

Basically, the way of using Tabs without a callback is kind of flawed, because Dash needs to be informed about prop changes, but can't update Dash on prop changes without setProps, right? Short from making setProps available on the Tabs component even if there are no callbacks, or removing the Tabs functionality of not needing callbacks to work, I'm out of ideas! I'll think about it some more tomorrow. Thanks for your help, Chris!

[chriddyp]

Here's the logic where we determine whether or not to pass in setProps or not, I recommend digging into that:
https://github.com/plotly/dash-renderer/blob/b1cfc996563bd0b57f9487b15212dd5b782e5b3a/src/components/core/NotifyObservers.react.js#L68-L73

From that logic, I forget if that we're checking if that component is an output or if we're checking if it's an input but I believe that its checking if that component has any properties that are Inputs, meaning that they need to share their updates with dash-renderer so that it can fire callbacks.

If so, you're right in that the component could be an output, so we still need to check if any props have changed, we can't just rely on this.state.

And since the selected property itself could be an output (another control could update which tab is selected), we need to update our this.state.selected if the property has changed.

However, no other property can change within the tab itself (via user interaction), so we don't need to hold anything else in state, we only need to keep track of selected.

So, could we have logic like this?

constructor() {
    this.state = {selected: this.props.selected || this.props.children[0].props.children.value};
}

componentWillReceiveProps(newProps) {
    if(newProps.selected !== this.props.selected) {
         /*
          *  user updated selected via some callback or 
          * the tab was changed by calling `setProps`
          */
         this.setState({selected: newProps.selected});
    }
}

selectHandler(value) {
    if (this.props.setProps) { this.props.setProps({selected: value}); }
    else { this.setState({selected: value}); }
}

render() {
    const {selected} = this.state;
    const {style, ...} = this.props;
    return ...
}

In summary:

• Initialize the state with the selected value or a default
• If its controlled by dash-renderer (setProps is defined), then use setProps to update the selected value
• Otherwise, if it's uncontrolled, then only udpate the state via this.setState and ignore the props coming in (as they haven't changed)
• If value changes during a render, then it means that a callback updated it - hydrate the state with the new value

[valentijnnieman]

@chriddyp You're absolutely right - I was thinking of it in the wrong way and didn't realize that we could just skip using this.state if setProps was defined. Was a pretty easy fix in the end, I was just going down the wrong rabbit hole I guess! Thanks for your help. I've pushed a fix to #315, maybe you can review it?