Vue / Knobs - optimize on force render

[igor-dv]

Issue: #4516

What I did

I've added the optimization that should partially address the #4516 problem.

This solves:

In case the knobs are bounded to data() function:

storiesOf('Addon|Knobs', module)
  .addDecorator(withKnobs)
  .add('Simple', () => {
    const name = text('Name', 'John Doe');
    const age = number('Age', 44);

    return {
      components: { SimpleKnobExample },
      template: '<simple-knob-example :name="name" :age="age" />',
      data() {
        return { name, age };
      },
    };
  })

we can easily extract this data and replace it in the existing story

This doesn't solve

The case where the story returns only template:

storiesOf('Addon|Knobs', module)
  .addDecorator(withKnobs)
  .add('All knobs', () => {
    /*a ton of knobs here*/
    return {
      template: `
          <div style="border:2px dotted ${colour}; padding: 8px 22px; border-radius: 8px">
            <h1>My name is ${name},</h1>
            <h3>today is ${new Date(today).toLocaleDateString('en-US', dateOptions)}</h3>
            <p>${stockMessage}</p>
            <p>Also, I have:</p>
            <ul>
              ${items.map(item => `<li key=${item}>${item}</li>`).join('')}
            </ul>
            <p>${salutation}</p>
          </div>
        `,
    };

I don't know how to force update the existing story with this template without recreating the story 🤷‍♂️

CC, @y-nk @pksunkara, would like to hear your thoughts if there are other use-cases when we can just update the component and not destroy it.

[codecov]

Codecov Report

Merging #4773 into next will decrease coverage by 0.04%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##             next    #4773      +/-   ##
==========================================
- Coverage   35.47%   35.42%   -0.05%     
==========================================
  Files         561      561              
  Lines        6825     6834       +9     
  Branches      911      913       +2     
==========================================
  Hits         2421     2421              
- Misses       3928     3935       +7     
- Partials      476      478       +2

Impacted Files	Coverage Δ
app/react/src/client/preview/render.js	7.69% <ø> (ø)	⬆️
app/vue/src/client/preview/render.js	0% <0%> (ø)	⬆️

---

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 6005742...2ac83dd. Read the comment docs.

[igor-dv]

Example:

https://deploy-preview-4773--storybooks-vue.netlify.com/?selectedKind=Addon%7CKnobs&selectedStory=Simple&full=0&addons=1&stories=1&panelRight=0&addonPanel=storybooks%2Fstorybook-addon-knobs

[y-nk]

I don't believe this will be necessary since Vue as observers for all values of component internal states ; but that won't kill performances either for most of the case, so it's ok to leave it if you feel it's safer.

[y-nk]

If you are to bind knobs to the internal state, this works completely.

That said, examples of addons-knob for React shows that knobs should be bound to component's properties, not internal state. From the official addons-knob repo, you can easily link back to this example (here's a sample of it) :

stories.add('with some text', () => {
  let content = text('Content', 'This is the content');
  content = content.replace(/\n/g, '<br />');

  return (
    <div
      dangerouslySetInnerHTML={{__html: content}}
    />
  );
});

You can see in this story that the content knob is bound to a property of the returned div. If that div would be a react component, it would still be a prop : storybook wouldn't try to call component.setState({ dangerouslySetInnerHTML: content }) to refresh it, but the Vue version of storybook does. That's what mentioned in my comment : for Vue, render calls are not binding to prop, rather to the internal state of the story.

In the example you provided, you bind the knobs to data, which would actually be the same as binding it to component.state in React, but it should be actually written this way :

storiesOf('Addon|Knobs', module)
  .addDecorator(withKnobs)
  .add('Simple', () => {
    const name = text('Name', 'John Doe');
    const age = number('Age', 44);

    return {
      components: { SimpleKnobExample },
+      props: ['name', 'age'],
      template: '<simple-knob-example :name="name" :age="age" />',
-      data() {
-        return { name, age };
-      },
    };
  })

Then, to make it work, you only need to render the story with component.data() bound the story's properties like that : h(component, { props: component.data() })

Here's a working example (fully in Vue, but only #L57-62 matters) : http://jsfiddle.net/bn8tvpdL/

It will probably fix the 2nd use case since nothing will be changed and Vue's diffing will do the magic.

[y-nk]

with permission of @igor-dv , i proposed to solve this.

What I did

First i went back to defining the most simple usecase, as it would be in React. Because Vue and React are different, we can't really translate, but what we know is that stories are returning components, and knobs are inserted into properties of those components (for the most part).

Then, I've been recoding the render function. The main idea is to keep the root container alive, and use an outside variable to pass props to the component's story. In order not to break HMR, we still need to recreate a new story everytime it refreshes, but only not if forceRender is true, which seems to link to when knobs are modified (it's undefined when you save the file and HMR reloads it).

Additional notes

It's perfectly ok that the 2nd example destroys/recreate the story every single time. The reason why is normal, is that the template is a full string and is interpreted as such. There's no deep diffing possible within a component when the template string is given in such a way because Vue tries to optimize the template rendering and detects a purely static string.

Such an example should not exist because it doesn't translate from React to Vue paradigm. Knobs should remain properties and derivated values should be computed values.

[igor-dv]

I've fixed a few minor things. I can't approve since it's my PR

@ndelangen @pksunkara