Upgrade storybook to v6 #47777

maddiedierker · 2022-08-23T17:29:26Z

Upgrades all packages within the @storybook/ scope to latest.

As part of this upgrade, Storybook has a new API (e.g., the storiesOf format we are using is deprecated and no longer works in this version of Storybook; see "storiesOf (Legacy) API" for details). This means we need to refactor all stories to use the new CSF format. Storybook has an automated tool for this, but it worked on 0/226 of our Storybook files 😢 so we have to do this refactor manually. I estimate the average Storybook file will take 5-10 minutes to refactor.

Implications of this upgrade:

All Storybook files need to be migrated to the new format. I've migrated Meter.story.jsx and Notification.story.jsx in this PR as examples.
Any stories using the deprecated storiesOf API will be ignored by Storybook. If you run yarn storybook and your stories don't show up, that's probably why! Our unit test harness still supports the old API, so we do not lose any test coverage during this migration.
All new stories must use the new format.
We can now use Storybook documentation and new addons because our API isn't years out of date!
We can upgrade to Webpack 5 without breaking Storybook.

I've added a README in apps/.storybook/ that outlines how Storybook should be used and documents this migration. This migration is also tracked in Storybook Files to Refactor (internal Google Doc) and PP-186.

Resources:

Component Story Format (CSF)
"How to write stories"
"storiesOf (Legacy) API"
Storybook Files to Refactor (tracks status of this migration)
PP-186 (tracks status of this migration in Jira)

… config

This reverts commit ce74acc.

maddiedierker · 2022-08-23T23:11:21Z

apps/src/templates/InlineMarkdown.jsx

@@ -27,7 +27,7 @@ export default class InlineMarkdown extends React.Component {
  };

  render() {
-    const rendered = markdownToReact.processSync(this.props.markdown).contents;
+    const rendered = markdownToReact.processSync(this.props.markdown).result;


note about this change: the unified library was upgraded with storybook, and the structure of the object returned from processSync changed with that upgrade. there are unit/UI tests that cover this and i manually tested components to make sure nothing changed

maddiedierker · 2022-08-24T17:20:16Z

apps/.storybook/main.js

+  //   }
+  // },
+  features: {
+    babelModeV7: true


details about this feature: https://storybook.js.org/docs/react/configure/babel#v7-mode

maddiedierker · 2022-08-24T17:21:28Z

apps/README.md

@@ -30,19 +30,24 @@ yarn start
 ### Seeing your development version of Apps in Dashboard

 1. To make your changes show up in dashboard, do the following after the first time you build apps:
-  - Set `use_my_apps: true` to your locals.yml config file.


my editor reformatted some extra stuff in this file when i made changes, sorry 🤷🏻‍♀️

bencodeorg

A couple notes/questions, but excited about this!

bencodeorg · 2022-08-24T20:50:21Z

apps/src/templates/Notification.story.jsx

+  buttonLink: 'to a new page',
+  dismissible: false
+};
+InfoCallToActionButton.storyName = 'Information - call-to-action button';


When do you add a storyName vs not?

i only renamed stories when i thought it made more sense for organization/readability purposes. in this file, i thought

Information - call-to-action-button Information - two buttons and a link Information

was more readable in the storybook panel than

Information Call To Action Button Information Two Buttons And A Link Information

bencodeorg · 2022-08-24T20:53:00Z

apps/src/templates/Meter.story.jsx

+// STORIES
+//
+
+export const HalfFull = Template.bind({});


Found this in docs if anyone is curious what's going on here:

Template.bind({}) is a standard JavaScript technique for making a copy of a function. We copy the Template so each exported story can set its own properties on it.

bencodeorg · 2022-08-24T20:56:48Z

apps/package.json

@@ -31,7 +31,7 @@
    "start:craft": "DEV=1 grunt dev --app=craft --watch-notify;",
    "test-audio": "echo \"Open your browser to http://127.0.0.1:8080/test/audio/audio_test.html\" && http-server .",
    "prestorybook": "curl -o build/package/css/application.css http://localhost-studio.code.org:3000/assets/application.css || curl -o build/package/css/application.css https://code-dot-org.github.io/cdo-styleguide/css/application.css",
-    "storybook": "start-storybook -p 9001 -s build/package/,../dashboard/public,../,../pegasus/sites.v3/code.org/public",


These are now in apps/.storybook/main.js if anyone's curious where they ended up :)

bencodeorg · 2022-08-24T22:05:52Z

apps/.storybook/main.js

+  ],
+  addons: ['@storybook/addon-actions', '@storybook/addon-options'],
+  framework: '@storybook/react',
+  // TODO: Add webpack5 configuration below when we upgrade to webpack 5.


Is this written down somewhere so we don't forget? Or will Storybook fully not work without this line uncommented

it won't work! i have it written down personally since i'll be shipping the webpack upgrade (hopefully, unless something horrible comes up...) right after this is merged

bencodeorg · 2022-08-24T22:17:22Z

apps/webpack.js

-});
+// Customize webpack config for storybook.
+// @param {Object} sbConfig - Webpack configuration from storybook library.
+function storybookConfig(sbConfig) {


For my understanding, how does this work differently now? Did storybook not provide webpack config previously?

i think storybook always provided some webpack configuration and would try to merge theirs with ours (previously provided by apps/.storybook/webpack.config.js) and it caused bugs and annoying confusion. so now there is a config function that supplies the storybook config and you can pick-and-choose what you want or just overwrite it (although i think that might break some storybook features if you just ignored their config)

For my understanding, how did you decide where to include sbConfig when building the returned object? eg, no ...sbConfig.resolve.alias in resolve.alias?

mostly guess-and-check 😅 i knew which configurations we cared about from our original storybookConfig options, so i tested out which options we needed from storybook and which we could overwrite/exclude without breaking anything

bencodeorg · 2022-08-24T22:21:22Z

apps/test/storybook/util/testStorybook.js

+}
+
+function testStorybookFile(file) {
+  const {default: defaultExport, ...stories} = file;


What is defaultExport? A JS thing, Storybook?

just JS. you can access the default export from the file object this way, but you can't name a variable named "default," so i'm renaming it here

maddiedierker · 2022-08-25T21:21:09Z

@bencodeorg is this okay to merge, or do you want to take another look?

bencodeorg

Some of the config stuff is over my head, but LGTM generally

apps/test/storybook/util/testStorybook.js

bencodeorg · 2022-08-25T22:33:30Z

apps/webpack.js

-});
+// Customize webpack config for storybook.
+// @param {Object} sbConfig - Webpack configuration from storybook library.
+function storybookConfig(sbConfig) {


For my understanding, how did you decide where to include sbConfig when building the returned object? eg, no ...sbConfig.resolve.alias in resolve.alias?

Madelyn Kasula added 15 commits August 17, 2022 15:38

Upgrade @storybook/ packages

83346ee

Replace deprecated storybook configurations with main.js config

92cfba6

Refactor storybookConfig to function; incorporate Storybook's webpack…

aed47b1

… config

Add @cdo/storybook webpack alias

1a22774

Add redux decorator

1519de5

Safeguard against falsy rendered.props in tests

ce74acc

Update Meter stories to new format

1fe4b7e

Add comments for organization

0ee963d

Update Notification stories to new format

535d0b9

Support .js and .jsx for Storybook

f10dcc1

Update Storybook unit test harness

4a4c82d

Use staticDirs instead of deprecated -s option

30e1b8f

Merge branch 'staging' into upgrade-storybook

2239bda

Revert "Safeguard against falsy rendered.props in tests"

fdb6942

This reverts commit ce74acc.

Access 'result' instead of 'contents' after unified upgrade

3f09e55

maddiedierker force-pushed the upgrade-storybook branch from 7af6159 to 3f09e55 Compare August 23, 2022 23:09

maddiedierker commented Aug 23, 2022

View reviewed changes

Madelyn Kasula added 2 commits August 23, 2022 16:38

Create separate README for Storybook

67043be

Add CSF and migration details to README

f60c0a0

maddiedierker changed the title ~~Upgrade storybook~~ Upgrade storybook to v6 Aug 24, 2022

maddiedierker marked this pull request as ready for review August 24, 2022 17:08

maddiedierker commented Aug 24, 2022

View reviewed changes

maddiedierker requested review from jmkulwik and a team August 24, 2022 17:26

bencodeorg reviewed Aug 24, 2022

View reviewed changes

bencodeorg approved these changes Aug 25, 2022

View reviewed changes

Update testStorybook function documentation

ae48919

maddiedierker merged commit 4854a7a into staging Aug 29, 2022

maddiedierker deleted the upgrade-storybook branch August 29, 2022 16:47

This was referenced Aug 29, 2022

Revert "Upgrade storybook to v6" #47847

Merged

Reapply upgrade storybook to v6 #47881

Merged

mikeharv mentioned this pull request Sep 9, 2022

[Storybook] Migrate DisallowedHtmlWarningDialog to CSF #48036

Merged

8 tasks

fisher-alice mentioned this pull request Sep 9, 2022

Storybook migrations - ImportProjectDialog, SignInOrAgeDialog, UploadErrorDialog #48039

Merged

8 tasks

mikeharv mentioned this pull request Sep 12, 2022

[Storybook] Migrate ResetSuccessDialog.story.js to CSF #48056

Merged

8 tasks

fisher-alice mentioned this pull request Sep 14, 2022

Migrated ResourceLink story, deleted ProjectsGallery story #48100

Merged

8 tasks

This was referenced Sep 30, 2022

Storybook migrations - MakerLanding, ScrollButtons #48410

Closed

Migrate MakerLanding and ScrollButtons storybooks #48411

Merged

fisher-alice mentioned this pull request Nov 14, 2022

Migrate PendingButton story #49124

Merged

8 tasks

mikeharv mentioned this pull request Nov 14, 2022

Migrate Project stories #49126

Merged

8 tasks

fisher-alice mentioned this pull request Nov 14, 2022

Migrate PaginationWrapper story #49130

Merged

8 tasks

mikeharv mentioned this pull request Nov 14, 2022

Migrate LibraryTable story #49131

Merged

8 tasks

fisher-alice mentioned this pull request Nov 15, 2022

Migrate SearchBar storybook #49150

Merged

8 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade storybook to v6 #47777

Upgrade storybook to v6 #47777

maddiedierker commented Aug 23, 2022 •

edited

maddiedierker Aug 23, 2022

maddiedierker Aug 24, 2022

maddiedierker Aug 24, 2022

bencodeorg left a comment

bencodeorg Aug 24, 2022

maddiedierker Aug 24, 2022

bencodeorg Aug 24, 2022

bencodeorg Aug 24, 2022

bencodeorg Aug 24, 2022

maddiedierker Aug 24, 2022

bencodeorg Aug 24, 2022

maddiedierker Aug 24, 2022

bencodeorg Aug 25, 2022

maddiedierker Aug 25, 2022

bencodeorg Aug 24, 2022

maddiedierker Aug 24, 2022

maddiedierker commented Aug 25, 2022

bencodeorg left a comment

bencodeorg Aug 25, 2022

Upgrade storybook to v6 #47777

Upgrade storybook to v6 #47777

Conversation

maddiedierker commented Aug 23, 2022 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bencodeorg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maddiedierker commented Aug 25, 2022

bencodeorg left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

maddiedierker commented Aug 23, 2022 •

edited