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 Storybook to v7, move builder to Vite #37331

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions package.json
Copy link
Member

Choose a reason for hiding this comment

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

The stories for DocumentCluster in teleterm fail with "ambiguous import: FileTransferDirection" and I didn't have time to investigate this. Maybe it's a matter of updating this branch with master?

Copy link
Member

Choose a reason for hiding this comment

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

@avatus Regarding your question as to what work is left to in this PR, here's what I see:

  • Before touching anything else, reconcile the stories that were renamed from .js to .jsx. Upgrade Storybook to v7, move builder to Vite #37331 (comment) This probably boils down to just removing the .js version.
    • It might be beneficial to amend those changes into 1c81e4b so that git properly sees that those stories were renamed – this should help with bringing this branch up to date.
  • Merge master and address any conflicts.
  • Bring https://github.com/gravitational/teleport.e/pull/3256 up to date and replace new calls to worker.use with proper usage of msw.
  • There's the possibility that we'll run into some new issues.

I just quickly ran git merge master on this branch as it is right now and it doesn't look that scary. There are conflicts in build/package.json, but resolving them should be easy – pick the version from master and then manually bump the versions just as they are currently bumped in this PR. Possibly install new minor/major versions of v7 that were released since this PR was created.

Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@
"build-term": "yarn workspace @gravitational/teleterm build",
"start-term": "yarn workspace @gravitational/teleterm start",
"package-term": "yarn workspace @gravitational/teleterm package",
"storybook": "start-storybook -p 9002 -c web/.storybook -s web/.storybook/public",
"storybook-smoke-test": "yarn storybook --ci --smoke-test",
"storybook": "VITE_NO_PROXY=1 storybook dev -p 9002 -c web/.storybook",
"storybook-smoke-test": "VITE_NO_PROXY=1 yarn storybook --ci --smoke-test",
"test": "jest",
"test-coverage": "jest --coverage && web/scripts/print-coverage-link.sh",
"test-update-snapshot": "yarn test -- --updateSnapshot",
Expand Down
3 changes: 0 additions & 3 deletions web/.storybook/.babelrc

This file was deleted.

126 changes: 0 additions & 126 deletions web/.storybook/main.js

This file was deleted.

72 changes: 72 additions & 0 deletions web/.storybook/main.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
/*
* Teleport
* Copyright (C) 2023 Gravitational, Inc.
*
* This program is free software: you can redistribute it and/or modify
* it under the terms of the GNU Affero General Public License as published by
* the Free Software Foundation, either version 3 of the License, or
* (at your option) any later version.
*
* This program is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
* GNU Affero General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import path, { dirname, join, resolve } from 'path';
import { Plugin, UserConfig } from 'vite';
import type { StorybookConfig } from '@storybook/react-vite';
import fs from 'fs';

const enterpriseTeleportExists = fs.existsSync(
path.join(__dirname, '/../../e/web')
);

function getAbsolutePath(value: string): any {
return dirname(require.resolve(join(value, 'package.json')));
}

function createStoriesPaths() {
const stories = ['../packages/**/*.story.@(ts|tsx)'];

// include enterprise stories if available (**/* pattern ignores dot dir names)
if (enterpriseTeleportExists) {
stories.unshift('../../e/web/**/*.story.@(ts|tsx)');
}

return stories;
}

const rootDirectory = path.resolve(__dirname, '..', '..');
const webDirectory = path.resolve(rootDirectory, 'web');

const config: StorybookConfig = {
stories: createStoriesPaths(),
addons: [getAbsolutePath('@storybook/addon-toolbars')],
framework: {
name: getAbsolutePath('@storybook/react-vite'),
options: {
builder: {
viteConfigPath: resolve(
__dirname,
'../../web/packages/teleport/vite.config.mts'
),
},
},
},
staticDirs: ['public'],
async viteFinal(config: UserConfig) {
config.plugins = config.plugins.filter(
(plugin: Plugin) =>
plugin.name !== 'teleport-html-plugin' &&
plugin.name !== 'teleport-transform-html-plugin'
);

return config;
},
};

export default config;
107 changes: 59 additions & 48 deletions web/.storybook/preview.js → web/.storybook/preview.tsx
Copy link
Member

Choose a reason for hiding this comment

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

As I mentioned in #34450, msw-storybook-addon has some problems with resetting the handlers between stories when using res.once.

For example, if you open the Access List story and then open "Empty with Igs", which uses res.once, other Access List stories will return 404 from cfg.getAccessManagementListUrl(). The workaround is to refresh the page between each story.

Now, the Access List story doesn't need to use res.once at all, it can just use res instead and the issue is gone. I already fixed it in the teleport.e branch. Other than that, we only have four stories which use res.once, so perhaps for now we can live with this problem. The issue in msw-storybook-addon repo doesn't mention any solutions to this. mswjs/msw-storybook-addon#82

Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,11 @@
* along with this program. If not, see <http://www.gnu.org/licenses/>.
*/

import React from 'react';
import { rest, setupWorker } from 'msw';
import { addDecorator, addParameters } from '@storybook/react';
import React, { ComponentType, PropsWithChildren } from 'react';
import {
bblpTheme,
darkTheme,
lightTheme,
bblpTheme,
} from './../packages/design/src/theme';
import DefaultThemeProvider from '../packages/design/src/ThemeProvider';
import Box from './../packages/design/src/Box';
Expand All @@ -32,35 +30,34 @@ import {
darkTheme as teletermDarkTheme,
lightTheme as teletermLightTheme,
} from './../packages/teleterm/src/ui/ThemeProvider/theme';
import { handlersTeleport } from './../packages/teleport/src/mocks/handlers';
import history from './../packages/teleport/src/services/history/history';
import { UserContextProvider } from 'teleport/User';
import { Preview } from '@storybook/react';
import { Theme } from 'design/theme/themes/types';
import { initialize, mswLoader } from 'msw-storybook-addon';

// Checks we are running non-node environment (browser)
if (typeof global.process === 'undefined') {
const worker = setupWorker(...handlersTeleport);
worker.start();

// So it can be accessed in stories more easily.
window.msw = { worker, rest };
}
initialize({
onUnhandledRequest: 'bypass',
});

history.init();

// wrap each story with theme provider
const ThemeDecorator = (storyFn, meta) => {
let ThemeProvider;
let theme;
interface ThemeDecoratorProps {
theme: string;
title: string;
}

function ThemeDecorator(props: PropsWithChildren<ThemeDecoratorProps>) {
let ThemeProvider: ComponentType<PropsWithChildren<{ theme: Theme }>>;
let theme: Theme;

if (meta.title.startsWith('Teleterm/')) {
if (props.title.startsWith('Teleterm/')) {
ThemeProvider = TeletermThemeProvider;
theme =
meta.globals.theme === 'Dark Theme'
? teletermDarkTheme
: teletermLightTheme;
props.theme === 'Dark Theme' ? teletermDarkTheme : teletermLightTheme;
} else {
ThemeProvider = DefaultThemeProvider;
switch (meta.globals.theme) {
switch (props.theme) {
case 'Dark Theme':
theme = darkTheme;
break;
Expand All @@ -75,29 +72,32 @@ const ThemeDecorator = (storyFn, meta) => {

return (
<ThemeProvider theme={theme}>
<Box p={3}>{storyFn()}</Box>
<Box p={3}>{props.children}</Box>
</ThemeProvider>
);
};
}

// wrap stories with an argument of {userContext: true} with user context provider
const UserDecorator = (storyFn, meta) => {
if (meta.args.userContext) {
const UserProvider = UserContextProvider;
interface UserDecoratorProps {
userContext?: boolean;
}

function UserDecorator(props: PropsWithChildren<UserDecoratorProps>) {
if (props.userContext) {
return (
<UserProvider>
<Box p={3}>{storyFn()}</Box>
</UserProvider>
<UserContextProvider>
<Box p={3}>{props.children}</Box>
</UserContextProvider>
);
}

return <Box p={3}>{storyFn()}</Box>;
};
return <Box p={3}>{props.children}</Box>;
}

addDecorator(UserDecorator);
addDecorator(ThemeDecorator);
addParameters({
options: {
const preview: Preview = {
args: {
userContext: false,
},
parameters: {
showPanel: false,
showNav: true,
isToolshown: true,
Expand All @@ -106,17 +106,28 @@ addParameters({
order: ['Teleport', 'TeleportE', 'Teleterm', 'Design', 'Shared'],
},
},
});

export const globalTypes = {
theme: {
name: 'Theme',
description: 'Global theme for components',
defaultValue: 'Dark Theme',
toolbar: {
icon: 'contrast',
items: ['Light Theme', 'Dark Theme', 'BBLP Theme'],
dynamicTitle: true,
loaders: [mswLoader],
globals: {
theme: {
name: 'Theme',
description: 'Global theme for components',
defaultValue: 'Dark Theme',
toolbar: {
icon: 'contrast',
items: ['Light Theme', 'Dark Theme', 'BBLP Theme'],
dynamicTitle: true,
},
},
},
decorators: [
(Story, meta) => (
<UserDecorator userContext={meta.args.userContext}>
<ThemeDecorator theme={meta.globals.theme} title={meta.title}>
<Story />
</ThemeDecorator>
</UserDecorator>
),
],
};

export default preview;
Loading
Loading