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.

Sign up for GitHub

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

Storybook: Upgrade to v8.0.x #67574

Merged
merged 9 commits into from
Dec 11, 2024
+1,279 −1,627
Merged

Storybook: Upgrade to v8.0.x #67574

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
919accf
Bump dependencies
tyxla Dec 10, 2024
86fc08e
Patch storybook-source-link to import from @storybook/manager-api
tyxla Dec 4, 2024
b1211a8
Add webpack5 compiler babel addon and enable react-docgen-typescript
tyxla Dec 4, 2024
974f446
Use @storybook/icons in introduction docs
tyxla Dec 4, 2024
b11de11
Update storybook-source-link patch to use @storybook/icons
tyxla Dec 4, 2024
d3d72b0
Fix status icons
tyxla Dec 4, 2024
5c5078a
Fix status icon styles
tyxla Dec 4, 2024
146f748
Preview: ArgsTable => Controls
tyxla Dec 4, 2024
aab89cd
Add explicit action spies
tyxla Dec 10, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2,784 changes: 1,179 additions & 1,605 deletions package-lock.json
View file
Open in desktop

Large diffs are not rendered by default.

26 changes: 14 additions & 12 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
"@emotion/babel-plugin": "11.11.0",
"@emotion/jest": "11.7.1",
"@emotion/native": "11.0.0",
"@geometricpanda/storybook-addon-badges": "2.0.1",
"@geometricpanda/storybook-addon-badges": "2.0.5",
Copy link
Member Author

Choose a reason for hiding this comment

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

Needs to be bumped to support v8

"@octokit/rest": "16.26.0",
"@octokit/types": "6.34.0",
"@octokit/webhooks-types": "5.8.0",
Expand All @@ -42,16 +42,18 @@
"@react-native/babel-preset": "0.73.10",
"@react-native/metro-babel-transformer": "0.73.10",
"@react-native/metro-config": "0.73.4",
"@storybook/addon-a11y": "7.6.15",
"@storybook/addon-actions": "7.6.15",
"@storybook/addon-controls": "7.6.15",
"@storybook/addon-docs": "7.6.15",
"@storybook/addon-toolbars": "7.6.15",
"@storybook/addon-viewport": "7.6.15",
"@storybook/react": "7.6.15",
"@storybook/react-webpack5": "7.6.15",
"@storybook/source-loader": "7.6.15",
"@storybook/theming": "7.6.15",
"@storybook/addon-a11y": "8.0.10",
"@storybook/addon-actions": "8.0.10",
"@storybook/addon-controls": "8.0.10",
"@storybook/addon-docs": "8.0.10",
"@storybook/addon-toolbars": "8.0.10",
"@storybook/addon-viewport": "8.0.10",
"@storybook/addon-webpack5-compiler-babel": "3.0.3",
Copy link
Member Author

Choose a reason for hiding this comment

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

Now necessary in order to specifically use Babel, see https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#removed-babelcore-and-babel-loader-from-storybookbuilder-webpack5

"@storybook/react": "8.0.10",
"@storybook/react-webpack5": "8.0.10",
"@storybook/source-loader": "8.0.10",
"@storybook/test": "8.0.10",
Copy link
Member Author

Choose a reason for hiding this comment

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

Used to explicitly declare action handler spies, see: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#implicit-actions-can-not-be-used-during-rendering-for-example-in-the-play-function

"@storybook/theming": "8.0.10",
"@testing-library/jest-dom": "5.16.5",
"@testing-library/react": "14.3.0",
"@testing-library/react-native": "12.4.3",
Expand Down Expand Up @@ -156,7 +158,7 @@
"snapshot-diff": "0.10.0",
"source-map-loader": "3.0.0",
"sprintf-js": "1.1.1",
"storybook": "7.6.15",
"storybook": "8.0.10",
"storybook-source-link": "2.0.9",
"strip-json-comments": "5.0.0",
"style-loader": "3.2.1",
Expand Down
4 changes: 4 additions & 0 deletions packages/components/src/custom-select-control-v2/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import type { Meta, StoryFn } from '@storybook/react';
import { fn } from '@storybook/test';

/**
* WordPress dependencies
Expand Down Expand Up @@ -44,6 +45,9 @@ const meta: Meta< typeof CustomSelectControlV2 > = {
</div>
),
],
args: {
onChange: fn(),
},
Comment on lines +48 to +50
Copy link
Member Author

Choose a reason for hiding this comment

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

Necessary since v8, see:

https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#implicit-actions-can-not-be-used-during-rendering-for-example-in-the-play-function

Copy link
Contributor

@ciampo ciampo Dec 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think that adding these lines of code is necessary since we're already specifying argTypesRegex above — I tested removing these lines on my machine and I was still getting "actions" firing correctly. This also matches the documentation about actions.

Given that I don't think we any Storybook tests, and that we typically use actions.argTypesRegex, I'm not sure we even need to list @storybook/test as a dependency for the time being?

If instead we wanted to change approach and go down the "explicit action spies" route, we'd need to refactor all stories specifying action or actions in the meta config object.

Copy link
Member Author

Choose a reason for hiding this comment

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

Unfortunately, we seem to need these, otherwise I'm getting an error when navigating to these 3 stories (I tested them all, one by one, manually):

Here's the error:

Screenshot 2024-12-11 at 16 40 07

Once I add the action spies, the errors disappear.

Copy link
Contributor

Choose a reason for hiding this comment

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

Weird. It doesn't make sense to me, since I would then expect this error to appear on a lot more stories. I think the checks a bit buggy (and we may not be the only ones).

For the sake of moving forward with the refactor, I'm OK keeping those changes and investigate the reason for the error in a separate follow-up PR

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, and yep, would like to take a look separately.

Opened an issue here: #67834

};
export default meta;

Expand Down
4 changes: 4 additions & 0 deletions packages/components/src/tab-panel/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import type { Meta, StoryFn } from '@storybook/react';
import { fn } from '@storybook/test';

/**
* WordPress dependencies
Expand All @@ -22,6 +23,9 @@ const meta: Meta< typeof TabPanel > = {
controls: { expanded: true },
docs: { canvas: { sourceState: 'shown' } },
},
args: {
onSelect: fn(),
},
Comment on lines +26 to +28
Copy link
Member Author

Choose a reason for hiding this comment

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

Necessary since v8, see:

https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#implicit-actions-can-not-be-used-during-rendering-for-example-in-the-play-function

};
export default meta;

Expand Down
5 changes: 5 additions & 0 deletions packages/components/src/tabs/stories/index.story.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import type { Meta, StoryFn } from '@storybook/react';
import { fn } from '@storybook/test';

/**
* WordPress dependencies
Expand Down Expand Up @@ -39,6 +40,10 @@ const meta: Meta< typeof Tabs > = {
controls: { expanded: true },
docs: { canvas: { sourceState: 'shown' } },
},
args: {
onActiveTabIdChange: fn(),
onSelect: fn(),
},
Comment on lines +43 to +46
Copy link
Member Author

Choose a reason for hiding this comment

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

Necessary since v8, see: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#implicit-actions-can-not-be-used-during-rendering-for-example-in-the-play-function

};
export default meta;

Expand Down
55 changes: 55 additions & 0 deletions patches/storybook-source-link+2.0.9.patch
View file
Open in desktop
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the reason for this patch? Is it necessary to migrate to v8 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we need this patch primarily for two reasons:

  • Icons from @storybook/components is deprecated and throws warnings
  • @storybook/addons imports no longer work and Storybook errors.

Support for v8 is in progress but not there yet, so we need the patch to unblock the upgrade temporarily. Hopefully, the v8 support will land soon. And maybe we can lend a hand if that doesn't happen for some reason.

Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
diff --git a/node_modules/storybook-source-link/dist/esm/Tool.js b/node_modules/storybook-source-link/dist/esm/Tool.js
index 100099e..53d37c4 100644
--- a/node_modules/storybook-source-link/dist/esm/Tool.js
+++ b/node_modules/storybook-source-link/dist/esm/Tool.js
@@ -1,7 +1,8 @@
import React from "react";
-import { Icons, IconButton, TooltipMessage, WithTooltip } from "@storybook/components";
-import { PARAM_KEY, PREFIX_PARAM_KEY, ICON_PARAM_KEY, INFO_LINK, TOOL_ID } from "./constants";
-import { useParameter } from '@storybook/api';
Comment on lines +8 to +9
Copy link
Member Author

Choose a reason for hiding this comment

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

Cleanup of unused code after the introduced changes.

+import { IconButton, TooltipMessage, WithTooltip } from "@storybook/components";
+import { RepoIcon } from '@storybook/icons';
Copy link
Member Author

Choose a reason for hiding this comment

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

This is half of the patch: using @storybook/icons instead of Icons from @storybook/components.
Reason: since v8, Icons is deprecated in favor of explicit icon components, see: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#icons-is-deprecated

+import { PARAM_KEY, PREFIX_PARAM_KEY, INFO_LINK, TOOL_ID } from "./constants";
+import { useParameter } from '@storybook/manager-api';

var Tooltip = function Tooltip() {
return /*#__PURE__*/React.createElement(TooltipMessage, {
@@ -24,7 +25,6 @@ export var getLink = function getLink(prefix, link) {
export var Tool = function Tool() {
var param_link = useParameter(PARAM_KEY, null);
var param_prefix = useParameter(PREFIX_PARAM_KEY, null);
- var param_icon = useParameter(ICON_PARAM_KEY, "repository");
Copy link
Member Author

Choose a reason for hiding this comment

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

param_icon is unused, so we're removing it.

var link = getLink(param_prefix, param_link);
return link ? /*#__PURE__*/React.createElement(IconButton, {
key: TOOL_ID,
@@ -35,9 +35,7 @@ export var Tool = function Tool() {
window.open(link);
}
}
- }, /*#__PURE__*/React.createElement(Icons, {
- icon: param_icon
- })) : /*#__PURE__*/React.createElement(WithTooltip, {
Comment on lines +29 to +31
Copy link
Member Author

Choose a reason for hiding this comment

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

Now going with a <RepoIcon /> instead of <Icons icon="repo" />.

+ }, /*#__PURE__*/React.createElement(RepoIcon)) : /*#__PURE__*/React.createElement(WithTooltip, {
placement: "top",
trigger: "click",
tooltip: /*#__PURE__*/React.createElement(Tooltip, null)
@@ -45,7 +43,5 @@ export var Tool = function Tool() {
key: TOOL_ID,
title: "View Source Repository",
active: false
- }, /*#__PURE__*/React.createElement(Icons, {
- icon: param_icon
- })));
Comment on lines +40 to +42
Copy link
Member Author

Choose a reason for hiding this comment

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

Same as above - using <RepoIcon /> instead of <Icons icon="repo" />.

+ }, /*#__PURE__*/React.createElement(RepoIcon)));
};
\ No newline at end of file
diff --git a/node_modules/storybook-source-link/dist/esm/preset/manager.js b/node_modules/storybook-source-link/dist/esm/preset/manager.js
index 845f81d..ca1d066 100644
--- a/node_modules/storybook-source-link/dist/esm/preset/manager.js
+++ b/node_modules/storybook-source-link/dist/esm/preset/manager.js
@@ -1,4 +1,4 @@
-import { addons, types } from "@storybook/addons";
Copy link
Member Author

