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

storybook v6 -> v7 #1214

Merged
merged 14 commits into from
Apr 24, 2023
Merged

storybook v6 -> v7 #1214

merged 14 commits into from
Apr 24, 2023

Conversation

penicillin0
Copy link
Contributor

@penicillin0 penicillin0 commented Apr 3, 2023

目的

  • Storybookのv7化

@netlify
Copy link

netlify bot commented Apr 3, 2023

Deploy Preview for ingred-ui ready!

Name Link
🔨 Latest commit a2be965
🔍 Latest deploy log https://app.netlify.com/sites/ingred-ui/deploys/6447004144d44000089f6cad
😎 Deploy Preview https://deploy-preview-1214--ingred-ui.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@@ -8,26 +8,25 @@ module.exports = {
"@storybook/addon-essentials",
"@storybook/addon-links",
"@storybook/addon-storysource",
"@storybook/addon-postcss",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

v7では使えなさそう
storybookjs/storybook#20529

そもそもPostCSS使いたくていれたものじゃなさそう
https://github.com/voyagegroup/ingred-ui/blame/master/.storybook/main.js#L11

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ref: #1226

@@ -0,0 +1,23 @@
{
Copy link
Contributor Author

Choose a reason for hiding this comment

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

v7からは.babelrcを作ってそれを読むようになるっぽい

https://storybook.js.org/docs/react/configure/babel#v7-mode

Comment on lines 14 to 17
options: {
sourceLoaderOptions: {
csfPluginOptions: {
injectStoryParameters: false,
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -1,6 +1,5 @@
import * as React from "react";
import { ThemeProvider, createTheme } from "../src/themes";
import "@storybook/addon-console";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

v7対応されなそう
storybookjs/storybook-addon-console#65

必要か判断した上で、代替手段を要調査

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@penicillin0
Copy link
Contributor Author

penicillin0 commented Apr 10, 2023

source codeが正しく表示されない

displayNameをいれれば表示されそう。
storybookjs/storybook#20920 (comment)
そもそもforwardRefしているところが、eslint(react/display-name)違反ではある。
選択肢としては

  • source codeは諦める
  • displayNameを全部指定するようにする
  • functionを使う <-これで

追記(4/18)
対応されたのでingred-uiとしてはこのままで問題ない、要議論
storybookjs/storybook#22048

@penicillin0
Copy link
Contributor Author

penicillin0 commented Apr 10, 2023

args table表示されない

storybookjs/storybook#13304
まとめると

  • default exportだと表示されないっぽい(実際直った)
  • import * as React from 'react';で直ってる人もいた(直らなかった)
  • 公式側もdefault exportでもどうにかできないか対応中

追記(4/18)
対応された、ingred-ui側では対応必要なし
storybookjs/storybook#22024

@penicillin0
Copy link
Contributor Author

penicillin0 commented Apr 17, 2023

現状Storybook側がバグ対応中のステータス。なのでv7化においては、以下の方針が良いと気がした。

  • 優先度低い機能なら無理に直そうとせず、issue化しておき、Storybook側の対応を待つ。
  • deprecatedなものはissue化しておいて、v7移行後に順次対応する。

@@ -168,6 +168,7 @@ module.exports = {
// I realize it's silly of me, but I'll turn it off here for the sake of specs for once.
"jsx-a11y/click-events-have-key-events": "off",
"jsx-a11y/no-static-element-interactions": "off",
"react-hooks/rules-of-hooks": "off",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Reactに状態をもたせる際、renderプロパティ内に関数を書く必要があり、この中でhooksを利用すると上記のエラーが出る。
いちいちエラーを消すのは手間であるため、*.stories.tsxではこのルールをoffにした。

@@ -67,15 +67,17 @@ describe("{{ inputs.name | pascal }} component testing", () => {

```typescript
import * as React from "react";
import { Story } from "@storybook/react/types-6-0";
import { StoryObj } from "@storybook/react";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Storyは非推奨、v7化以降はオブジェクトで書く。
https://github.com/storybookjs/storybook/blob/next/MIGRATION.md#story-type-deprecated

@@ -1,6 +1,5 @@
import * as React from "react";
import { ThemeProvider, createTheme } from "../src/themes";
import "@storybook/addon-console";
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment on lines 14 to 17
options: {
sourceLoaderOptions: {
csfPluginOptions: {
injectStoryParameters: false,
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@@ -14,7 +14,7 @@ export default {
},
parameters: {
docs: {
source: { type: "code" },
source: { language: "tsx" },
Copy link
Contributor Author

Choose a reason for hiding this comment

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

type: "code"の場合、オブジェクトがそのまま表示されてしまい、docとしての役割としては実装サンプルが見えているはずなので、language: "tsx"にする。
ref: https://storybook.js.org/docs/angular/api/doc-block-source#language

color: "secondary",
children: "secondary",
export const Primary: StoryObj<typeof Badge> = {
...Template,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

これまでTemplateはこんな感じに書く

ref: https://storybook.js.org/docs/react/writing-stories/introduction#using-args

@penicillin0 penicillin0 marked this pull request as ready for review April 17, 2023 15:59
@penicillin0 penicillin0 self-assigned this Apr 17, 2023
page: () => (
<>
<Title />
<Subtitle />
<Description markdown="`<Divider />` is wrapper of `<hr />` tag that separate content into clear groups." />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@takurinton
Copy link
Contributor

今日中にはレビューします!!

Copy link
Contributor

@takurinton takurinton left a comment

Choose a reason for hiding this comment

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

scaffdog のところ以外は概ね良さそうです!!(そこだけ修正してもらえると)

export const Example: Story<{{ inputs.name | pascal }}Props> = (args) => (
<{{ inputs.name | pascal }} {...args} />
);
export const Example: StoryStoryObj<{{ inputs.name | pascal }}Props> = {
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
export const Example: StoryStoryObj<{{ inputs.name | pascal }}Props> = {
export const Example: StoryObj<{{ inputs.name | pascal }}Props> = {

@takurinton
Copy link
Contributor

ありがとうございましたmmmmmmm

@penicillin0 penicillin0 merged commit 5d249d3 into master Apr 24, 2023
2 checks passed
@penicillin0 penicillin0 deleted the storybook-v7 branch April 24, 2023 22:21
takurinton added a commit that referenced this pull request Apr 25, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants