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.

Sign up for GitHub

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: migrate @fluentui/react-components #20749

Merged
merged 15 commits into from
Nov 26, 2021
Merged

chore: migrate @fluentui/react-components #20749

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
8299f41
chore: migrate @fluentui/react-components
layershifter Nov 24, 2021
00da790
Change files
layershifter Nov 24, 2021
4e78153
restore script
layershifter Nov 24, 2021
f2f5f26
restore changes
layershifter Nov 24, 2021
f4359dd
fix config
layershifter Nov 24, 2021
7ccebc6
update configs to include both JS & TS
layershifter Nov 24, 2021
cbecc87
fix typings errors
layershifter Nov 24, 2021
13cb126
fix SB config
layershifter Nov 24, 2021
3a7fd31
Merge branch 'master' of https://github.com/microsoft/fluentui into c…
layershifter Nov 26, 2021
0047238
bump pkg version
layershifter Nov 26, 2021
f972dcf
Apply suggestions from code review
layershifter Nov 26, 2021
135f847
fix fmt
layershifter Nov 26, 2021
a6738f9
Merge branch 'master' of https://github.com/microsoft/fluentui into c…
layershifter Nov 26, 2021
95fe527
run generator once more
layershifter Nov 26, 2021
c24a859
apply review suggestions
layershifter Nov 26, 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
7 changes: 7 additions & 0 deletions change/@fluentui-react-components-6fb4276e-8a87-493f-933b-0e128524e932.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "none",
"comment": "update tooling configs",
"packageName": "@fluentui/react-components",
"email": "[email protected]",
"dependentChangeType": "none"
}
3 changes: 3 additions & 0 deletions packages/react-components/.babelrc.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"plugins": ["module:@fluentui/babel-make-styles", "annotate-pure-calls", "@babel/transform-react-pure-annotations"]
}
1 change: 0 additions & 1 deletion packages/react-components/.npmignore
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
.cache/
.storybook/
.vscode/
bundle-size/
Expand Down
17 changes: 6 additions & 11 deletions packages/react-components/.storybook/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,14 @@
const rootMain = require('../../../.storybook/main');

const utils = require('./main.utils');

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

// add your own webpack tweaks if needed

return localConfig;
},
previewHead: rootMain.previewHead,
});
1 change: 0 additions & 1 deletion packages/react-components/.storybook/preview.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export const parameters = {
...rootPreview.parameters.docs,
container: FluentDocsContainer,
page: FluentDocsPage,
// container: FluentDocsContainer,
},
options,
};
17 changes: 14 additions & 3 deletions packages/react-components/.storybook/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,20 @@
{
"extends": "../tsconfig.json",
"compilerOptions": {
"outDir": "",
"allowJs": true,
"checkJs": true
"checkJs": true,
"types": ["static-assets", "environment", "inline-style-expand-shorthand", "storybook__addons"]
},
"exclude": ["../**/*.test.ts", "../**/*.test.js", "../**/*.test.tsx", "../**/*.test.jsx"],
"include": ["../src/**/*", "*.js"]
"exclude": [
"**/*.spec.ts",
"**/*.spec.tsx",
"**/*.test.ts",
"**/*.test.tsx",
"**/*.spec.js",
"**/*.spec.jsx",
"**/*.test.js",
"**/*.test.jsx"
],
"include": ["../src/**/*.stories.ts", "../src/**/*.stories.tsx", "*.js"]
}
2 changes: 1 addition & 1 deletion packages/react-components/config/api-extractor.local.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"$schema": "https://developer.microsoft.com/json-schemas/api-extractor/v7/api-extractor.schema.json",
"extends": "./api-extractor.json",
"mainEntryPointFilePath": "<projectFolder>/dist/<unscopedPackageName>/src/index.d.ts",
"mainEntryPointFilePath": "<projectFolder>/dist/packages/<unscopedPackageName>/src/index.d.ts",
"compiler": {
/**
* This is a quickfix to make build:local work
Expand Down
2 changes: 1 addition & 1 deletion packages/react-components/jest.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module.exports = {
preset: '../../jest.preset.js',
globals: {
'ts-jest': {
tsConfig: '<rootDir>/tsconfig.json',
tsConfig: '<rootDir>/tsconfig.spec.json',
diagnostics: false,
},
},
Expand Down
8 changes: 5 additions & 3 deletions packages/react-components/package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,11 +21,12 @@
"lint": "just-scripts lint",
"start": "yarn storybook",
"docs": "api-extractor run --config=config/api-extractor.local.json --local",
"build:local": "tsc -p . --module esnext --emitDeclarationOnly && node ../../scripts/typescript/normalize-import --output dist/react-components/src && yarn docs",
"build:local": "tsc -p ./tsconfig.lib.json --module esnext --emitDeclarationOnly && node ../../scripts/typescript/normalize-import --output ./dist/packages/react-components/src && yarn docs",
"storybook": "start-storybook --port 3000 -s ./public",
"storybook:docs": "start-storybook --port 3000 -s ./public --docs --no-manager-cache",
"build-storybook": "build-storybook -s ./public -o ./dist/storybook --docs",
"test": "jest"
"test": "jest",
"type-check": "tsc -b tsconfig.json"
},
"devDependencies": {
"@fluentui/eslint-plugin": "*",
Expand All @@ -34,7 +35,8 @@
"@types/react": "16.9.42",
"@types/react-dom": "16.9.10",
"react": "16.8.6",
"react-dom": "16.8.6"
"react-dom": "16.8.6",
"@fluentui/babel-make-styles": "9.0.0-beta.3"
},
"dependencies": {
"@fluentui/react-accordion": "9.0.0-beta.4",
Expand Down
7 changes: 0 additions & 7 deletions packages/react-components/src/AccessibilityScenarios/Menu.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,12 +1,5 @@
import * as React from 'react';

// https://github.com/microsoft/fluentui/pull/18695#issuecomment-868432982
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
import { Button } from '@fluentui/react-button';
// https://github.com/microsoft/fluentui/pull/18695#issuecomment-868432982
// eslint-disable-next-line @typescript-eslint/ban-ts-comment
// @ts-ignore
import {
Menu,
MenuTrigger,
Expand Down
13 changes: 10 additions & 3 deletions ...s/react-components/src/Concepts/Positioning/PositioningPopperImperativeHandle.stories.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,16 @@ import { PositioningProps } from '@fluentui/react-positioning';
export const PopperImperativeHandle = () => {
const [loading, setLoading] = React.useState(true);
const popperRef: PositioningProps['popperRef'] = React.useRef({ updatePosition: () => null });
const timeoutRef = React.useRef(0);
const timeoutRef = React.useRef<ReturnType<typeof setTimeout>>();
Copy link
Member Author

@layershifter layershifter Nov 24, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

@Hotell Now Node typings are leaking to stories, not sure that it's okay 😯 Is it happens because non all packages were migrated yet?

Copy link
Contributor

Choose a reason for hiding this comment

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

so because the custom logic that is being provided in root /.storybook and in react-components - (main.utils.js), which uses node APIs, node typings are being included to the checking program ( which is how TS works ). At this point there is no functionality that allows us to prevent that.

Only solution is to be explicit and stop doing antipatterns like accesing globals directly.

Instead:

timeoutRef.current = setTimeout(() => setLoading(false), 1000);

Do:

timeoutRef.current = window.setTimeout(() => setLoading(false), 1000);

This will resolve the type issues you're having.

Another thingy:

Now because I introduced tests in react-components/.storybook , we would need to add jest glboals to react-components/.storybook/tsconfig.json. That will pollute all stories, thus allowing anyone to use jest globals in stories. I don't think that's the proper solution ATM, so instead we can do following for that test file:

@filename react-components/.storybook/main.utils.test.js

+/// <reference types="jest" />
const utils = require('./main.utils');

describe(`main utils`, () => {

Copy link
Member Author

Choose a reason for hiding this comment

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

Is using <reference types="jest" /> is a practice that we should follow in such cases?

Copy link
Contributor

Choose a reason for hiding this comment

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

ideal scenario is to have this in separate package (logic,tests) with proper globals set. but as a temporary workaround/corner case I think so, yes.

Why?

  • tests are not shipped not exported so not used in some other context, thus it is ok to specify local globals inline, if not specified on package level

Copy link
Contributor

Choose a reason for hiding this comment

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

also going forward ( once we migrate to jest 27 ) we should stop using globals as jest provides imports for their API ( one less global in our lives yay )


const onOpenChange: PopoverProps['onOpenChange'] = (e, data) => {
if (!data.open) {
setLoading(true);
} else {
clearTimeout(timeoutRef.current);
if (timeoutRef.current) {
clearTimeout(timeoutRef.current);
}

timeoutRef.current = setTimeout(() => setLoading(false), 1000);
}
};
Expand All @@ -24,7 +27,11 @@ export const PopperImperativeHandle = () => {
}, [loading]);

React.useEffect(() => {
return () => clearTimeout(timeoutRef.current);
return () => {
if (timeoutRef.current) {
clearTimeout(timeoutRef.current);
}
};
});

return (
Expand Down
29 changes: 19 additions & 10 deletions packages/react-components/tsconfig.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,17 +1,26 @@
{
"extends": "../../tsconfig.base.json",
"include": ["src"],
"compilerOptions": {
"target": "ES2019",
"module": "CommonJS",
"lib": ["ES2019", "DOM"],
"outDir": "dist",
"jsx": "react",
"declaration": true,
"experimentalDecorators": true,
"noEmit": true,
"isolatedModules": true,
"importHelpers": true,
"jsx": "react",
"noUnusedLocals": true,
"preserveConstEnums": true,
"types": ["jest", "custom-global", "inline-style-expand-shorthand", "storybook__addons"]
}
"allowJs": true,
"checkJs": true
},
"include": [],
"files": [],
"references": [
{
"path": "./tsconfig.lib.json"
},
{
"path": "./tsconfig.spec.json"
},
{
"path": "./.storybook/tsconfig.json"
}
]
}
25 changes: 25 additions & 0 deletions packages/react-components/tsconfig.lib.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"noEmit": false,
"lib": ["ES2019", "dom"],
"outDir": "dist",
"declaration": true,
"types": ["static-assets", "environment", "inline-style-expand-shorthand"]
},
"exclude": [
"**/*.spec.ts",
"**/*.spec.tsx",
"**/*.test.ts",
"**/*.test.tsx",
"**/*.spec.js",
"**/*.spec.jsx",
"**/*.test.js",
"**/*.test.jsx",
"**/*.stories.ts",
"**/*.stories.tsx",
"**/*.stories.js",
"**/*.stories.jsx"
],
"include": ["./src/**/*.ts", "./src/**/*.tsx", "./src/**/*.js", "./src/**/*.jsx"]
}
20 changes: 20 additions & 0 deletions packages/react-components/tsconfig.spec.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,20 @@
{
"extends": "./tsconfig.json",
"compilerOptions": {
"module": "CommonJS",
"outDir": "dist",
"types": ["jest", "node", "inline-style-expand-shorthand"]
},
"include": [
"**/*.spec.ts",
"**/*.spec.tsx",
"**/*.test.ts",
"**/*.test.tsx",
"**/*.d.ts",
"**/*.spec.js",
"**/*.spec.jsx",
"**/*.test.js",
"**/*.test.jsx",
"**/*.d.ts"
]
}
3 changes: 2 additions & 1 deletion packages/react-storybook-addon/src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export type { FluentGlobals, FluentStoryContext } from './hooks';
export { themes, ThemeIds } from './theme';
export { themes } from './theme';
export type { ThemeIds } from './theme';
export { THEME_ID } from './constants';
5 changes: 5 additions & 0 deletions typings/storybook__addons/index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
// TypeScript Version: 3.1

import { ViewMode } from '@storybook/addons';
import * as React from 'react';

declare module '@storybook/addons' {
// PUBLIC API - extended definitions
Expand Down Expand Up @@ -35,6 +36,10 @@ declare module '@storybook/addons' {
*/
excludeDecorators: boolean;
};

container: React.ComponentType<any>;
page: React.ComponentType<any>;

/**
* https://github.com/storybookjs/storybook/tree/next/addons/docs/react#inline-stories
*/
Expand Down