Choose a reason for hiding this comment

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

The other half of the patch: addons and types are now imported from @storybook/manager-api, see https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#removed-deprecated-shim-packages

+import { addons, types } from "@storybook/manager-api";
import { ADDON_ID, TOOL_ID } from "../constants";
import { Tool } from "../Tool"; // Register the addon

4 changes: 4 additions & 0 deletions storybook/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ module.exports = {
'@storybook/addon-a11y',
'@storybook/addon-toolbars',
'@storybook/addon-actions',
'@storybook/addon-webpack5-compiler-babel',
Copy link
Member Author

Choose a reason for hiding this comment

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

Needed in order to opt-in to using Babel explicitly, see https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#removed-babelcore-and-babel-loader-from-storybookbuilder-webpack5

'storybook-source-link',
'@geometricpanda/storybook-addon-badges',
],
Expand All @@ -64,6 +65,9 @@ module.exports = {
docs: {
autodocs: true,
},
typescript: {
reactDocgen: 'react-docgen-typescript',
},
Comment on lines +68 to +70
Copy link
Member Author

Choose a reason for hiding this comment

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

Needed since the default in v8 is react-docgen, see: https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#react-docgen-component-analysis-by-default

webpackFinal: async ( config ) => {
return {
...config,
Expand Down
5 changes: 2 additions & 3 deletions storybook/preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
* External dependencies
*/
import {
ArgsTable,
Controls,
Description,
Primary,
Stories,
Expand Down Expand Up @@ -114,8 +114,7 @@ export const parameters = {
<Subtitle />
<Primary />
<Description />
{ /* `story="^"` enables Controls for the primary props table */ }
<ArgsTable story="^" />
<Controls />
Copy link
Member Author

Choose a reason for hiding this comment

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

ArgsTable was removed in favor of Controls and ArgTypes, see https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#argstable-doc-block-removed

<Stories includePrimary={ false } />
</>
),
Expand Down
6 changes: 4 additions & 2 deletions storybook/sidebar.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,9 @@ const Title = styled.span( {

const Icons = styled.span( {} );

const Icon = styled.span( {} );
const Icon = styled.span( {
lineHeight: 1,
Copy link
Member Author

Choose a reason for hiding this comment

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

Needed to better vertically center the icon in the respective menu item.

} );

/**
* Fetches tags from the Storybook API, and returns Icon
Expand All @@ -41,7 +43,7 @@ function useIcons( item ) {
return useMemo( () => {
let data = {};

if ( item.isComponent && item.children?.length ) {
if ( item.type === 'component' && item.children?.length ) {
Copy link
Member Author

Choose a reason for hiding this comment

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

I didn't see this change documented in the changelog, but without it, the badges won't appear - isComponent doesn't exist, but I saw type is component, so we're using that.

data = api.getData( item.children[ 0 ] ) ?? {};
}

Expand Down
10 changes: 6 additions & 4 deletions storybook/stories/docs/inline-icon.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,13 @@
* External dependencies
*/
import styled from '@emotion/styled';
import { Icons } from '@storybook/components';
Copy link
Member Author

Choose a reason for hiding this comment

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

Icons from @storybook/components is deprecated in favor of @storybook/icons, see https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#icons-is-deprecated


const StyledIcons = styled( Icons )`
const IconWrapper = ( { icon, ...props } ) => {
const IconComponent = icon;
return <IconComponent aria-hidden { ...props } />;
};

export const InlineIcon = styled( IconWrapper )`
display: inline-block !important;
width: 14px;
`;

export const InlineIcon = ( props ) => <StyledIcons aria-hidden { ...props } />;
3 changes: 2 additions & 1 deletion storybook/stories/docs/introduction.mdx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Meta } from '@storybook/blocks';
import { RepoIcon } from '@storybook/icons';
import { InlineIcon } from './inline-icon';

<Meta title="Docs/Introduction" name="page" />
Expand Down Expand Up @@ -28,7 +29,7 @@ The site shows the individual components in the sidebar and the Canvas on the ri

To view the documentation for each component use the **Docs** menu item in the top toolbar.

To view the source code for the component and its stories on GitHub, click the <InlineIcon icon="repository" /> View Source Repository button in the top right corner.
To view the source code for the component and its stories on GitHub, click the <InlineIcon icon={ RepoIcon } /> View Source Repository button in the top right corner.
Copy link
Member Author

Choose a reason for hiding this comment

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

We're using RepoIcon from @storybook/icons directly since Icon from @storybook/components is deprecated, see https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#icons-is-deprecated


To use it in your local development environment run the following command in the top level Gutenberg directory:

Expand Down
Loading