Support client-only tasks globally, and per-story. #40

nickpresta · 2020-05-27T23:23:38Z

Overview

For projects that don't server render their components, the server-related data is not helpful.

Clients can opt-in to client-only performance tests by adding:

import { addParameters } from '@storybook/client-api';
import { AllowedGroup } from 'storybook-addon-performance';

addParameters({
  performance: {
    allowedGroups: [AllowedGroup.Client],
  },
});

to their .storybook/preview.js file.

On a per-story basis, the allowedGroups parameter can be set alongside
the interactions parameter:

select.story = {
  name: 'React select',
  parameters: {
    performance: {
      interactions: interactionTasks,
      allowedGroups: [AllowedGroup.Client],
    },
  },
};

This change removes all service-side tasks, as well as the "hydrate"
task in from the client-side group.

Screenshots

Before

After

Documentation

I'm not sure how the maintainers want this functionality documented. The easiest place would be a small section in the README. There is also an examples directory that may be a good place to add a small example. Let me know 😄

atlassian-cla-bot · 2020-05-27T23:23:40Z

Hooray! All contributors have signed the CLA.

alexreardon · 2020-05-28T00:34:18Z

Brilliant!

alexreardon · 2020-05-28T00:36:03Z

Im thinking out loud here:

Could we change it from clientOnly: boolean to groups: []('client' | 'server')

alexreardon · 2020-05-28T00:36:46Z

That way, if somebody only wanted server, they could do that too. It would also mean if we have new groups in the future, we wouldn't need to change the API

nickpresta · 2020-05-28T01:15:29Z

@alexreardon That sounds good. I actually started out that way and then thought I was overengineering 😆

I'll give it another go tomorrow (I'm ET timezone) and push a new commit with the changes.

Thanks!

alexreardon · 2020-05-28T02:54:43Z

given that it is an allow list, perhaps it should be called allowed or allowedGroups

nickpresta · 2020-05-28T21:53:01Z

@alexreardon Hey again. I've pushed a commit which changes clientOnly to allowedGroups.

One thing I had to change to fix a bug, which was present in my initial commit, is the getResult function. I was running into an issue during the following sequence of events:

Globally configure Storybook to have all allowedGroups (client and server).
Override a specific story to only have the client group.
Run the tasks in the client-specific story (added as Only client in the landing page example)
Switch to any other story (these all have the server group configured).

An error occurs because, as far as I could tell, state.context is stale. The value is still pointing to the client story data, and the results don't contain the server group that the new panel is expecting. When getResult tries to find a result for the server group but can't, it throws. You can recreate this error by changing the line to throw instead of return null if you want to debug a but yourself.

As a "fix", I've changed the function to return null instead, and I handle null in the TaskGroupPanel component (to render nothing). Everything still seems to work as expected, so I'm not sure if this was meant to throw because something is actually wrong, or if it seemed "impossible" to happen as we were guarding against it. I do see a message that mentions wanting to use TS's invariant functionality, and no tests failed as a direct result, so I assumed we were trying to guard against the impossible.

I'm still missing documentation and I'm more than happy to address that in another commit to this PR. 😄

nickpresta · 2020-05-28T22:00:47Z

src/task-runner/get-element.tsx

