API: Mimic PureComponent behavior for Consumer children #6412
Conversation
This fixes a bug in react-native-server where `SET_CURRENT_STORY` was getting spammed over and over for the same story.
|
This pull request is automatically deployed with Now. Latest deployment for this branch: https://monorepo-git-react-native-server-rerenders.storybook.now.sh |
| this.prevData && | ||
| shallowEqualObjects(data, this.prevData) | ||
| ) { | ||
| return this.prevChildren; |
There was a problem hiding this comment.
So at first I thought this was a great idea, but later I realized this mean ALL descendant components would have to be pure for this to work as expected.
I removed the pure prop, since this is just too big of a assumption to make.
WDYT?
There was a problem hiding this comment.
later I realized this mean ALL descendant components would have to be pure for this to work as expected.
Why is that the case? Or, what's preventing descendants to not be pure?
There was a problem hiding this comment.
@benoitdion if you return prevChildren from render, any updates to the new children won't be part of the newly created XML tree.
There was a problem hiding this comment.
I'm still not sure I fully understand the problem. I added 7b2539a to show an example of a descendant updating when the provider data doesn't change.
When the provider data changes, shallowEqualObjects should return false and thechildren function will get called again.
| @@ -76,8 +62,7 @@ export default class ReactProvider extends Provider { | |||
| handleAPI(api) { | |||
| addons.loadAddons(api); | |||
|
|
|||
| api.onStory((kind, story) => { | |||
| this.selection = { kind, story }; | |||
| api.onStory(() => { | |||
There was a problem hiding this comment.
api.onStory should at some point be changed to api.on
I understand onStory is kinda convenient, but I don't want to add a method for every event.
Codecov Report
@@ Coverage Diff @@
## next #6412 +/- ##
==========================================
+ Coverage 40.37% 40.44% +0.06%
==========================================
Files 639 637 -2
Lines 8785 8761 -24
Branches 643 641 -2
==========================================
- Hits 3547 3543 -4
+ Misses 5139 5120 -19
+ Partials 99 98 -1
Continue to review full report at Codecov.
|
|
I'll fix the merge conflict & linting today and merge |
|
I'll take of it early next week if that's ok. There are a couple of things I wanted to clean up before merging. |
…onsumer is pure" This reverts commit 7b2539a.
benoitdion
force-pushed
the
react-native/server-rerenders
branch
from
fefada7 to
44156f5
Compare
shilman
changed the title
shilman
changed the title
This fixes a bug in react-native-server where
SET_CURRENT_STORYwas getting spammed over and over for the same story.