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

Bump @storybook/react to ^7.4.2 #1656

Merged
merged 19 commits into from
Oct 27, 2023

Conversation

yangwooseong
Copy link
Collaborator

@yangwooseong yangwooseong commented Oct 4, 2023

Self Checklist

  • I wrote a PR title in English and added an appropriate label to the PR.
  • I wrote the commit message in English and to follow the Conventional Commits specification.
  • I added the changeset about the changes that needed to be released. (or didn't have to)
  • I wrote or updated documentation related to the changes. (or didn't have to)
  • I wrote or updated tests related to the changes. (or didn't have to)
  • I tested the changes in various browsers. (or didn't have to)
    • Windows: Chrome, Edge, (Optional) Firefox
    • macOS: Chrome, Edge, Safari, (Optional) Firefox

Related Issue

Fixes #1255

Summary

  • 스토리북 메이저 버전을 7로 올리고 브레이킹 체인지에 대응합니다.

Details

버전 업에 따른 변경 사항은 다음과 같습니다.

  • main 파일 마이그레이션 (codemod 사용), ts 마이그레이션하여 스토리북에서 제공하는 타입 활용 (#). preview 파일도 themeProvider 분리하고 ts 마이그레이션
  • getTitle 유틸을 활용해서 component/.../lastFolderName형식으로 타이틀 얻어오던 것을 제거. 스토리북 7버전 부터는 동적으로 타이틀을 계산하는 것을 허용하지 않고 있고, 명시적으로 지정하지 않으면 알아서 경로를 잘 보여주게 됩니다. (#)
  • CSF2 -> CSF3으로 변경 (codemod 사용)
  • controloption 필드가 control 아래에 있었던 것을 하나 올려서 같은 depth 가 되도록 변경 (#)
  • default export 되는 meta 의 타입 단언을 타입 선언으로 변경
  • 이제 docs 가 스토리마다 있는 것이 아니라 컴포넌트 단위로 존재하게 됩니다.

이후 pr 에서 아래 이슈들을 차례대로 다루면 될 것 같습니다.

#1083
#1000

Breaking change? (Yes/No)

  • No

References

@yangwooseong yangwooseong added enhancement Issues or PR related to making existing features better dependencies Pull requests that update a dependency file (Dependabot) labels Oct 4, 2023
@yangwooseong yangwooseong self-assigned this Oct 4, 2023
@changeset-bot
Copy link

changeset-bot bot commented Oct 4, 2023

⚠️ No Changeset found

Latest commit: 7ea1b34

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2023

Chromatic Report

🚀 Congratulations! Your build was successful!

@codecov
Copy link

codecov bot commented Oct 4, 2023

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (199b181) 87.16% compared to head (7ea1b34) 87.16%.
Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1656   +/-   ##
=======================================
  Coverage   87.16%   87.16%           
=======================================
  Files         281      281           
  Lines        3904     3904           
  Branches      822      822           
=======================================
  Hits         3403     3403           
  Misses        426      426           
  Partials       75       75           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@yangwooseong yangwooseong force-pushed the feat/migrate-to-storybook-7 branch 2 times, most recently from 65678a5 to 6c17064 Compare October 11, 2023 13:34
@@ -54,10 +57,11 @@ const Template: StoryFn<typeof AlphaStack> = (args) => (
</AlphaStack>
)

export const Primary: StoryObj<typeof AlphaStack> = {
export const Primary: Story = {
Copy link
Collaborator Author

@yangwooseong yangwooseong Oct 11, 2023

Choose a reason for hiding this comment

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

https://storybook.js.org/blog/improved-type-safety-in-storybook-7/?ref=storybookblog.ghost.io 에 의하면 satiesfies 를 도입하여 강타입으로 스토리를 선언할 수 있어서 런타임 에러 방지가 가능하다고 합니다. 다만 이미 작성된 스토리를 마이그레이션하더라도 유의미한 차이는 없을 것 같아 굳이 모두 바꾸지는 않았습니다.

Copy link
Contributor

Choose a reason for hiding this comment

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

제 생각에도 지금은 큰 의미 없을 거 같고, #1331 에서 같이 챙겨볼 수 있을 거 같아요. (#849 와 함께 챙겨야 할듯)

export default {
title: '~/src/foundation/Typography',
const meta: Meta = {
title: 'Foundation/Typography',
Copy link
Collaborator Author

@yangwooseong yangwooseong Oct 11, 2023

Choose a reason for hiding this comment

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

오타 수정하여 Typography 스토리가 가장 위에 뜨고 있는 버그 수정

image

@yangwooseong yangwooseong force-pushed the feat/migrate-to-storybook-7 branch from e5367af to 17f154c Compare October 11, 2023 14:48
component: TabsComposition,
subcomponents: {
Copy link
Collaborator Author

@yangwooseong yangwooseong Oct 11, 2023

Choose a reason for hiding this comment

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

subcomponents 필드가 버전7에서 deprecated되었다고 합니다. 여기서 제안하는 ArgsTable 을 사용하는 것을 고민중입니다. -> 시도해보았으나 안되네요 😢

스크린샷 2023-10-16 오후 4 39 15

@@ -87,7 +58,7 @@ const innerWrapperStyle = {
borderRadius: 20,
}

function withFoundationProvider(Story, context) {
export function WithFoundationProvider(Story, context) {
Copy link
Collaborator Author

@yangwooseong yangwooseong Oct 16, 2023

Choose a reason for hiding this comment

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

WithFoundationProvider.jsx 를 tsx 로 변경하려면 tsconfig.include 에 파일을 포함시키고 rootDir를 '.' 로 해야되는데, 그러면 타입 빌드 결과가 달라지게 되서 jsx 그대로 두었습니다

Copy link
Contributor

Choose a reason for hiding this comment

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

복잡한 설정 없이 타이핑 이점을 누리기 위해서 아예 preview.ts 내부로 옮기는 건 어떤가요?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

혹시 src/ 폴더를 의미하신 걸까요?

Copy link
Contributor

@sungik-choi sungik-choi Oct 24, 2023

Choose a reason for hiding this comment

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

.storybook/preview.ts 에요. 해당 파일안에 직접 작성하면 컴파일하지 않아도 괜찮지 않을까? 싶었습니다

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

preview.ts 로 넣으면 path alias 를 사용못하게 되서 ../src/... 이런 식으로 import 를 해야하는데, 그렇게 하면 Text 컴포넌트 타입을 다르게 읽어서 타입 에러가 뜹니다. 이 부분에 대한 원인을 찾지 못했습니다.

image

@yangwooseong yangwooseong force-pushed the feat/migrate-to-storybook-7 branch from 7f884bc to 1469a96 Compare October 16, 2023 07:42
@yangwooseong yangwooseong marked this pull request as ready for review October 16, 2023 07:52
'../src/stories/Intro.stories.mdx',
'../src/**/*.stories.(tsx|mdx)',
Copy link
Contributor

Choose a reason for hiding this comment

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

'../src/stories/Intro.stories.mdx' 없이도 동일한 동작일 거 같아요

Comment on lines +40 to +52
webpackFinal: async (config) => {
config.resolve = {
...config.resolve,
// Apply tsconfig alias path
plugins: [
...(config?.resolve?.plugins ?? []),
new TsconfigPathsPlugin({}),
],
extensions: [
...(config.resolve?.extensions ?? []),
'.ts',
'.tsx',
]
}
Copy link
Contributor

Choose a reason for hiding this comment

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

중요하진 않지만, 나중에 Vite로 마이그레이션도 해볼만할 거 같아요.

}
}

export default config
Copy link
Contributor

Choose a reason for hiding this comment

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

newline

Comment on lines 1 to 3
---
"@channel.io/bezier-react": minor
---
Copy link
Contributor

Choose a reason for hiding this comment

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

버전 업은 하지 않아도 괜찮지 않을까요? 라이브러리를 사용하는 입장에서는 기능 추가 등의 변화가 있는 것도 아니고, 내부 컴포넌트 구현이 변경된 것도 아니라서 버전을 올리지 않아도 될 거 같아요.

@@ -87,7 +58,7 @@ const innerWrapperStyle = {
borderRadius: 20,
}

function withFoundationProvider(Story, context) {
export function WithFoundationProvider(Story, context) {
Copy link
Contributor

Choose a reason for hiding this comment

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

복잡한 설정 없이 타이핑 이점을 누리기 위해서 아예 preview.ts 내부로 옮기는 건 어떤가요?

@yangwooseong yangwooseong force-pushed the feat/migrate-to-storybook-7 branch from 1469a96 to 4dad50d Compare October 24, 2023 07:48
@yangwooseong yangwooseong merged commit 572ac77 into channel-io:main Oct 27, 2023
6 checks passed
@yangwooseong yangwooseong deleted the feat/migrate-to-storybook-7 branch October 27, 2023 01:00
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 (Dependabot) enhancement Issues or PR related to making existing features better
Projects
No open projects
Archived in project
Development

Successfully merging this pull request may close these issues.

Upgrade from Storybook 6.x to 7.0
2 participants