Refactor the Series and Collection data flow #302
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 30 commits
May 5, 2019 17:18
Simplify a lot of the logic in DataProvider by inverting the source of truth for Series and Collections. This change moves them from immutable props on DataProvider to separate children components which report their configuration (through the DataProvider's Context) into DataProvider's state. This allows DataProvider to be responsible for watching a lot fewer props and move the logic out to the separate components. Additionally, this ensures that the properties for Series and Collection are centrally-documented, due to them using TypeScript for their implementations. This also comes with some nice syntactic sugar around the grouping of Series into Collections (however, this is not required). Another benefit of this cleaned system is that the system can guarantee that Series and Collection objects which leave DataProvider are fully-populated. This will allow us to clean up default props littered throughout the codebase, for things like strokeWidth and color. Finally, in order to ease this massively-breaking API change, a thin backwards-compatibility shim has been introduced into DataProvider. However, this is not intended to be permanent and should be removed before the 0.5.0 stable version is released.
custom accessor support is coming
This required moving the calculated domains into the state
This leads to a crash if there aren't any series at all. Will fix later.
added 5 commits
May 5, 2019 17:39
Because hey what's another commit on this branch, right?
valgussev
approved these changes
May 6, 2019
added 2 commits
May 6, 2019 14:25
|
This is ready for review! Before merging, I'll squash all of these commits into something pretty. |
added 2 commits
May 6, 2019 14:56
It should be shared between Series and Collection
evancharlton
force-pushed
the
series-data-flow
branch
from
6045f7a
to
fe7a22f
Compare
added 5 commits
May 6, 2019 15:25
These are already covered by the Scatterplot stories but hey here we go
This required an update to Scaler where we need to handle the series props changing because they come in late.
|
[pr-server] |
|
There is one small known quirk with this PR -- I don't know if it's a regression or not. If you set I'm tempted to merge this anyway because it's very obscure, and the proper fix is much deeper -- and this PR is sprawling enough as it is. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Simplify a lot of the logic in DataProvider by inverting the source of
truth for Series and Collections. This change moves them from immutable
props on DataProvider to separate children components which report their
configuration (through the DataProvider's Context) into DataProvider's
state. This allows DataProvider to be responsible for watching a lot
fewer props and move the logic out to the separate components.
Additionally, this ensures that the properties for Series and Collection
are centrally-documented, due to them using TypeScript for their
implementations. This also comes with some nice syntactic sugar around
the grouping of Series into Collections (however, this is not required).
Another benefit of this cleaned system is that the system can guarantee
that Series and Collection objects which leave DataProvider are
fully-populated. This will allow us to clean up default props littered
throughout the codebase, for things like strokeWidth and color.
Finally, in order to ease this massively-breaking API change, a thin
backwards-compatibility shim has been introduced into DataProvider.
However, this is not intended to be permanent and should be removed
before the 0.5.0 stable version is released.