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

[core] Extract classes descriptions from TypeScript #25933

Merged
merged 17 commits into from
May 10, 2021

Conversation

eps1lon
Copy link
Member

@eps1lon eps1lon commented Apr 24, 2021

Will not remove the runtime classes descriptions in this PR for reviewability. I'll remove them in a follow-up.

Originally we extracted these descriptions from the runtime code e.g.

export const styles = {
  /* Styles applied to the root element */
  root: {}
}

For components using emotion that approach no longer worked. Instead, we parsed the AST of the TypeScript declaration files.

Relying on the AST is generally brittle since it breaks every time we change the pattern. Instead, we can rely on the TS type-checker (can re-use typescript-to-proptypes resulting in a significantly simpler docs:api implementation). This has the benefit of giving us a single approach that doesn't care about where the classes are located. It will document the same thing a dev will see when writing the code and looking at IntelliSense.

This revealed a couple of excess declared classes that have no effect at runtime (#25917) and missing classes (upcoming PR). You'll notice these issues in the current state of the PR.

Requires #25917
Required for #25754

@eps1lon eps1lon added the core Infrastructure work going on behind the scenes label Apr 24, 2021
@mui-pr-bot
Copy link

mui-pr-bot commented Apr 24, 2021

No bundle size changes (experimental)

Generated by 🚫 dangerJS against 9959ce6

@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 27, 2021
@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 28, 2021
It'll be ignored by `react-docgen` anyway
@github-actions github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Apr 29, 2021
@@ -94,7 +94,7 @@ describe('typescript-to-proptypes', () => {
});

if (fs.existsSync(outputPath)) {
expect(propTypes.replace(/\r?\n/g, '\n')).to.include(
expect(propTypes.replace(/\r?\n/g, '\n')).to.equal(
Copy link
Member Author

Choose a reason for hiding this comment

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

Better failure message due to displaying a diff. include does not "include" a diff in the failure message.

@github-actions github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label May 10, 2021
Let api object decide whether we want to document classes customization
@eps1lon eps1lon marked this pull request as ready for review May 10, 2021 12:02
@eps1lon
Copy link
Member Author

eps1lon commented May 10, 2021

Merging since there isn't much any changes to the documentation (the order for internal pickers components are for now ignored).

@eps1lon eps1lon merged commit 8cb115b into mui:next May 10, 2021
@eps1lon eps1lon deleted the feat/ttp-nested-jsdoc branch May 10, 2021 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants