Action addons is slow to log events

[tmeasday]

Issue details

A callback that logs the passed event directly to an action can incur a noticeable delay before logging.

For instance, in cra-kitchen-sink if you change the story to:

storiesOf('Button', module)
  .add('with text', () => (
    <Button onClick={e => action('clicked')(e,e,e,e,e)}>
      {setOptions({ selectedAddonPanel: 'storybook/actions/actions-panel' })}
      Hello Button
    </Button>

It takes probably 2-3s for the action to be logged in the actions pane, on my iMac.

Notes:

1. It seems to depend on the complexity of your app exactly how slow it is. In the kitchen sink app (a CRA app) you need (say) 5 events to see it clearly. I can't make it very noticeable in the official-storybook app (which is raw react), even with a lot of events. In my actual app it's noticeable with a single event.

2. React <=15.6.1 makes this worse as it passes essentially 2 events by default.

3. The time is actually in the manager side of things; serializing the events is fairly quick.

Please specify which version of Storybook and optionally any affected addons that you're running

• @storybook/react@3.3.2
• @storybook/addon-actions@3.3.2

[tmeasday]

I think a short term suggestion here would be to just treat native events / react versions / etc (e.g. instanceof Event) specially and not serialize them/deserialize them at all. I don't think inspecting a MouseEvent really tells me anything..?

What do you think @rhalff?

[rhalff]

@tmeasday I think that will be the best solution for now.

The original behavior was to just return the string [SyntheticEvent] by checking whether the preventDefault property existed on the object.

I've been rather occupied with this bug, but did not come to a solution yet.

I found there is also a rather nasty memory leak. There is an obvious one where all the actions are kept within state and every log increases the list. The problem is the clear button doesn't cause these items to be freed.

One of the objects keeping references is FiberNode within memoizedState and memoizedProps, but that's probably not the root cause.

[tmeasday]

Would you like me to submit a PR reverting this behavior for events?

[rhalff]

@tmeasday Added the changes I did in this pull request. I haven't created any tests for it yet though.

Let me know if you think this solution is sufficient.

[tmeasday]

Definitely sufficient. My only additional suggestion would be to treat that type specially and log it as such (i.e. so it doesn't look like a string). I'm not sure if react-inspector would support that though?

[rhalff]

I can also take a different approach and set the depth for events to 0, which will only load the first level of properties.

Which would look something like this (object names still missing):

You can get an idea on how performant that would be by changing the default in decycle to 0 (in master):

https://github.com/storybooks/storybook/blob/master/addons/actions/src/lib/decycle.js#L9

The depth was meant to be configurable and could be used for this case.

[tmeasday]

I can also take a different approach and set the depth for events to 0, which will only load the first level of properties.

I think this would be a great improvement to the current workaround!

[Hypnosphi]

Fix released as 3.3.4

[Ineigo]

For storybook/Vue, not fixed... Vue events are still being serialized.

trigger:

<-- ... -->
<form @submit="onSeforeSubmit">
        <slot />
        <button :class="stNames['submit']">{{ buttonTitle }}</button>
</form>
<-- ... -->

/* ... */
        onSeforeSubmit(e) {
            this.$emit('submit', e);
        }
/* ... */

story:

const stories = [
    {
        name: 'По умолчанию',
        component: () => ({
            components: { filtersList },
            methods: {
                action: action('submit'),
            },
            template:
`<filters-list
    @submit="action"
>
</filters-list>`,
        }),
    },
];
const storyInstance = storiesOf('Filters/List', module)
    .addDecorator(VueInfoAddon)
    .addDecorator(withKnobs);

for (let story of stories) {
    storyInstance.add(story.name, story.component);
}

result

Please fix it!

[Hypnosphi]

cc @rhalff

[dan-kez]

I'm seeing this issue in react as well.

[rhalff]

I've added a pull request which limits the depth of non plain objects being logged.

"plain" objects will still be logged until a depth of 10, which could be increased I guess.

While everything is being serialized and deserialized again the current implementation has it's limitations.

e.g. chrome devtools loads it's information lazily and is thus not limited to any depths.

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[Hypnosphi]

Released as 3.4.2

[ghost]

I still see this issue. Reducing depth worked.

[liamross]

Noticing a major performance issue with actions specifically on non-chrome browsers. In some cases, it freezes the entire window for multiple seconds (when mapping actions to sub-items in a grid). But even root level actions on a simple button cause 1 second of freezing.

[henrycity]

I had the same experience, Chrome works fine but there is a lag of rendering on other browsers.

[shilman]

We should fix this in 6.1 @ndelangen

[ndelangen]

@shilman I'd assume a LOT of the overhead was in telejson which got a huge improvement in perf lately, has this issue improved recently with the 6.0.0 release?

[stale]

Hi everyone! Seems like there hasn't been much going on in this issue lately. If there are still questions, comments, or bugs, please feel free to continue the discussion. Unfortunately, we don't have time to get to every issue. We are always open to contributions so please send us a pull request if you would like to help. Inactive issues will be closed after 30 days. Thanks!

[stale]

Hey there, it's me again! I am going close this issue to help our maintainers focus on the current development roadmap instead. If the issue mentioned is still a concern, please open a new ticket and mention this old one. Cheers and thanks for using Storybook!