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

chore: implement new storybook architecture - for converged packages/react-menu #17866

Merged
merged 6 commits into from
Apr 30, 2021
Merged
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: 4 additions & 0 deletions .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,17 @@
# docs/ [email protected]

#### Build stuff
/.storybook/ @microsoft/fluentui-react-build
scripts/ @microsoft/fluentui-react-build
package.json @microsoft/fluentui-react-build
yarn.lock @microsoft/fluentui-react-build
*.config.js @microsoft/fluentui-react-build
*.sh @microsoft/fluentui-react-build
*.yml @microsoft/fluentui-react-build
tsconfig.json @microsoft/fluentui-react-build
/tsconfig.base.json @microsoft/fluentui-react-build
/jest.config.js @microsoft/fluentui-react-build
/jest.preset.js @microsoft/fluentui-react-build
lerna.json @microsoft/fluentui-react-build
.codesandbox @microsoft/fluentui-react-build
.devops @microsoft/fluentui-react-build
Expand Down
38 changes: 38 additions & 0 deletions .storybook/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
const path = require('path');
const { TsconfigPathsPlugin } = require('tsconfig-paths-webpack-plugin');

/**
* @callback StorybookWebpackConfig
* @param {import("webpack").Configuration} config
* @param {{configType: 'DEVELOPMENT' | 'PRODUCTION'}} options - change the build configuration. 'PRODUCTION' is used when building the static version of storybook.
* @returns {import("webpack").Configuration}
*/

/**
* @typedef {{check:boolean; checkOptions: Record<string,unknown>; reactDocgen: string | boolean; reactDocgenTypescriptOptions: Record<string,unknown>}} StorybookTsConfig
*/

/**
* @typedef {{stories: string[] ; addons: string[]; typescript: StorybookTsConfig; babel: (options:Record<string,unknown>)=>Promise<Record<string,unknown>>; webpackFinal: StorybookWebpackConfig}} StorybookConfig
*/

module.exports = /** @type {Pick<StorybookConfig,'addons' |'stories' |'webpackFinal'>} */ ({
stories: [],
addons: [
'@storybook/addon-essentials',
'@storybook/addon-a11y',
'@storybook/addon-knobs/preset',
'storybook-addon-performance',
Copy link
Member

Choose a reason for hiding this comment

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

would we be OK reming the perf addon ? I have never seen it mentioned nor have I ever used it in any meaningful way

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we can do that, depends on requirements. It is used for react-examples that's why I added it here as well.

My understanding was that this is gonna be used for re-render perf metrics in the future ?

Copy link
Member

@ling1726 ling1726 Apr 29, 2021

Choose a reason for hiding this comment

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

re-render perf metrics

wasn't that why we created flamegrill ? 🤦‍♂️

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no idea

],
webpackFinal: config => {
const tsPaths = new TsconfigPathsPlugin({
configFile: path.resolve(__dirname, '../tsconfig.base.json'),
});

if (config.resolve) {
config.resolve.plugins ? config.resolve.plugins.push(tsPaths) : (config.resolve.plugins = [tsPaths]);
}

return config;
},
});
3 changes: 3 additions & 0 deletions .storybook/preview.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import { withFluentProvider, withStrictMode } from '@fluentui/react-storybook';

export const decorators = [withFluentProvider, withStrictMode];
9 changes: 9 additions & 0 deletions .storybook/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": "../tsconfig.base.json",
"compilerOptions": {
"noEmit": true,
"allowJs": true,
"checkJs": true,
"jsx": "preserve"
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "chore(react-examples): process collocated stories properly and remove react-menu from dependencies",
"packageName": "@fluentui/react-examples",
"email": "[email protected]",
"dependentChangeType": "patch"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

not really sure about the type, I'd say this is a breaking change hmm, but from what I remember, this package is not being used by anyone ?

Copy link
Member

Choose a reason for hiding this comment

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

Not sure about used, but it is published

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "chore(react-menu): implement local storybook with stories",
"packageName": "@fluentui/react-menu",
"email": "[email protected]",
"dependentChangeType": "none"
}
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -80,9 +80,9 @@
"@storybook/channels": "6.0.28",
"@storybook/core": "6.0.28",
"@storybook/react": "6.0.28",
"@testing-library/jest-dom": "5.11.9",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

reordered packages by yarn add

"@testing-library/react": "10.4.9",
"@testing-library/react-hooks": "5.0.3",
"@testing-library/jest-dom": "5.11.9",
"@types/copy-webpack-plugin": "6.4.0",
"@types/jest": "24.9.1",
"@types/jest-axe": "3.2.2",
Expand All @@ -101,9 +101,9 @@
"css-loader": "5.0.1",
"cypress": "6.6.0",
"cypress-real-events": "1.2.0",
"eslint-plugin-es": "4.1.0",
"chalk": "2.4.2",
"danger": "^6.0.5",
"eslint-plugin-es": "4.1.0",
"file-loader": "6.2.0",
"gulp": "^4.0.2",
"html-webpack-plugin": "5.1.0",
Expand All @@ -129,8 +129,9 @@
"syncpack": "^5.6.10",
"ts-jest": "24.3.0",
"ts-loader": "8.0.14",
"webpack": "5.21.2",
"tsconfig-paths-webpack-plugin": "3.5.1",
"typescript": "4.1.5",
"webpack": "5.21.2",
"webpack-cli": "4.3.1",
"webpack-dev-server": "4.0.0-beta.0",
"tmp": "0.2.1",
Expand Down
18 changes: 17 additions & 1 deletion packages/react-examples/.storybook/preview-loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,23 @@ export default function loader(source) {
// get unscoped dep names
const reactDeps = Object.keys(reactPackageJson.dependencies).map(d => d.split('/')[1] || d);
const reactDepsWithExamples = packagesWithExamples.filter(p => reactDeps.includes(p));
source = source.replace(/REACT_DEPS/g, reactDepsWithExamples.join('|'));

// @TODO
// - this is a temporary solution until all converged packages use new storybook configuration
// - after new config is in place remove this whole IF
//
// NOTE:
// - if we run storybook for react-components we wanna include all possible package collocated stories
// based on react-components package.json
if (packageName === 'react-components') {
const _convergedDependencies = reactDeps.filter(dependencyName => {
return dependencyName.startsWith('react-');
});

reactDepsWithExamples.push(..._convergedDependencies);
}

source = source.replace(/REACT_DEPS/g, [...new Set(reactDepsWithExamples)].join('|'));
}

return source;
Expand Down
124 changes: 97 additions & 27 deletions packages/react-examples/.storybook/preview.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,12 @@ import { withInfo } from '@storybook/addon-info';
import { withPerformance } from 'storybook-addon-performance';
import { withFluentProvider, withKeytipLayer, withStrictMode } from '@fluentui/storybook';

/**
* "PACKAGE_NAME" placeholder is being replaced by webpack loader - @link {./preview.loader}
* @type {string}
*/
const packageNamePlaceholder = 'PACKAGE_NAME';

addDecorator(withInfo);
addDecorator(withPerformance);
addDecorator(withKeytipLayer);
Expand Down Expand Up @@ -36,7 +42,7 @@ export const parameters = {
* NOTE:
* - this is a temporary workaround until we migrate to new storybook 6 APIs -> old `addDecorator` duplicates rendered decorators
* - source of this function is interpolated during runtime with webpack
* - "PACKAGE_NAME" placeholder is being replaced
*
*/
function addCustomDecorators() {
/**
Expand All @@ -46,7 +52,7 @@ function addCustomDecorators() {

if (
['react-button', 'react-cards', 'react-checkbox', 'react-slider', 'react-tabs', 'react-toggle'].includes(
'PACKAGE_NAME',
packageNamePlaceholder,
)
) {
initializeIcons();
Expand All @@ -66,7 +72,7 @@ function addCustomDecorators() {
'react-text',
'react-components',
'react-portal',
].includes('PACKAGE_NAME')
].includes(packageNamePlaceholder)
) {
customDecorators.add(withFluentProvider).add(withStrictMode);
}
Expand Down Expand Up @@ -109,15 +115,22 @@ function loadStories() {
require.context('../src/PACKAGE_NAME', true, /\.(Example|stories)\.tsx$/),
];

// @ts-ignore -- PACKAGE_NAME is replaced by a loader
if ('PACKAGE_NAME' === 'react' || 'PACKAGE_NAME' === 'react-components') {
if (packageNamePlaceholder === 'react' || packageNamePlaceholder === 'react-components') {
// For suite package storybooks, also show the examples of re-exported component packages.
// preview-loader will replace REACT_ DEPS with the actual list.
contexts.push(
require.context('../src', true, /(REACT_DEPS|PACKAGE_NAME)\/\w+\/[\w.]+\.(Example|stories)\.(tsx|mdx)$/),
);
}

// @TODO
// - this is a temporary solution until all converged packages use new storybook configuration
// - after new config is in place remove this whole IF
if (packageNamePlaceholder === 'react-components') {
// include package collocated stories within react-components
contexts.push(require.context('../../', true, /(REACT_DEPS)\/src\/[\w./]+\.(Example|stories)\.(tsx|mdx)$/));
}

for (const req of contexts) {
req.keys().forEach(key => {
generateStoriesFromExamples(key, stories, req);
Expand Down Expand Up @@ -150,8 +163,11 @@ function generateStoriesFromExamples(key, stories, req) {
// Depending on the starting point of the context, and the package layout, the key will be like one of these:
// ./ComponentName/ComponentName.Something.Example.tsx
// ./package-name/ComponentName/ComponentName.Something.Example.tsx
// ./package-name/src/.../ComponentName.stories.tsx - @TODO remove this line after new storybook setup has been applied for all converged packages
const segments = key.split('/');

if (segments.length < 3) {
console.warn(`Invalid storybook context location found: key: ${key} | segments: ${segments}`);
return;
}

Expand All @@ -161,28 +177,7 @@ function generateStoriesFromExamples(key, stories, req) {
return;
}

/** @type {string} */
let componentName;

// Story URLs are generated based off the story name
// In the case of `react-components` a (package name) suffix is added to each story
// This results in a difference name and URL between individual storybooks and the react-components suite storyboo
// https://storybook.js.org/docs/react/configure/sidebar-and-urls#permalinking-to-stories
// Use the id property in stories to ensure the same URL between individual and suite storyboo
/** @type {string} */
let componentId;

if (segments.length === 3) {
// ./ComponentName/ComponentName.Something.Example.tsx
componentName = segments[1];
componentId = segments[1];
} else {
// ./package-name/ComponentName/ComponentName.Something.Example.tsx
// For @fluentui/react, don't include the package name in the sidebar
// @ts-ignore -- PACKAGE_NAME is replaced by a loader
componentName = 'PACKAGE_NAME' === 'react' ? segments[2] : `${segments[2]} (${segments[1]})`;
componentId = segments[2];
}
const { componentName, componentId } = generateComponentName(segments);

if (!stories.has(componentName)) {
stories.set(componentName, {
Expand Down Expand Up @@ -220,4 +215,79 @@ function generateStoriesFromExamples(key, stories, req) {
}
}
}

/**
*
* @param {string[]} segments
* @returns {{componentName:string; componentId:string}}
*/
function generateComponentName(segments) {
/**
* @TODO
* - this is a temporary solution until all converged packages use new storybook configuration
* - after new config is in place remove this
*
* ./<package-name>/src/.../ComponentName.Something.Example.tsx
*/
const isCollocatedStory = segments.includes('src');

/**
* ./ComponentName/ComponentName.Something.Example.tsx
*/
const isReactExamplesStory = segments.length === 3;

/**
* For @fluentui/react, don't include the package name in the sidebar
* ./package-name/ComponentName/ComponentName.Something.Example.tsx
*/
// @ts-ignore -- PACKAGE_NAME is replaced by a loader
const isReactPackageStory = 'PACKAGE_NAME' === 'react';

// @TODO
// - this is a temporary solution until all converged packages use new storybook configuration
// - after new config is in place remove this whole IF
if (isCollocatedStory) {
// ./<package-name>/src/.../ComponentName.Something.Example.tsx
// ↓↓↓
// [., <package-name>, src, ..., ComponentName, ComponentName.Something.Example.tsx]
const packageName = segments[1];
const storyFileName = segments[segments.length - 1];
const [, storyName] = /(\w+)\.(Example|stories)\.(tsx|mdx)$/.exec(storyFileName) || [];

const componentName = `${storyName} (${packageName})`;
const componentId = storyName;

return { componentName, componentId };
}

if (isReactExamplesStory) {
// ./ComponentName/ComponentName.Something.Example.tsx
// ↓↓↓
// [., ComponentName, ComponentName.Something.Example.tsx]
const componentName = segments[1];
const componentId = segments[1];

return { componentName, componentId };
}

if (isReactPackageStory) {
// ./package-name/ComponentName/ComponentName.Something.Example.tsx
// ↓↓↓
// [., <package-name>, ComponentName, ComponentName.Something.Example.tsx]
const componentName = segments[1];
const componentId = segments[2];

return { componentName, componentId };
}

return {
componentName: `${segments[2]} (${segments[1]})`,
// Story URLs are generated based off the story name
// In the case of `react-components` a (package name) suffix is added to each story
// This results in a difference name and URL between individual storybooks and the react-components suite storybook
// https://storybook.js.org/docs/react/configure/sidebar-and-urls#permalinking-to-stories
// Use the id property in stories to ensure the same URL between individual and suite storybook
componentId: segments[2],
};
}
}
1 change: 0 additions & 1 deletion packages/react-examples/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,6 @@
"@fluentui/react-image": "^9.0.0-alpha.30",
"@fluentui/react-link": "^9.0.0-alpha.30",
"@fluentui/react-make-styles": "^9.0.0-alpha.29",
"@fluentui/react-menu": "^9.0.0-alpha.16",
"@fluentui/react-portal": "^9.0.0-alpha.6",
"@fluentui/react-provider": "^9.0.0-alpha.30",
"@fluentui/react-slider": "^1.0.0-beta.86",
Expand Down
11 changes: 11 additions & 0 deletions packages/react-menu/.storybook/main.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
const rootMain = require('../../../.storybook/main');

module.exports = /** @type {Pick<import('../../../.storybook/main').StorybookConfig,'addons'|'stories'|'webpackFinal'>} */ ({
stories: [...rootMain.stories, '../src/**/*.stories.mdx', '../src/**/*.stories.@(ts|tsx)'],
addons: [...rootMain.addons],
webpackFinal: (config, options) => {
const localConfig = { ...rootMain.webpackFinal(config, options) };

return localConfig;
},
});
3 changes: 3 additions & 0 deletions packages/react-menu/.storybook/preview.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import * as rootPreview from '../../../.storybook/preview';

export const decorators = [...rootPreview.decorators];
9 changes: 9 additions & 0 deletions packages/react-menu/.storybook/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
{
"extends": "../tsconfig.json",
"compilerOptions": {
"allowJs": true,
"checkJs": true
},
"exclude": ["../**/*.test.ts", "../**/*.test.js", "../**/*.test.tsx", "../**/*.test.jsx"],
"include": ["../src/**/*", "*.js"]
}
3 changes: 2 additions & 1 deletion packages/react-menu/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,8 @@
"e2e": "node e2e.js",
"just": "just-scripts",
"lint": "just-scripts lint",
"start": "just-scripts dev:storybook",
"start": "echo \"This is DEPRECATED instead use 'storybook'\" && just-scripts dev:storybook",
"storybook": "start-storybook",
"start-test": "echo \"This is DEPRECATED instead use 'test --watch'\" && just-scripts jest-watch",
"test": "jest",
"update-snapshots": "echo \"This is DEPRECATED instead use 'test -u'\" && just-scripts jest -u"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import {
MenuDivider,
MenuGroupHeader,
MenuProps,
} from '@fluentui/react-menu';
import { CutIcon, PasteIcon, EditIcon, AcceptIcon } from '@fluentui/react-icons-mdl2';
} from './index';
import { boolean } from '@storybook/addon-knobs';

import { CutIcon, PasteIcon, EditIcon, AcceptIcon } from './tmp-icons.stories';

export const TextOnly = (props: Pick<MenuProps, 'openOnHover' | 'openOnContext' | 'defaultOpen'>) => (
<Menu openOnHover={props.openOnHover} openOnContext={props.openOnContext} defaultOpen={props.defaultOpen}>
<MenuTrigger>
Expand Down Expand Up @@ -236,3 +237,8 @@ export const SelectionGroup = () => (
</MenuList>
</Menu>
);

export default {
title: 'Menu',
component: Menu,
};
Loading