support datepickersingle as an uncontrolled component

[chriddyp]

In dash-renderer, we now provide setProps for all components, regardless of whether they are connected to the backend.

However, the setProps that we provide needs an ID in order for it to update the component:

https://github.com/plotly/dash-renderer/blob/84c484b26452458a28843d02e4431cc88a623e14/src/TreeContainer.js#L146-L181

Some of our components, like DatePickerSingle, have legacy logic that handled old setProps logic - they would setState if setProps wasn't defined.

In some cases, like DatePickerSingle, this logic wasn't tight: instead of just calling setState, they would call setProps and setState, which makes it pretty hard to reason about when renderings happen. It's also fragile - if dash-renderer changes the way that it renders components, that rendering order may break the components.

I believe this is what happened in the dev-tools PR - the way that the components were rendered by dash-renderer changed which caused some of this fragile logic to break the components in certain cases.

This PR improves the state/props management of DatePickerSingle to be explicit about when to use state and when to use props by making it conditional on the presence of the id.

It also cleans up the moment-conversions by treating them as a data conversion step during render. It was pretty hard to reason about the logic before (moment props were in state)

In practice, most controls will have an ID (as they are inputs to callbacks). However, as users are authoring their app's layout, they may include some controls without IDs to start. If we don't manage the state, then the user may be surprised the component reacts different to user input when it is "unconnected" (without an ID) vs when it is connected.

This should fix the image tests issues we are seeing in #523

[chriddyp]

Are we committed to keeping these jest tests? I feel like they hook too far deeply into the underlying implementation (setPropsSpy) and as we say in this case, didn't catch the issue.

The actual behaviour is already tested in the integration tests.

[alexcjohnson]

Does DatePickerRange have the same problem? Looks like it uses a very similar pattern.

I'm OK with this as a stopgap, but please open an issue in dash-renderer to support setProps with no id, so that we can remove setState entirely (or at least stick it in an if(!setProps) that would only be used in case people want to test their components outside dash-renderer)

[chriddyp]

Does DatePickerRange have the same problem?

It does, a similar commit is on its way up.

please open an issue in dash-renderer to support setProps with no id,

👍 - plotly/dash-renderer#163

[alexcjohnson]

Are we committed to keeping these jest tests?

Dunno about these specific ones, if you're confident the behavior they're testing is covered elsewhere feel free to 🔪. But I do like having the framework around for situations where we want to test a million similar cases more efficiently than selenium keypresses.

[byronz]

StaleElementReferenceException: Message: stale element reference: element is not attached to the page document oh, this is an old selenium friend of mine

[chriddyp]

oh no! i'll leave that one to you @byronz 😸

[byronz]

@chriddyp I think the test failure captures a bug in implementation logic

[byronz]

this swallow the start date in bothdates
mode

[chriddyp]

😸