From c35a00d4d63fe7fc4dc86ee9877da9f1f82b8d66 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sun, 24 Nov 2024 01:17:19 +0100 Subject: [PATCH 1/2] [core] Remove useless fragments --- .../ApiBuilders/ComponentApiBuilder.ts | 93 +++++++++---------- .../ClickAwayListener/ClickAwayListener.tsx | 2 +- packages/mui-material/src/NoSsr/NoSsr.tsx | 1 - 3 files changed, 45 insertions(+), 51 deletions(-) diff --git a/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts b/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts index 9d223108cd4d7f..67520133ad1e12 100644 --- a/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts +++ b/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts @@ -628,58 +628,53 @@ export default async function generateComponentApi( const filename = componentInfo.filename; let reactApi: ComponentReactApi; - if (componentInfo.isSystemComponent || componentInfo.name === 'Grid2') { - try { - reactApi = docgenParse( - src, - (ast) => { - let node; - astTypes.visit(ast, { - visitVariableDeclaration: (variablePath) => { - const definitions: any[] = []; - if (variablePath.node.declarations) { - variablePath - .get('declarations') - .each((declarator: any) => definitions.push(declarator.get('init'))); - } - - definitions.forEach((definition) => { - // definition.value.expression is defined when the source is in TypeScript. - const expression = definition.value?.expression - ? definition.get('expression') - : definition; - if (expression.value?.callee) { - const definitionName = expression.value.callee.name; - - if (definitionName === `create${componentInfo.name}`) { - node = expression; - } - } - }); - - return false; - }, - }); - - return node; - }, - defaultHandlers, - { filename }, - ); - } catch (error) { - // fallback to default logic if there is no `create*` definition. - if ((error as Error).message === 'No suitable component definition found.') { - reactApi = docgenParse(src, null, defaultHandlers.concat(muiDefaultPropsHandler), { - filename, - }); - } else { - throw error; - } - } - } else { + try { reactApi = docgenParse(src, null, defaultHandlers.concat(muiDefaultPropsHandler), { filename, }); + } catch (error) { + // fallback to default logic if there is no `create*` definition. + if ((error as Error).message === 'No suitable component definition found.') { + reactApi = docgenParse(src, (ast) => { + let node; + // TODO migrate to react-docgen v6, using Babel AST now + astTypes.visit(ast, { + visitFunctionDeclaration: (path) => { + if (path.node.params[0].name === 'props') { + node = path; + } + return false; + }, + visitVariableDeclaration: (path) => { + const definitions: any[] = []; + if (path.node.declarations) { + path + .get('declarations') + .each((declarator: any) => definitions.push(declarator.get('init'))); + } + definitions.forEach((definition) => { + // definition.value.expression is defined when the source is in TypeScript. + const expression = definition.value?.expression + ? definition.get('expression') + : definition; + if (expression.value?.callee) { + const definitionName = expression.value.callee.name; + if (definitionName === `create${componentInfo.name}`) { + node = expression; + } + } + }); + return false; + }, + }); + + return node; + }, defaultHandlers, { + filename, + }); + } else { + throw error; + } } if (!reactApi.props) { diff --git a/packages/mui-material/src/ClickAwayListener/ClickAwayListener.tsx b/packages/mui-material/src/ClickAwayListener/ClickAwayListener.tsx index 6a695e84f78352..5987a8f04dc1c8 100644 --- a/packages/mui-material/src/ClickAwayListener/ClickAwayListener.tsx +++ b/packages/mui-material/src/ClickAwayListener/ClickAwayListener.tsx @@ -210,7 +210,7 @@ function ClickAwayListener(props: ClickAwayListenerProps): React.JSX.Element { return undefined; }, [handleClickAway, mouseEvent]); - return {React.cloneElement(children, childrenProps)}; + return React.cloneElement(children, childrenProps); } ClickAwayListener.propTypes /* remove-proptypes */ = { diff --git a/packages/mui-material/src/NoSsr/NoSsr.tsx b/packages/mui-material/src/NoSsr/NoSsr.tsx index c7e2dfbc191a6e..52b2b04cf74997 100644 --- a/packages/mui-material/src/NoSsr/NoSsr.tsx +++ b/packages/mui-material/src/NoSsr/NoSsr.tsx @@ -38,7 +38,6 @@ function NoSsr(props: NoSsrProps): React.JSX.Element { } }, [defer]); - // We need the Fragment here to force react-docgen to recognise NoSsr as a component. return {mountedState ? children : fallback}; } From 16b88687e5ffeebe591126e0892891a0fad89e65 Mon Sep 17 00:00:00 2001 From: Olivier Tassinari Date: Sun, 24 Nov 2024 01:29:04 +0100 Subject: [PATCH 2/2] one step further --- .../templates/shared-theme/AppTheme.js | 3 +- .../templates/shared-theme/AppTheme.tsx | 7 +- docs/pages/joy-ui/api/stack.json | 8 +- docs/pages/material-ui/api/container.json | 7 +- docs/pages/material-ui/api/grid-2.json | 14 ++-- docs/pages/material-ui/api/stack.json | 8 +- docs/pages/system/api/container.json | 7 +- docs/pages/system/api/grid.json | 14 ++-- docs/pages/system/api/stack.json | 8 +- .../ApiBuilders/ComponentApiBuilder.ts | 76 ++++++++++--------- .../utils/defaultPropsHandler.ts | 5 ++ .../utils/getPropsFromComponentNode.ts | 1 + .../ClickAwayListener/ClickAwayListener.tsx | 2 +- packages/mui-base/src/NoSsr/NoSsr.tsx | 4 +- packages/mui-base/src/Portal/Portal.tsx | 8 +- packages/mui-material/src/Hidden/HiddenJs.js | 3 +- packages/mui-material/src/NoSsr/NoSsr.tsx | 3 +- packages/mui-material/src/Portal/Portal.tsx | 8 +- 18 files changed, 102 insertions(+), 84 deletions(-) diff --git a/docs/data/material/getting-started/templates/shared-theme/AppTheme.js b/docs/data/material/getting-started/templates/shared-theme/AppTheme.js index b1d55ca4408a14..6fde1fc9fd9be3 100644 --- a/docs/data/material/getting-started/templates/shared-theme/AppTheme.js +++ b/docs/data/material/getting-started/templates/shared-theme/AppTheme.js @@ -9,7 +9,8 @@ import { navigationCustomizations } from './customizations/navigation'; import { surfacesCustomizations } from './customizations/surfaces'; import { colorSchemes, typography, shadows, shape } from './themePrimitives'; -function AppTheme({ children, disableCustomTheme, themeComponents }) { +function AppTheme(props) { + const { children, disableCustomTheme, themeComponents } = props; const theme = React.useMemo(() => { return disableCustomTheme ? {} diff --git a/docs/data/material/getting-started/templates/shared-theme/AppTheme.tsx b/docs/data/material/getting-started/templates/shared-theme/AppTheme.tsx index 1fe35d6faeef48..a4a512c30aef59 100644 --- a/docs/data/material/getting-started/templates/shared-theme/AppTheme.tsx +++ b/docs/data/material/getting-started/templates/shared-theme/AppTheme.tsx @@ -17,11 +17,8 @@ interface AppThemeProps { themeComponents?: ThemeOptions['components']; } -export default function AppTheme({ - children, - disableCustomTheme, - themeComponents, -}: AppThemeProps) { +export default function AppTheme(props: AppThemeProps) { + const { children, disableCustomTheme, themeComponents } = props; const theme = React.useMemo(() => { return disableCustomTheme ? {} diff --git a/docs/pages/joy-ui/api/stack.json b/docs/pages/joy-ui/api/stack.json index 008afff7737bbc..8826d1eebc8a85 100644 --- a/docs/pages/joy-ui/api/stack.json +++ b/docs/pages/joy-ui/api/stack.json @@ -6,14 +6,16 @@ "type": { "name": "union", "description": "'column-reverse'
| 'column'
| 'row-reverse'
| 'row'
| Array<'column-reverse'
| 'column'
| 'row-reverse'
| 'row'>
| object" - } + }, + "default": "'column'" }, "divider": { "type": { "name": "node" } }, "spacing": { "type": { "name": "union", "description": "Array<number
| string>
| number
| object
| string" - } + }, + "default": "0" }, "sx": { "type": { @@ -22,7 +24,7 @@ }, "additionalInfo": { "sx": true } }, - "useFlexGap": { "type": { "name": "bool" } } + "useFlexGap": { "type": { "name": "bool" }, "default": "false" } }, "name": "Stack", "imports": ["import Stack from '@mui/joy/Stack';", "import { Stack } from '@mui/joy';"], diff --git a/docs/pages/material-ui/api/container.json b/docs/pages/material-ui/api/container.json index 29b78f807d6157..0f60f58e920597 100644 --- a/docs/pages/material-ui/api/container.json +++ b/docs/pages/material-ui/api/container.json @@ -2,13 +2,14 @@ "props": { "classes": { "type": { "name": "object" }, "additionalInfo": { "cssApi": true } }, "component": { "type": { "name": "elementType" } }, - "disableGutters": { "type": { "name": "bool" } }, - "fixed": { "type": { "name": "bool" } }, + "disableGutters": { "type": { "name": "bool" }, "default": "false" }, + "fixed": { "type": { "name": "bool" }, "default": "false" }, "maxWidth": { "type": { "name": "union", "description": "'xs'
| 'sm'
| 'md'
| 'lg'
| 'xl'
| false
| string" - } + }, + "default": "'lg'" }, "sx": { "type": { diff --git a/docs/pages/material-ui/api/grid-2.json b/docs/pages/material-ui/api/grid-2.json index 08188af737d19a..5c82fc3f32bdbd 100644 --- a/docs/pages/material-ui/api/grid-2.json +++ b/docs/pages/material-ui/api/grid-2.json @@ -5,7 +5,8 @@ "type": { "name": "union", "description": "Array<number>
| number
| object" - } + }, + "default": "12" }, "columnSpacing": { "type": { @@ -13,12 +14,13 @@ "description": "Array<number
| string>
| number
| object
| string" } }, - "container": { "type": { "name": "bool" } }, + "container": { "type": { "name": "bool" }, "default": "false" }, "direction": { "type": { "name": "union", "description": "'column-reverse'
| 'column'
| 'row-reverse'
| 'row'
| Array<'column-reverse'
| 'column'
| 'row-reverse'
| 'row'>
| object" - } + }, + "default": "'row'" }, "offset": { "type": { @@ -42,13 +44,15 @@ "type": { "name": "union", "description": "Array<number
| string>
| number
| object
| string" - } + }, + "default": "0" }, "wrap": { "type": { "name": "enum", "description": "'nowrap'
| 'wrap-reverse'
| 'wrap'" - } + }, + "default": "'wrap'" } }, "name": "Grid2", diff --git a/docs/pages/material-ui/api/stack.json b/docs/pages/material-ui/api/stack.json index ae424d7b57eea6..65e1dc1c8a28d2 100644 --- a/docs/pages/material-ui/api/stack.json +++ b/docs/pages/material-ui/api/stack.json @@ -6,14 +6,16 @@ "type": { "name": "union", "description": "'column-reverse'
| 'column'
| 'row-reverse'
| 'row'
| Array<'column-reverse'
| 'column'
| 'row-reverse'
| 'row'>
| object" - } + }, + "default": "'column'" }, "divider": { "type": { "name": "node" } }, "spacing": { "type": { "name": "union", "description": "Array<number
| string>
| number
| object
| string" - } + }, + "default": "0" }, "sx": { "type": { @@ -22,7 +24,7 @@ }, "additionalInfo": { "sx": true } }, - "useFlexGap": { "type": { "name": "bool" } } + "useFlexGap": { "type": { "name": "bool" }, "default": "false" } }, "name": "Stack", "imports": ["import Stack from '@mui/material/Stack';", "import { Stack } from '@mui/material';"], diff --git a/docs/pages/system/api/container.json b/docs/pages/system/api/container.json index 3f094034d61076..ae5bd4660c2576 100644 --- a/docs/pages/system/api/container.json +++ b/docs/pages/system/api/container.json @@ -2,13 +2,14 @@ "props": { "classes": { "type": { "name": "object" }, "additionalInfo": { "cssApi": true } }, "component": { "type": { "name": "elementType" } }, - "disableGutters": { "type": { "name": "bool" } }, - "fixed": { "type": { "name": "bool" } }, + "disableGutters": { "type": { "name": "bool" }, "default": "false" }, + "fixed": { "type": { "name": "bool" }, "default": "false" }, "maxWidth": { "type": { "name": "union", "description": "'xs'
| 'sm'
| 'md'
| 'lg'
| 'xl'
| false
| string" - } + }, + "default": "'lg'" }, "sx": { "type": { diff --git a/docs/pages/system/api/grid.json b/docs/pages/system/api/grid.json index 3a3c9d23850287..4cd038309b7d7b 100644 --- a/docs/pages/system/api/grid.json +++ b/docs/pages/system/api/grid.json @@ -5,7 +5,8 @@ "type": { "name": "union", "description": "Array<number>
| number
| object" - } + }, + "default": "12" }, "columnSpacing": { "type": { @@ -13,12 +14,13 @@ "description": "Array<number
| string>
| number
| object
| string" } }, - "container": { "type": { "name": "bool" } }, + "container": { "type": { "name": "bool" }, "default": "false" }, "direction": { "type": { "name": "union", "description": "'column-reverse'
| 'column'
| 'row-reverse'
| 'row'
| Array<'column-reverse'
| 'column'
| 'row-reverse'
| 'row'>
| object" - } + }, + "default": "'row'" }, "offset": { "type": { @@ -42,13 +44,15 @@ "type": { "name": "union", "description": "Array<number
| string>
| number
| object
| string" - } + }, + "default": "0" }, "wrap": { "type": { "name": "enum", "description": "'nowrap'
| 'wrap-reverse'
| 'wrap'" - } + }, + "default": "'wrap'" } }, "name": "Grid", diff --git a/docs/pages/system/api/stack.json b/docs/pages/system/api/stack.json index a5d69b0c81f265..2c838e2175b73d 100644 --- a/docs/pages/system/api/stack.json +++ b/docs/pages/system/api/stack.json @@ -6,14 +6,16 @@ "type": { "name": "union", "description": "'column-reverse'
| 'column'
| 'row-reverse'
| 'row'
| Array<'column-reverse'
| 'column'
| 'row-reverse'
| 'row'>
| object" - } + }, + "default": "'column'" }, "divider": { "type": { "name": "node" } }, "spacing": { "type": { "name": "union", "description": "Array<number
| string>
| number
| object
| string" - } + }, + "default": "0" }, "sx": { "type": { @@ -22,7 +24,7 @@ }, "additionalInfo": { "sx": true } }, - "useFlexGap": { "type": { "name": "bool" } } + "useFlexGap": { "type": { "name": "bool" }, "default": "false" } }, "name": "Stack", "imports": ["import Stack from '@mui/system/Stack';", "import { Stack } from '@mui/system';"], diff --git a/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts b/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts index 67520133ad1e12..c724a1486056d0 100644 --- a/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts +++ b/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts @@ -635,43 +635,49 @@ export default async function generateComponentApi( } catch (error) { // fallback to default logic if there is no `create*` definition. if ((error as Error).message === 'No suitable component definition found.') { - reactApi = docgenParse(src, (ast) => { - let node; - // TODO migrate to react-docgen v6, using Babel AST now - astTypes.visit(ast, { - visitFunctionDeclaration: (path) => { - if (path.node.params[0].name === 'props') { - node = path; - } - return false; - }, - visitVariableDeclaration: (path) => { - const definitions: any[] = []; - if (path.node.declarations) { - path - .get('declarations') - .each((declarator: any) => definitions.push(declarator.get('init'))); - } - definitions.forEach((definition) => { - // definition.value.expression is defined when the source is in TypeScript. - const expression = definition.value?.expression - ? definition.get('expression') - : definition; - if (expression.value?.callee) { - const definitionName = expression.value.callee.name; - if (definitionName === `create${componentInfo.name}`) { - node = expression; - } + reactApi = docgenParse( + src, + (ast) => { + let node; + // TODO migrate to react-docgen v6, using Babel AST now + astTypes.visit(ast, { + visitFunctionDeclaration: (functionPath) => { + // @ts-ignore + if (functionPath.node.params[0].name === 'props') { + node = functionPath; } - }); - return false; - }, - }); + return false; + }, + visitVariableDeclaration: (variablePath) => { + const definitions: any[] = []; + if (variablePath.node.declarations) { + variablePath + .get('declarations') + .each((declarator: any) => definitions.push(declarator.get('init'))); + } + definitions.forEach((definition) => { + // definition.value.expression is defined when the source is in TypeScript. + const expression = definition.value?.expression + ? definition.get('expression') + : definition; + if (expression.value?.callee) { + const definitionName = expression.value.callee.name; + if (definitionName === `create${componentInfo.name}`) { + node = expression; + } + } + }); + return false; + }, + }); - return node; - }, defaultHandlers, { - filename, - }); + return node; + }, + defaultHandlers.concat(muiDefaultPropsHandler), + { + filename, + }, + ); } else { throw error; } diff --git a/packages/api-docs-builder/utils/defaultPropsHandler.ts b/packages/api-docs-builder/utils/defaultPropsHandler.ts index 9b49c019f1af88..6836e85cb677ec 100644 --- a/packages/api-docs-builder/utils/defaultPropsHandler.ts +++ b/packages/api-docs-builder/utils/defaultPropsHandler.ts @@ -131,6 +131,11 @@ function getExplicitPropsDeclaration( ): NodePath | undefined { const functionNode = getRenderBody(componentDefinition, importer); + // No function body available to inspect. + if (!functionNode.value) { + return undefined; + } + let propsPath: NodePath | undefined; // visitVariableDeclarator, can't use visit body.node since it looses scope information functionNode diff --git a/packages/api-docs-builder/utils/getPropsFromComponentNode.ts b/packages/api-docs-builder/utils/getPropsFromComponentNode.ts index 0c04dcd987c2f7..a40625ddd4c31c 100644 --- a/packages/api-docs-builder/utils/getPropsFromComponentNode.ts +++ b/packages/api-docs-builder/utils/getPropsFromComponentNode.ts @@ -51,6 +51,7 @@ function isStyledFunction(node: ts.VariableDeclaration): boolean { ); } +// TODO update to reflect https://github.com/DefinitelyTyped/DefinitelyTyped/pull/65135 function getJSXLikeReturnValueFromFunction(type: ts.Type, project: TypeScriptProject) { return type .getCallSignatures() diff --git a/packages/mui-base/src/ClickAwayListener/ClickAwayListener.tsx b/packages/mui-base/src/ClickAwayListener/ClickAwayListener.tsx index e84ca9b1382b42..5f8f4da604c046 100644 --- a/packages/mui-base/src/ClickAwayListener/ClickAwayListener.tsx +++ b/packages/mui-base/src/ClickAwayListener/ClickAwayListener.tsx @@ -209,7 +209,7 @@ function ClickAwayListener(props: ClickAwayListenerProps): React.JSX.Element { return undefined; }, [handleClickAway, mouseEvent]); - return {React.cloneElement(children, childrenProps)}; + return React.cloneElement(children, childrenProps); } ClickAwayListener.propTypes /* remove-proptypes */ = { diff --git a/packages/mui-base/src/NoSsr/NoSsr.tsx b/packages/mui-base/src/NoSsr/NoSsr.tsx index c637df94e70b61..8468d401c64a86 100644 --- a/packages/mui-base/src/NoSsr/NoSsr.tsx +++ b/packages/mui-base/src/NoSsr/NoSsr.tsx @@ -38,8 +38,8 @@ function NoSsr(props: NoSsrProps): React.JSX.Element { } }, [defer]); - // We need the Fragment here to force react-docgen to recognise NoSsr as a component. - return {mountedState ? children : fallback}; + // TODO casting won't be needed at one point https://github.com/DefinitelyTyped/DefinitelyTyped/pull/65135 + return (mountedState ? children : fallback) as React.JSX.Element; } NoSsr.propTypes /* remove-proptypes */ = { diff --git a/packages/mui-base/src/Portal/Portal.tsx b/packages/mui-base/src/Portal/Portal.tsx index 830b229d8eb175..af607b51532adc 100644 --- a/packages/mui-base/src/Portal/Portal.tsx +++ b/packages/mui-base/src/Portal/Portal.tsx @@ -64,14 +64,10 @@ const Portal = React.forwardRef(function Portal( }; return React.cloneElement(children, newProps); } - return {children}; + return children; } - return ( - - {mountNode ? ReactDOM.createPortal(children, mountNode) : mountNode} - - ); + return mountNode ? ReactDOM.createPortal(children, mountNode) : mountNode; }) as React.ForwardRefExoticComponent>; Portal.propTypes /* remove-proptypes */ = { diff --git a/packages/mui-material/src/Hidden/HiddenJs.js b/packages/mui-material/src/Hidden/HiddenJs.js index b591452cef6299..a9ab0d0a965ee2 100644 --- a/packages/mui-material/src/Hidden/HiddenJs.js +++ b/packages/mui-material/src/Hidden/HiddenJs.js @@ -1,5 +1,4 @@ 'use client'; -import * as React from 'react'; import PropTypes from 'prop-types'; import exactProp from '@mui/utils/exactProp'; import withWidth, { isWidthDown, isWidthUp } from './withWidth'; @@ -50,7 +49,7 @@ function HiddenJs(props) { return null; } - return {children}; + return children; } HiddenJs.propTypes = { diff --git a/packages/mui-material/src/NoSsr/NoSsr.tsx b/packages/mui-material/src/NoSsr/NoSsr.tsx index 52b2b04cf74997..0b18a597a751ae 100644 --- a/packages/mui-material/src/NoSsr/NoSsr.tsx +++ b/packages/mui-material/src/NoSsr/NoSsr.tsx @@ -38,7 +38,8 @@ function NoSsr(props: NoSsrProps): React.JSX.Element { } }, [defer]); - return {mountedState ? children : fallback}; + // TODO casting won't be needed at one point https://github.com/DefinitelyTyped/DefinitelyTyped/pull/65135 + return (mountedState ? children : fallback) as React.JSX.Element; } NoSsr.propTypes /* remove-proptypes */ = { diff --git a/packages/mui-material/src/Portal/Portal.tsx b/packages/mui-material/src/Portal/Portal.tsx index 8bd141d8a5f858..a028eb856836e0 100644 --- a/packages/mui-material/src/Portal/Portal.tsx +++ b/packages/mui-material/src/Portal/Portal.tsx @@ -64,14 +64,10 @@ const Portal = React.forwardRef(function Portal( }; return React.cloneElement(children, newProps); } - return {children}; + return children; } - return ( - - {mountNode ? ReactDOM.createPortal(children, mountNode) : mountNode} - - ); + return mountNode ? ReactDOM.createPortal(children, mountNode) : mountNode; }) as React.ForwardRefExoticComponent>; Portal.propTypes /* remove-proptypes */ = {