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

Migrate storybook to 6.2.9 #18693

Merged
merged 52 commits into from
Jul 22, 2021
Merged
Show file tree
Hide file tree
Changes from 15 commits
Commits
Show all changes
52 commits
Select commit Hold shift + click to select a range
df00dc0
fix no static file found error on sb start
TristanWatanabe Jun 17, 2021
704f2e2
clean upgrade to 6.1.21
TristanWatanabe Jun 23, 2021
4e2898c
upgrade to 6.2.9
TristanWatanabe Jun 24, 2021
0501c6f
yarn lock file
TristanWatanabe Jun 24, 2021
8050fae
replace class with className in mdx file
TristanWatanabe Jun 24, 2021
fd72afd
Change files
TristanWatanabe Jun 24, 2021
528e246
Merge branch 'master' into migrate-storybook
TristanWatanabe Jun 24, 2021
0d1b1d9
run npx browserslist@latest --update-db
TristanWatanabe Jun 24, 2021
1990505
configure addon-postcss and add autoprefixer as dev dep
TristanWatanabe Jun 24, 2021
4764ee5
match autoprefixer versions
TristanWatanabe Jun 24, 2021
c550c17
update import paths
TristanWatanabe Jun 24, 2021
076a03b
add moduleNameMapper for storybook file
ecraig12345 Jun 24, 2021
b0d14fa
remove new style class injected in body by SB
TristanWatanabe Jun 25, 2021
060fd1a
remove temporary fix
TristanWatanabe Jun 25, 2021
8a652de
Merge branch 'master' into migrate-storybook
TristanWatanabe Jun 28, 2021
573bdc8
set fixed version of autoprefixer and remove from scripts package
TristanWatanabe Jun 28, 2021
0851937
deduplicate babel scope except for babel/core
TristanWatanabe Jun 30, 2021
0e2da26
run yarn after deduplication
TristanWatanabe Jun 30, 2021
74e02e3
Merge branch 'master' into migrate-storybook
TristanWatanabe Jun 30, 2021
05ad646
fix yarn lock file
TristanWatanabe Jul 1, 2021
b6f4d2e
deduplicate
TristanWatanabe Jul 1, 2021
968ee6d
update yarn lock after deduplication
TristanWatanabe Jul 1, 2021
28da3c1
Merge branch 'migrate-storybook' of https://github.com/TristanWatanab…
TristanWatanabe Jul 1, 2021
387d762
update yarn lock
TristanWatanabe Jul 1, 2021
c466480
Merge branch 'master' into migrate-storybook
TristanWatanabe Jul 1, 2021
b5b9a94
fix storybook startup issue for converged pkgs
TristanWatanabe Jul 1, 2021
26e055b
remove sb deps in vr-tests
TristanWatanabe Jul 1, 2021
148907d
Merge branch 'master' into migrate-storybook
TristanWatanabe Jul 15, 2021
1d9acb6
update yarn lock
TristanWatanabe Jul 15, 2021
63f3422
remove changes by linter
TristanWatanabe Jul 15, 2021
0e80abb
remove changes caused by linter
TristanWatanabe Jul 15, 2021
83ac9ed
update storybook package deps to 6.2.9
TristanWatanabe Jul 15, 2021
8802131
add webpack5 support, autoprefixer 10, and update overrideDefaultBabe…
TristanWatanabe Jul 15, 2021
7814d32
update webpack config file-loader
TristanWatanabe Jul 15, 2021
b337471
add crypto and stream polyfills to react-examples SB webpack config
TristanWatanabe Jul 15, 2021
bb17948
remove addon-info
TristanWatanabe Jul 15, 2021
677a750
Merge branch 'master' into migrate-storybook
TristanWatanabe Jul 15, 2021
4fbc960
add vr polyfill to react-examples SB webpack config
TristanWatanabe Jul 15, 2021
14f7a08
add rule to eslintrc file
TristanWatanabe Jul 15, 2021
5551645
Merge branch 'master' into migrate-storybook
TristanWatanabe Jul 16, 2021
a9cf49c
remove open argType from react-popover Default story
TristanWatanabe Jul 16, 2021
50d4cbf
Change files
TristanWatanabe Jul 16, 2021
0a03584
remove sb builder webpack5 from satisfied ignore list
TristanWatanabe Jul 16, 2021
8797c07
Merge branch 'master' into migrate-storybook
TristanWatanabe Jul 19, 2021
2ebdab9
remove postcss addon
TristanWatanabe Jul 20, 2021
37bf1be
update vrtests eslintrc
TristanWatanabe Jul 20, 2021
38e74b5
Merge branch 'master' into migrate-storybook
TristanWatanabe Jul 20, 2021
43ed940
update yarn lock file
TristanWatanabe Jul 20, 2021
d93c6ea
cleanup
TristanWatanabe Jul 21, 2021
6c7d61c
Merge branch 'master' into migrate-storybook
TristanWatanabe Jul 21, 2021
7b9c974
remove changes made by linter
TristanWatanabe Jul 21, 2021
640c4d3
Merge branch 'master' into migrate-storybook
TristanWatanabe Jul 22, 2021
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
8 changes: 8 additions & 0 deletions .storybook/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,14 @@ module.exports = /** @type {Pick<StorybookConfig,'addons' |'stories' |'webpackFi
'@storybook/addon-a11y',
'@storybook/addon-knobs/preset',
'storybook-addon-performance',
{
name: '@storybook/addon-postcss',
Copy link
Contributor

Choose a reason for hiding this comment

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

q: why is this needed ?

Copy link
Member Author

Choose a reason for hiding this comment

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

Since we require postcss 8.2.4, this needed to be added per document snippet below:

image

Copy link
Contributor

Choose a reason for hiding this comment

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

hmm I don't think we need postcss for vNext (as styling is processed by make-styles - so vendor prefix etc are being properly added - am I right @layershifter ?)

Copy link
Member

Choose a reason for hiding this comment

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

Yes, it's correct, we don't need PostCSS for vNext 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.

Got it - I will remove addon-postcss from the storybook addons and also remove the postcss.config.js file i created.

options: {
postcssLoaderOptions: {
implementation: require('postcss'),
},
},
},
],
webpackFinal: config => {
const tsPaths = new TsconfigPathsPlugin({
Expand Down
16 changes: 1 addition & 15 deletions apps/vr-tests/.storybook/preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,7 @@ setAddon({
},
});

/**
* @type {import('@storybook/react').Meta['decorators']}
*/
export const decorators = [removeCanvasInlineStyles];

/**
* Temporary solution to remove inline styles injected by new default SB layout (https://storybook.js.org/docs/react/configure/story-layout)
* TODO - remove this once we migrate to SB 6.1
* @see https://github.com/storybookjs/storybook/issues/12041#issuecomment-717177177
* @param {Parameters<import('@storybook/react').Meta['decorators'][number]>[0]} Story
*/
function removeCanvasInlineStyles(Story) {
document.body.removeAttribute('style');
return createElement(Story);
}
export const parameters = { layout: 'none' };

// For static storybook per https://github.com/screener-io/screener-storybook#testing-with-static-storybook-app
if (typeof window === 'object') {
Expand Down
6 changes: 3 additions & 3 deletions apps/vr-tests/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@
"@fluentui/react-theme-provider": "^9.0.0-alpha.44",
"@fluentui/scripts": "^1.0.0",
"@fluentui/storybook": "^1.0.0",
"@storybook/addons": "6.0.28",
"@storybook/channels": "6.0.28",
"@storybook/react": "6.0.28",
"@storybook/addons": "6.2.9",
"@storybook/channels": "6.2.9",
"@storybook/react": "6.2.9",
Copy link
Member

Choose a reason for hiding this comment

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

Remove these since they're now declared at the root

Suggested change
"@storybook/addons": "6.2.9",
"@storybook/channels": "6.2.9",
"@storybook/react": "6.2.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.

Removing these seems to have caused a build failure 🤔

Copy link
Member

Choose a reason for hiding this comment

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

What's the error?

Copy link
Member Author

Choose a reason for hiding this comment

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

image

"@types/react": "16.9.42",
"@types/react-dom": "16.9.10",
"babel-loader": "8.2.2",
Expand Down
2 changes: 1 addition & 1 deletion apps/vr-tests/src/utilities/FabricDecorator.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import * as React from 'react';
import { DecoratorFunction } from '@storybook/addons';
import { StoryFnReactReturnType } from '@storybook/react/dist/client/preview/types';
import { StoryFnReactReturnType } from '@storybook/react/dist/ts3.9/client/preview/types';

export const FabricDecorator: DecoratorFunction<StoryFnReactReturnType> = story => (
<div style={{ display: 'flex' }}>
Expand Down
2 changes: 1 addition & 1 deletion apps/vr-tests/src/utilities/FluentProviderDecorator.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { FluentProvider } from '@fluentui/react-provider';
import { webLightTheme } from '@fluentui/react-theme';
import { DecoratorFunction } from '@storybook/addons';
import { StoryFnReactReturnType } from '@storybook/react/dist/client/preview/types';
import { StoryFnReactReturnType } from '@storybook/react/dist/ts3.9/client/preview/types';
import * as React from 'react';

export const FluentProviderDecorator: DecoratorFunction<StoryFnReactReturnType> = story => (
Expand Down
2 changes: 1 addition & 1 deletion apps/vr-tests/src/utilities/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ export interface IStoryConfig {
rtl?: boolean;
}

declare module '@storybook/addons/dist/types' {
declare module '@storybook/addons/dist/ts3.9/types' {
Hotell marked this conversation as resolved.
Show resolved Hide resolved
// eslint-disable-next-line @typescript-eslint/naming-convention
interface StoryApi<StoryFnReturnType = unknown> {
/** adds a story, but via VR Tests' addon which auto adds variants like RTL */
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "fix: replace class with className in mdx file",
"packageName": "@fluentui/react-examples",
"email": "tristan.watanabe@gmail.com",
"dependentChangeType": "patch"
}
22 changes: 12 additions & 10 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@
"rename-package": "node -r ./scripts/ts-node-register ./scripts/rename-package.ts",
"run:published": "node ./scripts/monorepo/runPublished.js",
"runto:lerna": "node ./scripts/monorepo/runTo.js",
"satisfied": "satisfied --skip-invalid --ignore \"sass|@testing-library|vue\"",
"satisfied": "satisfied --skip-invalid --ignore \"angular|builder-webpack5|sass|svelte|@testing-library|vue\"",
"scrub": "node ./scripts/scrub.js",
"start": "node scripts/start.js",
"start:legacy": "yarn workspace @fluentui/public-docsite-resources start",
Expand Down Expand Up @@ -82,16 +82,17 @@
"@nrwl/node": "12.1.0",
"@nrwl/tao": "12.1.0",
"@nrwl/workspace": "12.1.0",
"@storybook/addon-a11y": "6.0.28",
"@storybook/addon-actions": "6.0.28",
"@storybook/addon-docs": "6.0.28",
"@storybook/addon-essentials": "6.0.28",
"@storybook/addon-a11y": "6.2.9",
"@storybook/addon-actions": "6.2.9",
"@storybook/addon-docs": "6.2.9",
"@storybook/addon-essentials": "6.2.9",
"@storybook/addon-info": "6.0.0-alpha.2",
"@storybook/addon-knobs": "6.0.28",
"@storybook/addons": "6.0.28",
"@storybook/channels": "6.0.28",
"@storybook/core": "6.0.28",
"@storybook/react": "6.0.28",
"@storybook/addon-knobs": "6.2.9",
"@storybook/addon-postcss": "2.0.0",
"@storybook/addons": "6.2.9",
"@storybook/channels": "6.2.9",
"@storybook/core": "6.2.9",
"@storybook/react": "6.2.9",
"@testing-library/jest-dom": "5.11.9",
"@testing-library/react": "10.4.9",
"@testing-library/react-hooks": "5.0.3",
Expand All @@ -108,6 +109,7 @@
"@types/webpack-env": "1.16.0",
"@types/yargs": "13.0.11",
"ajv": "8.4.0",
"autoprefixer": "^9.7.2",
Copy link
Contributor

Choose a reason for hiding this comment

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

please use fixed versions in root devDependencies

"babel-plugin-annotate-pure-calls": "0.4.0",
"beachball": "1.53.1",
"chalk": "4.1.0",
Expand Down
4 changes: 4 additions & 0 deletions packages/react-examples/jest.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ const config = createConfig({
setupFiles: [path.resolve(path.join(__dirname, 'config', 'tests.js'))],

snapshotSerializers: [resolveMergeStylesSerializer()],

moduleNameMapper: {
'@storybook/addon-docs/blocks$': '@storybook/addon-docs/dist/cjs/blocks',
Copy link
Contributor

Choose a reason for hiding this comment

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

q: is this a know limitation (to explicitly provide CJS storybooks output) ? I mean we don't run tests on stories do we?

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure about the limitation but this is a known issue (see here) that's supposed to have been fixed in 6.3.x. Tests were written for the utils.tsx file in react-examples/react-components/Migrations (see file here) which caused the initial failure.

},
});

module.exports = config;
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,16 @@ import './intro.css';

<Meta title="Concepts/Introduction" />

<h1 class="fluent">Fluent UI React vNext</h1>
<h1 className="fluent">Fluent UI React vNext</h1>
Hotell marked this conversation as resolved.
Show resolved Hide resolved

<img src="./fluentui-banner2.jpg" alt="Hello3" />

> **⚠ NOT PRODUCTION READY COMPONENTS - API SURFACES MAY CHANGE WITHOUT NOTICE**

---

<h2 class="fluent">Overview</h2>
<div class="fluent-text">
<h2 className="fluent">Overview</h2>
<div className="fluent-text">

Fluent UI vNext is a set of UI components and utilities resulting from an effort to converge the set of React based component libraries in production today: `@fluentui/react` and `@fluentui/react-northstar`.

Expand All @@ -27,6 +27,6 @@ Each component is designed adhere to following attributes:

</div>

<h2 class="fluent">Questions?</h2>
<h2 className="fluent">Questions?</h2>

Reach out to the Fluent UI React team on [Github](https://github.com/microsoft/fluentui)
6 changes: 3 additions & 3 deletions packages/storybook/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,9 @@
"@fluentui/react-storybook": "^9.0.0-alpha.0",
"@fluentui/react": "^8.21.0",
"@fluentui/theme": "^2.1.3",
"@storybook/addon-knobs": "6.0.28",
"@storybook/addon-essentials": "6.0.28",
"@storybook/addons": "6.0.28",
"@storybook/addon-knobs": "6.2.9",
"@storybook/addon-essentials": "6.2.9",
"@storybook/addons": "6.2.9",
"@fluentui/azure-themes": "^8.1.29",
"@fluentui/theme-samples": "^8.1.29",
"tslib": "^2.1.0"
Expand Down
7 changes: 7 additions & 0 deletions postcss.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
module.exports = {
TristanWatanabe marked this conversation as resolved.
Show resolved Hide resolved
plugins: [
require('autoprefixer')({
flexbox: 'no-2009',
}),
],
};
2 changes: 1 addition & 1 deletion scripts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
"@types/puppeteer": "1.12.3",
"@types/read-pkg-up": "^6.0.0",
"async": "^2.6.1",
"autoprefixer": "^10.2.1",
Copy link
Member

Choose a reason for hiding this comment

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

Why the downgrade from autoprefixer 10 to 9? (looking at the root package.json)

Copy link
Member Author

Choose a reason for hiding this comment

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

Due to changes in 6.2 that required us to manually add autoprefixer as a dependency, I wanted to add the autoprefixer version that storybook used to automatically include which is v9. This was also an issue tracked here that was supposed to be solved by simply installing addons-postcss but clearly was not in this case.

image

Was also getting this error below when I used autoprefixer: 10.2.1:

image

Copy link
Member

Choose a reason for hiding this comment

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

autoprefixer 10 in scripts was the version that was actually being used on our production code, so at least in my opinion it would be best if we can keep using it. In the error above, it looks like the reason you're getting an error saying that the autoprefixer plugin requires postcss 8 (when that's the version we already have installed at the root) is because @storybook/builder-webpack4 must specify some different version of postcss--you can tell by the fact that the error path is under node_modules/@storybook/builder-webpack4/node_modules/postcss rather than just node_modules/postcss.
image

So the next thing to look at is whether there's a newer version of @storybook/builder-webpack4 (and/or @storybook/addon-info which apparently pulled it in, based on the stack), and see if that specifies an updated version of autoprefixer. Or if all else fails we can add a yarn resolution, or have multiple versions specified and add a syncpack policy exception. (It's also weird that latest storybook would depend on something which presumably uses webpack 4 to begin with...)

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 i agree about sticking to autoprefixer 10. Looking at the yarn.lock file, @storybook/builder-webpack4 has a dependency on postcss "^7.0.35". This isn't the case if we started using storybook/builder-webpack5 which was introduced in storybook 6.2 but that's still considered experimental so I didn't want to upgrade as that may introduce more issues. One of the issues is definitely us still using @storybook/addon-info which is deprecated as of storybook 6.0. Just to test things out, I removed storybook/addon-info and the react and converged packages started up just fine without it and the error involving it disappeared. However, trying to start storybook on react-components yielded a similar error (minus the addon-info error) but this one is seemingly rooted in the @storybook/builder-webpack4 module.

image

Copy link
Member

Choose a reason for hiding this comment

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

I'd really like to get off anything using webpack 4 if possible, because overall that's likely to introduce more problems since the rest of our repo is on webpack 5.

How are we using addon-info? Can we just remove it?

Copy link
Member

Choose a reason for hiding this comment

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

If you search for addon-info in the repo I'm only seeing a few references, none of which appear to relate to converged. And it looks like the only thing it's being used for is showing the source code of the stories in v8, which IMO is not critical. So I'd say just remove it for now, and if we need a way to see story source later we can figure that out separately.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok I'll remove addon-info and try to incorporate @storybook/builder-webpack5

Copy link
Member Author

Choose a reason for hiding this comment

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

Both tasks above have been completed

"autoprefixer": "^9.7.2",
TristanWatanabe marked this conversation as resolved.
Show resolved Hide resolved
"babel-jest": "^24.5.0",
"babel-plugin-annotate-pure-imports": "^1.0.0-1",
"babel-plugin-iife-wrap-react-components": "^1.0.0-5",
Expand Down
3 changes: 2 additions & 1 deletion scripts/tasks/storybook.ts
Original file line number Diff line number Diff line change
Expand Up @@ -49,9 +49,10 @@ function getCommonOptions(): StorybookCommonOptions {
console.log(`node heap limit = ${require('v8').getHeapStatistics().heap_size_limit / (1024 * 1024)} MB`);

const localConfigDir = path.join(process.cwd(), '.storybook');
const localStaticDir = path.join(process.cwd(), 'static');

return {
staticDir: [path.join(process.cwd(), 'static')],
staticDir: fs.existsSync(localStaticDir) ? [localStaticDir] : [],
Hotell marked this conversation as resolved.
Show resolved Hide resolved
configDir: fs.existsSync(localConfigDir)
? localConfigDir
: path.join(findGitRoot(), 'packages/react-examples/.storybook'),
Expand Down
Loading