[core] Rework `typescript-to-proptypes` to share the AST parsing with `parseStyles` #38517

flaviendelangle · 2023-08-17T11:46:59Z

I'm diverging from the API exposed by the original typescript-to-proptypes API.
Why ? Because we now have quite a lot of tooling that rely on the typescript AST, and the typescript-to-proptypes has a lot of interesting we could not use because it was totally independent of the rest.

@michaldudak @alexfauquette , I am not sure where the code used both by parseStyle and typescript-to-proptypes (and other codes in the future) should live.
For example, createTypeScriptProject will be used by X.

I'll propose some evolution on the API of those methods to share more logic with X later.

mui-bot · 2023-08-17T12:42:18Z

Netlify deploy preview

https://deploy-preview-38517--material-ui.netlify.app/

Bundle size report

No bundle size changes (Toolpad)
No bundle size changes

Generated by 🚫 dangerJS against 5920e67

flaviendelangle · 2023-08-18T12:31:38Z

packages/mui-material/src/SwipeableDrawer/SwipeableDrawer.js

   */
  onOpen: PropTypes.func.isRequired,
  /**
   * If `true`, the component is shown.
   * @default false
   */
-  open: PropTypes.bool.isRequired,


This fix is correct

flaviendelangle · 2023-08-18T12:31:53Z

docs/data/joy/components/autocomplete/Virtualize.js

@@ -56,6 +56,7 @@ const OuterElementType = React.forwardRef((props, ref) => {
 });

 // Adapter for react-window
+


No idea where this white line is coming from...

flaviendelangle · 2023-08-21T09:18:17Z

docs/pages/material-ui/api/swipeable-drawer.json

@@ -3,15 +3,21 @@
    "onClose": {
      "type": { "name": "func" },
      "required": true,
-      "signature": { "type": "function(event: object) => void", "describedArgs": ["event"] }
+      "signature": {


Not sure why the parsing is better now 😆

flaviendelangle · 2023-08-21T09:18:49Z

packages/mui-material/src/Hidden/Hidden.js

@@ -86,22 +86,22 @@ Hidden.propTypes /* remove-proptypes */ = {
   */
  initialWidth: PropTypes.oneOf(['xs', 'sm', 'md', 'lg', 'xl']),
  /**
-   * If `true`, component is hidden on screens below (but not including) this size.


Those new descriptions are the right one from the type definition 🤔

flaviendelangle · 2023-08-21T09:21:04Z

packages/typescript-to-proptypes/src/injectPropTypesInFile.ts

@@ -319,7 +325,7 @@ function plugin(
        if (!node.id) {
          return;
        }
-        const props = propTypes.body.find((prop) => prop.name === node.id!.name);


I don't know why the return format of the parser was this ast.body format, but I find the new one a lot easier to read.

alexfauquette · 2023-08-21T09:44:01Z

For example, createTypeScriptProject will be used by X.

Do you have some specific project in mind? (I assume you do not go into those AST just for pleasure 😁)

flaviendelangle · 2023-08-21T09:45:16Z

Do you have some specific project in mind?

I plan to rebuild getTypeScriptProjects to be a superset of this method, so that I can pass the X typescript projects to any shared method and have it work.

docs/translations/api-docs/hidden/hidden.json

michaldudak · 2023-08-30T20:28:03Z

docs/translations/api-docs/swipeable-drawer/swipeable-drawer.json

@@ -39,5 +39,57 @@
      "description": "The duration for the transition, in milliseconds. You may specify a single timeout for all transitions, or individually with an object."
    }
  },
-  "classDescriptions": {}
+  "classDescriptions": {


packages/api-docs-builder/utils/createTypeScriptProject.ts

michaldudak · 2023-08-30T20:36:15Z

packages/api-docs-builder/utils/getPropsFromComponentNode.ts

+  }
+  //  x = React.memo((props:type) { return <div/> })
+  //  x = React.forwardRef((props:type) { return <div/> })
+  if (ts.isCallExpression(node.initializer) && node.initializer.arguments.length > 0) {


We also have React.memo(React.forwardRef(...)) (Base UI's Option), but since there are no changes in generated files, I assume it works fine 👍

I guess it does 👌

@flaviendelangle, it looks like it doesn't after all. While working on #36287, I changed the wording of the PropTypes warning message and noticed that it was not updated on some components.
The affected components are:

base/Option

base/Portal

base/TextareaAutosize

joy/Box

lab/Timeline

material-next/SliderRoot

material-next/SliderRail

material-next/SliderTrack

material-next/SliderThumb

material-next/SliderValueLabel

material-next/SliderMark

material-next/SliderMarkLabel

material/Popper

material/SliderRoot

material/SliderRail

material/SliderTrack

material/SliderThumb

material/SliderValueLabel

material/SliderMark

material/SliderMarkLabel

Do we agree that the previous version did not handle this scenario as well ?
If so, I can have a look on how to improve the new one 👍

Partially. For example, base/Option did not work correctly before, but joy/Box was fine.

For base/Option, the injector did not handler nested wrapper function list memo(forwardRef(...)), I have a working fix.

For joy/Box, I'm investigating

For joy/Box, I was returning too soon so all the createX were probably broken at the parsing phase

For base/Portal (and probably others) we don't support correctly the as React.ForwardRefExoticComponent<...>

For SliderRoot and equivalent, I suspect that the proptypes have been written manually, because I see nothing in the code to generate the proptypes of styled(...)

While working to fix anotherbug, I enabled the proptypes of the styled by mistake so if we want we can do it, but right now they are never generated.

package.json

michaldudak

Looks good to me (and most importantly, it works ;)). Please handle the failing CI check and it'll be good to go.

flaviendelangle · 2023-09-01T06:51:23Z

I think this CI failure is caused by a71e6a0 which is not related at all to my PR.

By the way, I would find it great not to push commits without PRs as it often leads to this kind of problem...

michaldudak · 2023-09-01T06:55:55Z

💯 agree. I think @oliviertassinari is the only offender here ;)

oliviertassinari · 2023-09-04T00:12:54Z

I think this CI failure is caused by a71e6a0 which is not related at all to my PR.

Oops, #38758 will help.

… `parseStyles` (mui#38517)

flaviendelangle force-pushed the rework-ttp branch from 5c0357b to 05e4e69 Compare August 17, 2023 11:47

flaviendelangle self-assigned this Aug 17, 2023

flaviendelangle added the core Infrastructure work going on behind the scenes label Aug 17, 2023

flaviendelangle force-pushed the rework-ttp branch from 05e4e69 to e096071 Compare August 17, 2023 12:39

flaviendelangle force-pushed the rework-ttp branch 2 times, most recently from 1b17451 to 50e64e6 Compare August 17, 2023 13:07

[core] Rework typescript-to-proptypes

b7d18f5

flaviendelangle force-pushed the rework-ttp branch from 50e64e6 to b7d18f5 Compare August 17, 2023 13:10

flaviendelangle added 2 commits August 18, 2023 10:39

Merge branch 'master' into rework-ttp

f00a488

Fix parseStyles

3332ee4

flaviendelangle force-pushed the rework-ttp branch 6 times, most recently from 7581015 to 7191668 Compare August 18, 2023 12:29

flaviendelangle commented Aug 18, 2023

View reviewed changes

flaviendelangle changed the title ~~[core] Rework typescript-to-proptypes~~ [core] Rework typescript-to-proptypes to share the AST parsing with parseStyles Aug 18, 2023

Migrate formatteddTSDemos

a3c51c6

flaviendelangle force-pushed the rework-ttp branch from 7191668 to a3c51c6 Compare August 18, 2023 12:48

Work on tests

8274555

flaviendelangle force-pushed the rework-ttp branch from c00cf3a to 8274555 Compare August 21, 2023 08:57

flaviendelangle commented Aug 21, 2023

View reviewed changes

flaviendelangle requested review from alexfauquette and michaldudak August 21, 2023 09:21

flaviendelangle marked this pull request as ready for review August 21, 2023 09:44

michaldudak reviewed Aug 30, 2023

View reviewed changes

flaviendelangle added 2 commits September 1, 2023 07:39

Merge branch 'master' into rework-ttp

f98009d

Code review: Michal

cca9214

flaviendelangle requested a review from michaldudak September 1, 2023 05:48

michaldudak approved these changes Sep 1, 2023

View reviewed changes

Merge branch 'master' into rework-ttp

956be1f

github-actions bot added the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 2, 2023

Merge

5920e67

github-actions bot removed the PR: out-of-date The pull request has merge conflicts and can't be merged label Sep 4, 2023

flaviendelangle merged commit f0af89f into mui:master Sep 4, 2023
8 checks passed

flaviendelangle deleted the rework-ttp branch September 4, 2023 10:19

flaviendelangle mentioned this pull request Sep 6, 2023

[core] Fix prop-types generation #38831

Merged

mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Sep 8, 2023

[core] Rework typescript-to-proptypes to share the AST parsing with…

e2bc7be

… `parseStyles` (mui#38517)

mnajdova pushed a commit to mnajdova/material-ui that referenced this pull request Sep 8, 2023

[core] Rework typescript-to-proptypes to share the AST parsing with…

c8f866e

… `parseStyles` (mui#38517)

xcode-it pushed a commit to xcode-it/material-ui that referenced this pull request Sep 11, 2023

[core] Rework typescript-to-proptypes to share the AST parsing with…

d1c5d53

… `parseStyles` (mui#38517)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[core] Rework `typescript-to-proptypes` to share the AST parsing with `parseStyles` #38517

[core] Rework `typescript-to-proptypes` to share the AST parsing with `parseStyles` #38517

flaviendelangle commented Aug 17, 2023 •

edited

Loading

mui-bot commented Aug 17, 2023 •

edited

Loading

flaviendelangle Aug 18, 2023

flaviendelangle Aug 18, 2023

flaviendelangle Aug 21, 2023

flaviendelangle Aug 21, 2023

flaviendelangle Aug 21, 2023

alexfauquette commented Aug 21, 2023

flaviendelangle commented Aug 21, 2023

michaldudak Aug 30, 2023

michaldudak Aug 30, 2023

flaviendelangle Sep 1, 2023

michaldudak Sep 6, 2023

flaviendelangle Sep 6, 2023

michaldudak Sep 6, 2023

flaviendelangle Sep 6, 2023

flaviendelangle Sep 6, 2023

flaviendelangle Sep 6, 2023 •

edited

Loading

flaviendelangle Sep 6, 2023 •

edited

Loading

michaldudak left a comment

flaviendelangle commented Sep 1, 2023

michaldudak commented Sep 1, 2023

oliviertassinari commented Sep 4, 2023

		@@ -56,6 +56,7 @@ const OuterElementType = React.forwardRef((props, ref) => {
		});

		// Adapter for react-window

[core] Rework typescript-to-proptypes to share the AST parsing with parseStyles #38517

[core] Rework typescript-to-proptypes to share the AST parsing with parseStyles #38517

Conversation

flaviendelangle commented Aug 17, 2023 • edited Loading

mui-bot commented Aug 17, 2023 • edited Loading

Netlify deploy preview

Bundle size report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexfauquette commented Aug 21, 2023

flaviendelangle commented Aug 21, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

flaviendelangle Sep 6, 2023 • edited Loading

Choose a reason for hiding this comment

flaviendelangle Sep 6, 2023 • edited Loading

Choose a reason for hiding this comment

michaldudak left a comment

Choose a reason for hiding this comment

flaviendelangle commented Sep 1, 2023

michaldudak commented Sep 1, 2023

oliviertassinari commented Sep 4, 2023

[core] Rework `typescript-to-proptypes` to share the AST parsing with `parseStyles` #38517

[core] Rework `typescript-to-proptypes` to share the AST parsing with `parseStyles` #38517

flaviendelangle commented Aug 17, 2023 •

edited

Loading

mui-bot commented Aug 17, 2023 •

edited

Loading

flaviendelangle Sep 6, 2023 •

edited

Loading

flaviendelangle Sep 6, 2023 •

edited

Loading