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

🪟 🔧 Upgrade to Storybook 7 #22056

Merged
merged 9 commits into from
Jan 31, 2023
Merged

🪟 🔧 Upgrade to Storybook 7 #22056

merged 9 commits into from
Jan 31, 2023

Conversation

timroes
Copy link
Collaborator

@timroes timroes commented Jan 28, 2023

What

This upgraded Storybook to the new Storybook 7 Beta. Since we're currently blocked on the switch to pnpm on Storybook, I already updated now during the Beta phase. This also now makes sure to use Vite for building storybook instead of webpack.

Please regard the inline comments for additional details.

@octavia-squidington-iv octavia-squidington-iv added the area/frontend Related to the Airbyte webapp label Jan 28, 2023
@@ -38,11 +39,13 @@ export const withProviders = (getStory) => (
<MemoryRouter>
<IntlProvider messages={messages} locale={"en"}>
<ThemeProvider theme={theme}>
<ConfigServiceProvider defaultConfig={defaultConfig} providers={[]}>
<ConfigServiceProvider config={config}>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ This is kinda "funny" that this worked beforehand. So it seems that Storybook never did typecheck those files, until Storybook 7 (or use using a main.ts instead of main.js).

@@ -154,7 +152,7 @@
"orval": "^6.11.0-alpha.10",
"prettier": "^2.6.2",
"react-select-event": "^5.5.0",
"storybook-addon-mock": "^2.4.1",
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ This isn't compatible with Storybook 7 anymore. I've checked the only place it was used ConnectorForm and it seems this endpoint is no longer called during storybook (I assume this was resolved due to some refactoring we did in this form), so we no longer need to mock any data in storybook.

@@ -7,7 +7,7 @@ import { loadDatadog } from "utils/datadog";
import { loadOsano } from "utils/dataPrivacy";
import { loadSentry } from "utils/sentry";

import "./globals";
import "./dayjs-setup";
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ This is required, due to some missmatch in require paths, we can't have this file named be globals in src directly 🤷 No one knows exactly why the pathes are missmatched, but this is the easiest workaround for now.

} as ComponentMeta<typeof ConnectorForm>;

const Template: ComponentStory<typeof ConnectorForm> = (args) => {
const Template: ComponentStory<typeof ConnectorForm> = (args: React.ComponentProps<typeof ConnectorForm>) => {
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ Given those files are typechecked now, this is now needed.

@timroes timroes marked this pull request as ready for review January 30, 2023 16:36
.github/workflows/gradle.yml Outdated Show resolved Hide resolved
<FeatureService features={[]}>
{getStory()}
</FeatureService>
<AppMonitoringServiceProvider>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ This broke the ConnectorForm which I think didn't work on master anymore.

@timroes timroes temporarily deployed to more-secrets January 30, 2023 17:15 — with GitHub Actions Inactive
@timroes timroes temporarily deployed to more-secrets January 30, 2023 17:15 — with GitHub Actions Inactive
@timroes timroes requested a review from a team as a code owner January 30, 2023 19:27
@@ -71,6 +71,10 @@ task buildStorybook(type: NpmTask) {
inputs.dir 'src'

outputs.dir 'build/storybook'

environment = [
'NODE_OPTIONS': '--max_old_space_size=4096'
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

ℹ️ It seems storybooks compile process with Vite requires more memory. I saw CI failing with an heap out of memory error on this, so giving it more space here.

@timroes timroes requested a review from edmundito January 30, 2023 21:28
Copy link
Contributor

@edmundito edmundito left a comment

Choose a reason for hiding this comment

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

Changes look good. Tested locally and in chromatic

@timroes timroes temporarily deployed to more-secrets January 31, 2023 09:03 — with GitHub Actions Inactive
@timroes timroes temporarily deployed to more-secrets January 31, 2023 09:03 — with GitHub Actions Inactive
@github-actions
Copy link
Contributor

Airbyte Code Coverage

There is no coverage information present for the Files changed

Total Project Coverage 24.51%

@timroes timroes temporarily deployed to more-secrets January 31, 2023 09:15 — with GitHub Actions Inactive
@timroes timroes temporarily deployed to more-secrets January 31, 2023 09:15 — with GitHub Actions Inactive
@timroes timroes temporarily deployed to more-secrets January 31, 2023 09:24 — with GitHub Actions Inactive
@timroes timroes temporarily deployed to more-secrets January 31, 2023 09:24 — with GitHub Actions Inactive
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/frontend Related to the Airbyte webapp
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants