Skip to content
This repository has been archived by the owner on Jun 3, 2024. It is now read-only.

Async components #692

Merged
merged 12 commits into from
Nov 4, 2019
Merged

Async components #692

merged 12 commits into from
Nov 4, 2019

Conversation

Marc-Andre-Rivet
Copy link
Contributor

@Marc-Andre-Rivet Marc-Andre-Rivet commented Nov 1, 2019

Follow up to #616

Brings async to components:

  • datepicker single/range
  • dropdown
  • markdown
  • upload

Slider/RangeSlider behave incorrectly when asynced -- they seem to execute just fine but they display nothing. Needs to be investigated further, and there is diminishing returns.

Brings dcc.min.js entry point down from ~1.09MB to ~374kB.

None of these components have side-effects, so they do not use the async decorator.

  • changelog
  • fix percy diffs for datepickers

@Marc-Andre-Rivet Marc-Andre-Rivet added this to the Dash v1.6 milestone Nov 1, 2019
@Marc-Andre-Rivet Marc-Andre-Rivet marked this pull request as ready for review November 1, 2019 20:48
}

componentWillMount() {
this.propsToState(this.props, true);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the most significant code change. Since the real/fragment component is wrapped, its props lifecycle is slightly different, propsToState is updated to only update the state when the value changes or if forced (on mount)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm missing something - this seems like it would have been a good idea before, but somehow more than a good idea it's actually necessary now? What's the bad thing that happens if we don't do this?

Copy link
Contributor Author

@Marc-Andre-Rivet Marc-Andre-Rivet Nov 4, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

props override state, always - the component essentially doesn't work at all anymore - I haven't looked deeply into it but the wrapper addition seems to affect the lifecycle of the wrapped component. I'm unwilling to make the wrapper pure as the real component isn't.

Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#692 (comment) is just for information, no change needed. This looks great. 💃

@Marc-Andre-Rivet
Copy link
Contributor Author

Will wait on #693 for the test fix as this alone will trigger an incorrect baseline b/c of November calendar 👽

@Marc-Andre-Rivet Marc-Andre-Rivet merged commit afcab48 into dev Nov 4, 2019
@Marc-Andre-Rivet Marc-Andre-Rivet deleted the async-components branch November 4, 2019 19:27
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants