mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.37k stars 32.14k forks source link

[docs-infra] Default values are not documented in Joy UI and MUI System #38459

Open alexfauquette opened 1 year ago

alexfauquette commented 1 year ago

Duplicates

Latest version

Steps to reproduce 🕹

The problem comes from docs:api script since the @default is defined, but no default is visible in the JSON file.

It appears for system the muiDefaultPropsHandler is not applied, and adding it the the handlers make the script crash: needs further investigation

Current behavior 😯

No response

Expected behavior 🤔

No response

Context 🔦

No response

Your environment 🌎

npx @mui/envinfo ``` Don't forget to mention which browser you used. Output from `npx @mui/envinfo` goes here. ```
ZeeshanTamboli commented 1 year ago

The same issue is also present for Material UI - https://mui.com/material-ui/api/stack/#Stack-prop-direction.

ZeeshanTamboli commented 1 year ago

We could do the following:

--- a/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts
+++ b/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts
@@ -9,11 +9,12 @@ import remark from 'remark';
 import remarkVisit from 'unist-util-visit';
 import { Link } from 'mdast';
 import { defaultHandlers, parse as docgenParse, ReactDocgenApi } from 'react-docgen';
+import { parse as parseDoctrine } from 'doctrine';
 import { unstable_generateUtilityClass as generateUtilityClass } from '@mui/utils';
 import { renderMarkdown } from '@mui/markdown';
 import { LANGUAGES } from 'docs/config';
 import { ComponentInfo, writePrettifiedFile } from '../buildApiUtils';
-import muiDefaultPropsHandler from '../utils/defaultPropsHandler';
+import muiDefaultPropsHandler, { getJsdocDefaultValue } from '../utils/defaultPropsHandler';
 import parseTest from '../utils/parseTest';
 import generatePropTypeDescription, { getChained } from '../utils/generatePropTypeDescription';
 import createDescribeableProp, {
@@ -552,7 +553,7 @@ const attachTranslations = (reactApi: ReactApi) => {
   reactApi.translations = translations;
 };

-const attachPropsTable = (reactApi: ReactApi) => {
+const attachPropsTable = (reactApi: ReactApi, isSystemComponent?: boolean) => {
   const propErrors: Array<[propName: string, error: Error]> = [];
   type Pair = [string, ReactApi['propsTable'][string]];
   const componentProps: ReactApi['propsTable'] = _.fromPairs(
@@ -569,7 +570,15 @@ const attachPropsTable = (reactApi: ReactApi) => {
         return [] as any;
       }

-      const defaultValue = propDescriptor.jsdocDefaultValue?.value;
+      let defaultValue = propDescriptor.jsdocDefaultValue?.value;
+
+      if (isSystemComponent && !defaultValue && propDescriptor.description) {
+        defaultValue = getJsdocDefaultValue(
+          parseDoctrine(propDescriptor.description, {
+            sloppy: true,
+          }),
+        )?.value;
+      }

       const {
         signature: signatureType,
@@ -834,7 +843,7 @@ export default async function generateComponentApi(
     reactApi.styles.globalClasses = {};
   }

-  attachPropsTable(reactApi);
+  attachPropsTable(reactApi, isSystemComponent);
   attachTranslations(reactApi);

   // eslint-disable-next-line no-console
diff --git a/packages/api-docs-builder/utils/defaultPropsHandler.ts b/packages/api-docs-builder/utils/defaultPropsHandler
.ts
index 77b8942afa..f7dd3bf9eb 100644
--- a/packages/api-docs-builder/utils/defaultPropsHandler.ts
+++ b/packages/api-docs-builder/utils/defaultPropsHandler.ts
@@ -55,7 +55,7 @@ function getDefaultValue(propertyPath: NodePath, importer: Importer) {
   return null;
 }

-function getJsdocDefaultValue(jsdoc: Annotation): { value: string } | undefined {
+export function getJsdocDefaultValue(jsdoc: Annotation): { value: string } | undefined {
   const defaultTag = jsdoc.tags.find((tag) => tag.title === 'default');
   if (defaultTag === undefined) {
     return undefined;

But this above logic will not compare the implementation's default value with the JSDoc's default value tag as it does for other components (which is what muiDefaultPropsHandler does). I don't think we will be able to pass muiDefaultPropsHandler for system components because of how the components are defined. We check for createStack, createGrid in the AST resolver and it does not have default prop values in its definitions.

alexfauquette commented 1 year ago

I thought it was just the @default form proptypes but I I understand correctly, it adds the default value only if the props is defined in a code with the following shape

const {
    'aria-label': ariaLabel,
    'aria-valuetext': ariaValuetext,
    /* ..., */
    ...other
  } = props;

But this above logic will not compare the implementation's default value with the JSDoc's default value tag as it does for other components

From what I understand it's already the case when some props are simply propagated from one component to another: https://github.com/mui/material-ui/blob/master/packages/api-docs-builder/utils/defaultPropsHandler.ts/#L81-L93