Use store revisions to ensure that stories re-render on HMR.

[tmeasday]

For #2587

Issue:

To avoid unnecessarily re-rendering stories we don't re-render if the story name+kind haven't changed.

However on HMR, this leads to not rerendering / breakage, as in this case the name/kind haven't changed, but the store's implementation itself changed.

What I did

I added the concept of a revision to the storystore. This implies that the same name+kind can be different as the store itself has changed. I got the idea from styleguidist which I think saw something similar in when I was reading their source ;)

Moving forward, if we want dynamic stores (yes please!) we may need to generalise the concept of revision to meaning more just "incrementing on each HMR".

How to test

Try changing stories implementations in the kitchen sink.

Is this testable with jest or storyshots? No

Does this need a new example in the kitchen sink apps? No

Does this need an update to the documentation? No

We should ensure this is merged into #2241

[codecov]

Codecov Report

Merging #2588 into master will decrease coverage by 0.01%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #2588      +/-   ##
==========================================
- Coverage   32.68%   32.67%   -0.02%     
==========================================
  Files         398      398              
  Lines        8838     8842       +4     
  Branches      945      943       -2     
==========================================
  Hits         2889     2889              
+ Misses       5305     5297       -8     
- Partials      644      656      +12

Impacted Files	Coverage Δ
app/react/src/client/preview/render.js	0% <ø> (ø)	⬆️
app/react/src/client/preview/client_api.js	83.72% <0%> (-2%)	⬇️
app/react/src/client/preview/story_store.js	0% <0%> (ø)	⬆️
lib/ui/src/modules/api/actions/api.js	51.85% <0%> (ø)	⬆️
lib/codemod/src/transforms/move-buildin-addons.js	46.87% <0%> (ø)	⬆️
addons/info/src/components/PropVal.js	82.82% <0%> (ø)	⬆️
...t-native/src/preview/components/StoryView/index.js	0% <0%> (ø)	⬆️
lib/ui/src/modules/ui/components/search_box.js	36.36% <0%> (ø)	⬆️
lib/codemod/src/transforms/update-addon-info.js	50% <0%> (ø)	⬆️
...react-native/src/manager/components/PreviewHelp.js	0% <0%> (ø)	⬆️
... and 42 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d455f97...cc25d2b. Read the comment docs.

[Hypnosphi]

Should we do the same for other apps?

[tmeasday]

They don’t have this immediate problem. We’ll get this revision code in all apps when we land the general story store PR

…

On Thu, 28 Dec 2017 at 12:26 pm, Filipp Riabchun ***@***.***> wrote: Should we do the same for other apps? — You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub <#2588 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAIFystmGsXL00rZMHgk6HRqihGWt1mTks5tEu47gaJpZM4RN6Q1> .

[Hypnosphi]

They don’t have this immediate problem

Why do they need the revisions then?

[danielduan]

this looks like a decent enough patch. also curious as to why the other two platforms don't need it.

[shilman]

LGTM. Have confirmed the bug and the fix on my machine, but don't understand the broader implications of this change (if any).

[tmeasday]

@Hypnosphi

They don’t have this immediate problem
Why do they need the revisions then?

Good question, perhaps we can discuss elsewhere (I'll make this into a PR against #2241 and we can go into detail there).

I think it is conceptually correct that we keep track in the storystore that the stories have changed; ie the output of Foo:bar (kind="Foo", name="bar") is different to what it was previously.

In this case, such a distinction is needed because our React code is trying to avoid unnecessary re-renders when the story choice hasn't changed. There was no way (until this patch) for the React code to know if the story implementation had changed.

@danielduan

also curious as to why the other two platforms don't need it

They don't need it immediately because they are not (AFAIK) making this optimization [I don't really know if they need to either]. This patch is a reaction to a bug introduced when I made this optimization in React in #2503.