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

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

Open
2 tasks done
alexfauquette opened this issue Aug 13, 2023 · 3 comments
Open
2 tasks done
Assignees
Labels
bug 🐛 Something doesn't work docs Improvements or additions to the documentation package: joy-ui Specific to @mui/joy package: material-ui Specific to @mui/material package: system Specific to @mui/system scope: docs-infra Specific to the docs-infra product

Comments

@alexfauquette
Copy link
Member

Duplicates

  • I have searched the existing issues

Latest version

  • I have tested the 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.
@alexfauquette alexfauquette added status: waiting for maintainer These issues haven't been looked at yet by a maintainer scope: docs-infra Specific to the docs-infra product labels Aug 13, 2023
@danilo-leal danilo-leal changed the title [docs-infra] The default values are not documented in Joy and system [docs-infra] Default values are not documented in Joy UI and MUI System Aug 14, 2023
@ZeeshanTamboli
Copy link
Member

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

@ZeeshanTamboli ZeeshanTamboli added bug 🐛 Something doesn't work docs Improvements or additions to the documentation package: system Specific to @mui/system package: material-ui Specific to @mui/material package: joy-ui Specific to @mui/joy and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Sep 19, 2023
@ZeeshanTamboli
Copy link
Member

ZeeshanTamboli commented Sep 19, 2023

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
Copy link
Member Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work docs Improvements or additions to the documentation package: joy-ui Specific to @mui/joy package: material-ui Specific to @mui/material package: system Specific to @mui/system scope: docs-infra Specific to the docs-infra product
Projects
Status: Backlog
Development

No branches or pull requests

3 participants