Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Upgrade Storybook to v6.1.3 #897

Merged
merged 40 commits into from Nov 24, 2020

Conversation

anicholls
Copy link
Contributor

@anicholls anicholls commented Nov 3, 2020

Summary

To make use of Storybook Composition, we need to be on Storybook v6. This PR upgrades CK to use the latest version. This includes:

  • Ditching the deprecated style of storybook config (config.js, addons.js, etc.) for the new style (main.js, manager.js, preview.js).
  • TS is now built in by default, so our webpack/babel config has been simplified and now lives in main.js
  • The | hierarchy separator is gone, and top level sections are just pulled from the string before the first /
  • Due to this deprecation, I had to change the organizational structure for all of our theming stories (and had to combine several others into a single story file). You can no longer have multiple default exports with the same name in different files (so all stories in the same storybook folder need to come from the same file). Previously, we usually had one file for visual testing, one file for theming visual testing, both placing a story in the same folder. Unfortunately, if you specify parameters for one exported story it applies to the entire folder of stories, so it was impossible to put themed and unthemed stories in the same file. Since they have to be in separate files, the themed stories had to be put under a Theming folder in storybook. I should probably open an issue for the parameter issue...
  • I simplified our addons to use addon-essentials, which includes Actions (disabled due to poor performance), Backgrounds, Controls, Docs, Viewport, and Toolbars
  • There are also several gotchas that I had to address (see below)

Additional References

Here are some gotchas that I ran into:

  • Due to this issue, all mdx files have to be prefixed with .stories. For example card.stories.mdx will work, but card.mdx or stories.mdx will not.

  • Due to this issue, static class/function members must be defined after the class/function is defined, but before it is exported. If you define it in both places, this will avoid any Typescript errors. This does not apply for functional components since their static members are always defined after instantiation. For example:

    class MyComponent extends React.Component<RadioGroupProps> {
      static Foo = Foo;
      // ...
    }
    MyComponent.Foo = Foo;
    
    export default Foo;
  • Due to this issue and this issue, if you have combined exports for components and types, you need to use the plugin @babel/plugin-transform-modules-commonjs. You may still be required to separate some exports. For example:

    export { MyComponent, MyComponentProps } from "./lib/MyComponent";
    
    // becomes
    
    export { MyComponent } from "./lib/MyComponent";
    export type { MyComponentProps } from "./lib/MyComponent";
  • Since we had to combine all stories of the same folder structure into the same file, we now have to conditionally add parameters to some stories but not others. Be careful doing this if the story is a simple re-assignment. Since they have the same functional reference, the parameter will apply to both. Make sure you use a function that returns the original story instead.

    export const ButtonThemedStates = ButtonStates;
    ButtonThemedStates.parameters = {
      canvasProviderDecorator: {
        theme: customColorTheme,
      },
    };
    
    // Instead, do this:
    
    export const ButtonThemedStates = () => <ButtonStates />;
    ButtonThemedStates.parameters = {
      canvasProviderDecorator: {
        theme: customColorTheme,
      },
    };
  • Due to this issue, JSX elements now need a line break between them. Unfortunately divs or React Fragments do not work with Storybook's Meta component, so the only solution I've found is adding markdown between JSX tags, or saving the file without formatting.

@anicholls anicholls changed the title Upgrade storybook to v6.0.28 feat: Upgrade Storybook to v6.0.28 Nov 3, 2020
@cypress
Copy link

cypress bot commented Nov 3, 2020



Test summary

560 0 1 0


Run details

Project canvas-kit
Status Passed
Commit 2298f60 ℹ️
Started Nov 24, 2020 6:49 PM
Ended Nov 24, 2020 6:55 PM
Duration 05:04 💡
OS Linux Ubuntu Linux - 18.04
Browser Electron 80

View run in Cypress Dashboard ➡️


This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@anicholls anicholls added the ready for review Code is ready for review label Nov 4, 2020
@lychyi lychyi added this to In Progress in Current Sprint (7/20 - 8/9) via automation Nov 16, 2020
@lychyi lychyi moved this from In Progress to Needs Review in Current Sprint (7/20 - 8/9) Nov 16, 2020
.github/workflows/ci.yml Outdated Show resolved Hide resolved
.storybook/manager.js Outdated Show resolved Hide resolved
@anicholls anicholls changed the title feat: Upgrade Storybook to v6.0.28 feat: Upgrade Storybook to v6.1.3 Nov 23, 2020
@@ -48,6 +50,7 @@
"babel-loader": "^8.1.0",
"babel-plugin-emotion": "^10.0.29",
"babel-plugin-transform-es2015-modules-commonjs": "^6.26.2",
"caniuse-lite": "^1.0.30001154",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's with these version numbers? Looks like they are already on 1.0.30001161

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

haha right? We were getting console warnings and the suggested fix was adding this dep. This was the latest when I added it, but I can upgrade again if you want

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No, just saw it and was wondering which prompted me to look it up. They have about 400 versions in the 1.0.3x range

@NicholasBoll NicholasBoll added the dependencies Pull requests that update a dependency file label Nov 24, 2020
@anicholls anicholls merged commit 504391a into Workday:master Nov 24, 2020
Current Sprint (7/20 - 8/9) automation moved this from Needs Review to Done Nov 24, 2020
@anicholls anicholls deleted the storybook-upgrade branch November 24, 2020 19:04
@jaclynjessup jaclynjessup removed ready for review Code is ready for review sprint commitment labels Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

5 participants