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

Jump to bottom

[docs-infra] Allow JSDoc tags #42327

Merged
merged 2 commits into from
May 22, 2024
Merged

[docs-infra] Allow JSDoc tags #42327

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
60d512a
Allow JSDoc tags in components
aarongarciah May 21, 2024
9611931
Allow JSDoc tags in hooks
aarongarciah May 22, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion ...nslations/api-docs/use-menu-item-context-stabilizer/use-menu-item-context-stabilizer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"hookDescription": "Stabilizes the ListContext value for the MenuItem component, so it doesn't change when sibling items update.\n\n@param id The id of the MenuItem. If undefined, it will be generated with useId.\n@returns The stable ListContext value and the id of the MenuItem.",
"hookDescription": "Stabilizes the ListContext value for the MenuItem component, so it doesn't change when sibling items update.",
"parametersDescriptions": {},
"returnValueDescriptions": {}
}
2 changes: 1 addition & 1 deletion docs/translations/api-docs/use-option-context-stabilizer/use-option-context-stabilizer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"hookDescription": "Stabilizes the ListContext value for the Option component, so it doesn't change when sibling Options update.\n\n@param value The value of the Option.\n@returns The stable ListContext value.",
"hookDescription": "Stabilizes the ListContext value for the Option component, so it doesn't change when sibling Options update.",
"parametersDescriptions": {},
"returnValueDescriptions": {}
}
21 changes: 18 additions & 3 deletions packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import type { Link } from 'mdast';
import { defaultHandlers, parse as docgenParse, ReactDocgenApi } from 'react-docgen';
import { renderMarkdown } from '@mui/internal-markdown';
import { ComponentClassDefinition } from '@mui/internal-docs-utils';
import { parse as parseDoctrine, Annotation } from 'doctrine';
import { ProjectSettings, SortingStrategiesType } from '../ProjectSettings';
import { ComponentInfo, toGitHubPath, writePrettifiedFile } from '../buildApiUtils';
import muiDefaultPropsHandler from '../utils/defaultPropsHandler';
Expand Down Expand Up @@ -147,7 +148,7 @@ export async function computeApiDescription(
* *
* * - [Icon API](https://mui.com/api/icon/)
*/
async function annotateComponentDefinition(api: ReactApi) {
async function annotateComponentDefinition(api: ReactApi, componentJsdoc: Annotation) {
const HOST = 'https://mui.com';

const typesFilename = api.filename.replace(/\.js$/, '.d.ts');
Expand Down Expand Up @@ -318,6 +319,14 @@ async function annotateComponentDefinition(api: ReactApi) {
markdownLines.push(`- inherits ${inheritanceAPILink}`);
}

if (componentJsdoc.tags.length > 0) {
markdownLines.push('');
}

componentJsdoc.tags.forEach((tag) => {
markdownLines.push(`@${tag.title}${tag.name ? ` ${tag.name} -` : ''} ${tag.description}`);
});

const jsdoc = `/**\n${markdownLines
.map((line) => (line.length > 0 ? ` * ${line}` : ` *`))
.join('\n')}\n */`;
Expand Down Expand Up @@ -738,13 +747,19 @@ export default async function generateComponentApi(
reactApi.props = {};
}

const { getComponentImports = defaultGetComponentImports } = projectSettings;
const componentJsdoc = parseDoctrine(reactApi.description);

// We override `reactApi.description` with `componentJsdoc.description` because
// the former can include JSDoc tags that we don't want to render in the docs.
reactApi.description = componentJsdoc.description;

// Ignore what we might have generated in `annotateComponentDefinition`
const annotatedDescriptionMatch = reactApi.description.match(/(Demos|API):\r?\n\r?\n/);
if (annotatedDescriptionMatch !== null) {
reactApi.description = reactApi.description.slice(0, annotatedDescriptionMatch.index).trim();
}

const { getComponentImports = defaultGetComponentImports } = projectSettings;
reactApi.filename = filename;
reactApi.name = componentInfo.name;
reactApi.imports = getComponentImports(componentInfo.name, filename);
Expand Down Expand Up @@ -825,7 +840,7 @@ export default async function generateComponentApi(
: !skipAnnotatingComponentDefinition
) {
// Add comment about demo & api links (including inherited component) to the component file
await annotateComponentDefinition(reactApi);
await annotateComponentDefinition(reactApi, componentJsdoc);
}
}

Expand Down
18 changes: 16 additions & 2 deletions packages/api-docs-builder/ApiBuilders/HookApiBuilder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import traverse from '@babel/traverse';
import { defaultHandlers, parse as docgenParse, ReactDocgenApi } from 'react-docgen';
import kebabCase from 'lodash/kebabCase';
import upperFirst from 'lodash/upperFirst';
import { parse as parseDoctrine, Annotation } from 'doctrine';
import { renderMarkdown } from '@mui/internal-markdown';
import { ProjectSettings } from '../ProjectSettings';
import { computeApiDescription } from './ComponentApiBuilder';
Expand Down Expand Up @@ -108,7 +109,7 @@ export interface ReactApi extends ReactDocgenApi {
* *
* * - [useButton API](https://mui.com/base-ui/api/use-button/)
*/
async function annotateHookDefinition(api: ReactApi) {
async function annotateHookDefinition(api: ReactApi, hookJsdoc: Annotation) {
const HOST = 'https://mui.com';

const typesFilename = api.filename.replace(/\.js$/, '.d.ts');
Expand Down Expand Up @@ -285,6 +286,14 @@ async function annotateHookDefinition(api: ReactApi) {
})`,
);

if (hookJsdoc.tags.length > 0) {
markdownLines.push('');
}

hookJsdoc.tags.forEach((tag) => {
markdownLines.push(`@${tag.title}${tag.name ? ` ${tag.name} -` : ''} ${tag.description}`);
});

const jsdoc = `/**\n${markdownLines
.map((line) => (line.length > 0 ? ` * ${line}` : ` *`))
.join('\n')}\n */`;
Expand Down Expand Up @@ -546,6 +555,11 @@ export default async function generateHookApi(

const parameters = await extractInfoFromType(`${upperFirst(name)}Parameters`, project);
const returnValue = await extractInfoFromType(`${upperFirst(name)}ReturnValue`, project);
const hookJsdoc = parseDoctrine(reactApi.description);

// We override `reactApi.description` with `hookJsdoc.description` because
// the former can include JSDoc tags that we don't want to render in the docs.
reactApi.description = hookJsdoc.description;

// Ignore what we might have generated in `annotateHookDefinition`
const annotatedDescriptionMatch = reactApi.description.match(/(Demos|API):\r?\n\r?\n/);
Expand Down Expand Up @@ -588,7 +602,7 @@ export default async function generateHookApi(
await generateApiJson(apiPagesDirectory, reactApi);

// Add comment about demo & api links to the component hook file
await annotateHookDefinition(reactApi);
await annotateHookDefinition(reactApi, hookJsdoc);
}

return reactApi;
Expand Down
6 changes: 3 additions & 3 deletions packages/mui-base/src/useMenuItem/useMenuItemContextStabilizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,16 @@ import { ListContext, ListContextValue, ListItemState } from '../useList';
/**
* Stabilizes the ListContext value for the MenuItem component, so it doesn't change when sibling items update.
*
* @param id The id of the MenuItem. If undefined, it will be generated with useId.
* @returns The stable ListContext value and the id of the MenuItem.
*
* Demos:
*
* - [Menu](https://mui.com/base-ui/react-menu/#hooks)
*
* API:
*
* - [useMenuItemContextStabilizer API](https://mui.com/base-ui/react-menu/hooks-api/#use-menu-item-context-stabilizer)
*
* @param id - The id of the MenuItem. If undefined, it will be generated with useId.
* @returns The stable ListContext value and the id of the MenuItem.
Comment on lines +16 to +18
Copy link
Member

Choose a reason for hiding this comment

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

The docs API generation is using naming template to find the types to parse. That's why currently have wrong parameter table 🙈

image

material-ui/packages/api-docs-builder/ApiBuilders/HookApiBuilder.ts

Lines 547 to 549 in bfcb0b6

const parameters = await extractInfoFromType(`${upperFirst(name)}Parameters`, project);
const returnValue = await extractInfoFromType(`${upperFirst(name)}ReturnValue`, project);

Copy link
Member Author

Choose a reason for hiding this comment

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

I saw this, yeah! I was about to open an issue about it. It looks like some hooks like useMenuItemContextStabilizer and useOptionContextStabilizer are missing the *Parameters and *ReturnValue types. But even if they had those types, it looks like the API generation for the parameter table in hooks only works for hooks that accept a single object parameter. Same for the return value.

We also need to improve how we display the parameters table in the docs since it's not clear that the hook accepts a single object vs separated parameters when looking at it.

image

Copy link
Member

Choose a reason for hiding this comment

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

It feels like the script generation should crash in those cases: https://next--material-ui.netlify.app/base-ui/react-menu/hooks-api/#use-menu-item-context-stabilizer. Forcing to either make the API private, or to use the right naming convention for its type.

*/
export function useMenuItemContextStabilizer(id: string | undefined) {
const listContext = React.useContext(ListContext as React.Context<ListContextValue<string>>);
Expand Down
6 changes: 3 additions & 3 deletions packages/mui-base/src/useOption/useOptionContextStabilizer.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,16 +5,16 @@ import { ListContext, ListContextValue } from '../useList';
/**
* Stabilizes the ListContext value for the Option component, so it doesn't change when sibling Options update.
*
* @param value The value of the Option.
* @returns The stable ListContext value.
*
* Demos:
*
* - [Select](https://mui.com/base-ui/react-select/#hooks)
*
* API:
*
* - [useOptionContextStabilizer API](https://mui.com/base-ui/react-select/hooks-api/#use-option-context-stabilizer)
*
* @param value - The value of the Option.
* @returns The stable ListContext value.
*/
export function useOptionContextStabilizer<OptionValue>(value: OptionValue) {
const listContext = React.useContext(ListContext as React.Context<ListContextValue<OptionValue>>);
Expand Down
2 changes: 2 additions & 0 deletions packages/mui-material/src/Hidden/Hidden.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,8 @@ export interface HiddenProps {
* API:
*
* - [Hidden API](https://mui.com/material-ui/api/hidden/)
*
* @deprecated The Hidden component was deprecated in Material UI v5. To learn more, see [the Hidden section](/material-ui/migration/v5-component-changes/#hidden) of the migration docs.
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we use absolute URLs here for links to work in IDE?

Copy link
Member Author

@aarongarciah aarongarciah May 23, 2024 •
edited

Choose a reason for hiding this comment

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

Yes, we should use absolute URLs for links inside comments, but this is an existing issue that needs to be tackled separately. We have this problem in all props e.g. https://github.com/mui/material-ui/blob/next/packages/mui-material/src/ListItem/ListItem.js#L372

JSDoc comments are used to generate the docs so we can't assume we're always in https://mui.com. See: https://github.com/mui/material-ui/blob/master/packages/api-docs-builder/ApiBuilders/ComponentApiBuilder.ts#L111-L117

We should probably use absolute URLs and adjust the docs infra to replace the base URL with the correct one. I'll open an issue.

Copy link
Member

Choose a reason for hiding this comment

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

@cherniavskii good catch!

@aarongarciah please assign me to the issue when you create it 😊

Copy link
Member Author

Choose a reason for hiding this comment

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

@DiegoAndai done! #42361

*/
declare const Hidden: React.JSXElementConstructor<HiddenProps>;

Expand Down
2 changes: 2 additions & 0 deletions packages/mui-material/src/Hidden/Hidden.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ import HiddenCss from './HiddenCss';

/**
* Responsively hides children based on the selected implementation.
*
* @deprecated The Hidden component was deprecated in Material UI v5. To learn more, see [the Hidden section](/material-ui/migration/v5-component-changes/#hidden) of the migration docs.
*/
function Hidden(props) {
const {
Expand Down