-export default function getElement(
-  getNode: () => React.ReactNode,
-): () => React.ReactElement {
+export default function getElement(getNode: () => React.ReactNode): () => React.ReactElement {


This change was from running prettier on the project.

.storybook/preview.js

src/types.ts

alexreardon · 2020-06-22T23:10:35Z

This would be cool to see! Can we help push it along?

nickpresta · 2020-06-24T18:28:43Z

@alexreardon thanks for checking in with me. I've made the changes you've requested. Let me know if there's anything else I can do to help get this merged.

EDIT I took a crack at adding documentation. I've added a separate commit (d36f31c) that includes it in the README.

src/panel/panel.tsx

alexreardon · 2020-06-25T23:30:17Z

This is looking great! Thanks heaps for your contribution

.storybook/preview.js

For projects that don't server render their components, the server-related data is not helpful. Clients can opt-in to client-only performance tests by adding: import { addParameters } from '@storybook/client-api'; addParameters({ performance: { clientOnly: true, }, }); to their .storybook/preview.js file. On a per-story basis, the `clientOnly` parameter can be set alongside the `interactions` parameter: select.story = { name: 'React select', parameters: { performance: { interactions: interactionTasks, clientOnly: true, }, }, }; This change removes all service-sid tasks, as well as the "hydrate" task in from the client-side tasks.

Removes the `defaultAllowedGroups` from the Storybook configuration to reduce confusion. This default is applied automatically, anyways.

These functions were made equivalent after the change to support a nullable result. We don't need to keep both functions.

nickpresta · 2020-07-07T23:30:51Z

@alexreardon I've resolved all merge conflicts and I'm up-to-date with the master branch. Feel free to merge the PR (I'm not sure I have permission) if it looks good.

I'm not sure how you want to handle changelogs/version bumps but I'm happy to take a crack at it if you like.

Thanks again for all your help.

We're not server-side rendering components and I suspect that this is triggering exceptions when the storybook-addon-performance tries to run server tasks. Luckily there is a workaround, introduced in atlassian-labs/storybook-addon-performance#40. chore/storybook-performance-addon

We're not server-side rendering components and I suspect that this is triggering exceptions when the storybook-addon-performance tries to run server tasks. Luckily there is a workaround, introduced in atlassian-labs/storybook-addon-performance#40. chore/storybook-performance-addon diff --git a/.storybook/preview.js b/.storybook/preview.js index c05206c8..c47272ba 100644 --- a/.storybook/preview.js +++ b/.storybook/preview.js @@ -79,6 +79,12 @@ const withTrackingAction = (Story) => ( </TrackingRoot> ); +addParameters({ + performance: { + allowedGroups: ['client'], + }, +}); + export const decorators = [ withThemeProvider, // withStoryStyles, chore/storybook-performance-addon chore/storybook-performance-addon

* chore: add performance addon to Storybook This will enable tracking basic performance metrics when working on Circuit components. See: https://github.com/atlassian-labs/storybook-addon-performance chore/storybook-performance-addon * test(components): measure popover interaction performance with storybook-addon-performance chore/storybook-performance-addon * fix: disable SSR performance checks We're not server-side rendering components and I suspect that this is triggering exceptions when the storybook-addon-performance tries to run server tasks. Luckily there is a workaround, introduced in atlassian-labs/storybook-addon-performance#40. chore/storybook-performance-addon * test(components): measure checkbox interaction performance We're not server-side rendering components and I suspect that this is triggering exceptions when the storybook-addon-performance tries to run server tasks. Luckily there is a workaround, introduced in atlassian-labs/storybook-addon-performance#40. chore/storybook-performance-addon * fix: run SSR tasks in development builds chore/storybook-performance-addon * fix: use document.querySelector and userEvent 1. document.querySelector should be used to select elements in interaction tasks, because getBy* breaks when using the storybook-addon-performance with several copies (it throws an exception if several elements match). getAllBy* also doesn't work because it returns an array of elements. The addon's docs use querySelector for this so we'll do the same 2. we will use userEvent instead of fireEvent chore/storybook-performance-addon * fix: remove findByText timeout It will time out after a default 1s, this is long enough for this component. chore/storybook-performance-addon

nickpresta force-pushed the configure-client-only-tasks branch 4 times, most recently from e6b8516 to 7a8350c Compare May 28, 2020 00:00

nickpresta force-pushed the configure-client-only-tasks branch 2 times, most recently from 713a1aa to 05c4567 Compare May 28, 2020 21:50

nickpresta force-pushed the configure-client-only-tasks branch from 05c4567 to d8e0c14 Compare May 28, 2020 21:59

nickpresta commented May 28, 2020

View reviewed changes

alexreardon requested changes May 29, 2020

View reviewed changes

.storybook/preview.js Outdated Show resolved Hide resolved

src/types.ts Outdated Show resolved Hide resolved

nickpresta force-pushed the configure-client-only-tasks branch 2 times, most recently from 914a99b to bbb2145 Compare June 24, 2020 18:27

alexreardon reviewed Jun 25, 2020

View reviewed changes

src/panel/panel.tsx Outdated Show resolved Hide resolved

alexreardon approved these changes Jun 25, 2020

View reviewed changes

.storybook/preview.js Outdated Show resolved Hide resolved

alexreardon approved these changes Jun 28, 2020

View reviewed changes

nickpresta added 6 commits July 7, 2020 19:29

Switch to allowedGroups instead of clientOnly.

71936c1

Switch from enum to string literal type.

56f6ac3

Removes the `defaultAllowedGroups` from the Storybook configuration to reduce confusion. This default is applied automatically, anyways.

Add documentation to README.

91ece35

Remove getResult and use findResult.

07fc750

These functions were made equivalent after the change to support a nullable result. We don't need to keep both functions.

Remove unneeded import.

d766fbd

nickpresta force-pushed the configure-client-only-tasks branch from 9263b49 to d766fbd Compare July 7, 2020 23:29

alexreardon merged commit 1dfde66 into atlassian-labs:master Aug 7, 2020

robinmetral mentioned this pull request Nov 13, 2020

Hydrate is run when only client tasks are allowed #57

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support client-only tasks globally, and per-story. #40

Support client-only tasks globally, and per-story. #40

nickpresta commented May 27, 2020 •

edited

Loading

atlassian-cla-bot bot commented May 27, 2020 •

edited

Loading

alexreardon commented May 28, 2020

alexreardon commented May 28, 2020

alexreardon commented May 28, 2020

nickpresta commented May 28, 2020

alexreardon commented May 28, 2020

nickpresta commented May 28, 2020 •

edited

Loading

nickpresta May 28, 2020

alexreardon commented Jun 22, 2020

nickpresta commented Jun 24, 2020 •

edited

Loading

alexreardon commented Jun 25, 2020

nickpresta commented Jul 7, 2020

Support client-only tasks globally, and per-story. #40

Support client-only tasks globally, and per-story. #40

Conversation

nickpresta commented May 27, 2020 • edited Loading

Overview

Screenshots

Before

After

Documentation

atlassian-cla-bot bot commented May 27, 2020 • edited Loading

alexreardon commented May 28, 2020

alexreardon commented May 28, 2020

alexreardon commented May 28, 2020

nickpresta commented May 28, 2020

alexreardon commented May 28, 2020

nickpresta commented May 28, 2020 • edited Loading

nickpresta May 28, 2020

Choose a reason for hiding this comment

alexreardon commented Jun 22, 2020

nickpresta commented Jun 24, 2020 • edited Loading

alexreardon commented Jun 25, 2020

nickpresta commented Jul 7, 2020

nickpresta commented May 27, 2020 •

edited

Loading

atlassian-cla-bot bot commented May 27, 2020 •

edited

Loading

nickpresta commented May 28, 2020 •

edited

Loading

nickpresta commented Jun 24, 2020 •

edited

Loading