cca-io / rescript-mui

ReScript bindings for MUI
MIT License
255 stars 52 forks source link

Timeline for MUI v5 #182

Closed huntwj closed 2 weeks ago

huntwj commented 2 years ago

Hey folks,

Do we have a timeline for if/when we will support Mui v5?

Thanks for all your work!

cknitt commented 2 years ago

Hi,

Not yet, unfortunately, as we are currently extremely busy with client projects. But we are planning to look into it in the next months.

evilpeach commented 2 years ago

Hi,

Could you update the timeline for supporting Mui v5?

Thanks

fhammerschmidt commented 2 years ago

We still have all our projects on v4, so there was no incentive to tackle this. v5 is unfortunately a huge undertaking and will take some time. That said, I will try to at least push my working branch soon, I "only" have to untangle the mess that is the new folder structure of MUI from which we generate all the things.

I don't want to promise anything, but the aim is to at least have an alpha release at the end of the year.

illusionalsagacity commented 1 year ago

@fhammerschmidt would it be feasible to help with the v5 effort?

fhammerschmidt commented 1 year ago

@illusionalsagacity Sorry that it took me so long to answer. I am afraid that this year there won't be a new release as it is the final day. 😭

I adapted the first part of the project, the "component-extractor", to MUI-V5 until the step where it actually parses the source files. But I get parsing errors for every file so there still seems to be something missing. You can find my progress at this branch: https://github.com/cca-io/rescript-material-ui/tree/mui-v5.

The development process basically consists of changing stuff and executing the build.sh in tools/component-extractor and trying to fix the errors that come up.

I hope we nail that soon, otherwise I'd start writing all the bindings by hand myself.

illusionalsagacity commented 1 year ago

@fhammerschmidt All good, it is/was the holidays after all. 🙂

I made a PR for a lot of the parsing errors, now there's some for not being able to find the component definition and some other require issues with paths and such:

Error parsing src for ~/Source/github.com/cca-io/rescript-material-ui/tools/component-extractor/material/Box/Box.js
Error: No suitable component definition found.

Error parsing src for ~/Source/github.com/cca-io/rescript-material-ui/tools/component-extractor/material/Container/Container.js
Error: No suitable component definition found.

Error parsing src for ~/Source/github.com/cca-io/rescript-material-ui/tools/component-extractor/material/FormControl/FormControl.js
Error: No suitable component definition found.

Error parsing src for ~/Source/github.com/cca-io/rescript-material-ui/tools/component-extractor/material/Popper/Popper.js
Error: No suitable component definition found.

Error parsing src for ~/Source/github.com/cca-io/rescript-material-ui/tools/component-extractor/material/styles/CssVarsProvider.js
Error: No suitable component definition found.

Error parsing src for ~/Source/github.com/cca-io/rescript-material-ui/tools/component-extractor/material/styles/ThemeProvider.js
Error: No suitable component definition found.

Error: No suitable component definition found.

Error: Cannot find module '../ButtonGroup/ButtonGroupContext'
Require stack:
- ~/Source/github.com/cca-io/rescript-material-ui/tools/component-extractor/material/Button/Button.js
- ~/Source/github.com/cca-io/rescript-material-ui/tools/component-extractor/material/Button/index.js
- ~/Source/github.com/cca-io/rescript-material-ui/tools/component-extractor/lab/LoadingButton/LoadingButton.js
- ~/Source/github.com/cca-io/rescript-material-ui/tools/component-extractor/src/extract.js

I'll keep looking into it, from the Box component it looks like there will be some special cases potentially -- but not too familiar with how the extractor works yet.

const Box = createBox({
  defaultTheme,
  defaultClassName: 'MuiBox-root',
  generateClassName: ClassNameGenerator.generate,
});

Box.propTypes /* remove-proptypes */ = {
  // ----------------------------- Warning --------------------------------
  // | These PropTypes are generated from the TypeScript type definitions |
  // |     To update them edit the d.ts file and run "yarn proptypes"     |
  // ----------------------------------------------------------------------
  /**
   * @ignore
   */
  children: PropTypes.node,
  /**
   * The component used for the root node.
   * Either a string to use a HTML element or a component.
   */
  component: PropTypes.elementType,
  /**
   * The system prop that allows defining system overrides as well as additional CSS styles.
   */
  sx: PropTypes.oneOfType([
    PropTypes.arrayOf(PropTypes.oneOfType([PropTypes.func, PropTypes.object, PropTypes.bool])),
    PropTypes.func,
    PropTypes.object,
  ]),
};
zatchheems commented 1 year ago

hey there folks. quick thanks to the maintainers on this library! I've been using rescript-material-ui in a production environment for about a year and have been pleased with it thus far. I'm looking forward to using some of the features in MUI v5 such as loading indicators on buttons, alerts and skeletons outside of MuiLab, etc

I'm chiming in just to say that I am also interested in contributing to this effort, and am happy to start wrestling with errors when I have some free time.

TL;DR: how can I help?

illusionalsagacity commented 1 year ago

I have a branch with the tools/component-extractor/build.sh script running, although I'm not confident that the output is correct.

There's still some components being ignored, probably need to have a hard-coded json schema because I don't think react-docgen is capable of handling the component factory pattern mui is using.

Next up seems to be:

fhammerschmidt commented 1 year ago

TBH I was not confident either before your changes, it's all a bit of a research project currently.

@illusionalsagacity Do you know how many components would require the hard-coded schema? It's possible to add fixed modules (see tools/binding-generator/src/fixed-modules) as well.

illusionalsagacity commented 1 year ago

TBH I was not confident either before your changes, it's all a bit of a research project currently.

@illusionalsagacity Do you know how many components would require the hard-coded schema? It's possible to add fixed modules (see tools/binding-generator/src/fixed-modules) as well.

I believe it's at least these:

  // createX pattern
   '/Box.js',
   '/Container.js',
   '/FormControl.js',
   // Typescript
   '/Popper.js', // Popper.tsx
   '/CssVarsProvider.js', // also createX pattern
   '/ThemeProvider.js',

The contexts may also need to be defined if not considered internal by MUI

   '/DialogContext.js',
   '/StepContext.js',
   '/StepperContext.js',
LukasDeco commented 1 year ago

I am also interested in using MUI v5 with this library. I am happy to help resolve errors/anything to help with the effort. 🐎

fhammerschmidt commented 1 year ago

@LukasDeco thank you, very appreciated!

Please check out the https://github.com/cca-io/rescript-material-ui/tree/mui-v5 branch and try to continue what @illusionalsagacity mentioned in their comment

The goal is to generate the majority of the bindings automatically, but manually written modules can be added as well.

LukasDeco commented 1 year ago

@fhammerschmidt not sure if something isn't right in my local setup (WSL Ubuntu 20.04.4 LTS). But the rescript-material-ui-lab package wouldn't build with many errors about Mui.Any:

We've found a bug for you!
  /home/luksd/Repos/rescript-material-ui/public/rescript-material-ui-lab/src/TimelineOppositeContent.res:3:29-37

  1 │ module Sx_arrayOf = {
  2 │   type t
  3 │   external sx_arrayOf_func: Mui.Any.t => t = "%identity"
  4 │   external obj: {..} => t = "%identity"
  5 │   external bool: bool => t = "%identity"
The module or file Mui.Any can't be found.
LukasDeco commented 1 year ago

@fhammerschmidt disregard earlier comment.

Is there an example of adding the hard-coded schema for one of the components like Box? I see Box.res and many other components under tools/binding-generator/src/fixed-modules/ but I'm not sure what additions need to be made. You mentioned using this module for components like Box and ThemeProvider, but those already have .res files in this folder. So I guess I'm just looking for a concrete example of adding one of these "hard-coded schemas". Thanks! :smile:

fhammerschmidt commented 1 year ago

The fixed modules are just completely hand-crafted files with rescript bindings that get copied over when executing yarn build in tools/binding-generator. In the mui-v5 branch, you now also need to add the corresponding JS component to IGNORED_COMPONENTS in tools/component/extractor/src/extract.js.

illusionalsagacity commented 1 year ago

I believe I already added the ones I noted to IGNORED_COMPONENTS

A few of them are considered "internal" by mui so we shouldn't need to write bindings for them all. Ironically the bindings should be more ergonomic with JSXv4 since you can spread the dom props, which should make a lot of the pain points disappear.

I'm hoping to be able to come back to this soon(tm), probably will be looking at the binding generator script to make sure the bindings modules and such are correct.

LukasDeco commented 1 year ago

I did see what was added to IGNORED_COMPONENTS and that's what you posted @illusionalsagacity. Did you have a chance to look at it? Would my efforts be best spent working on the tools/json-schema-parser?

LukasDeco commented 1 year ago

@illusionalsagacity were you able to work on it recently or any guidance for me on what part would be better to work on?

illusionalsagacity commented 1 year ago

@illusionalsagacity were you able to work on it recently or any guidance for me on what part would be better to work on?

I haven't, although it looks like some progress has been made that I haven't looked too closely at yet.

fhammerschmidt commented 1 year ago

So I will use the generator to create one final set of bindings and then switch to manually updating them from there.

We think this will make it easier to get contributions and stay up to date. For instance in rescript-react-native we maintain the bindings manually and it works very well.

One reason for this is because the generator very often does not really yield results that are on par with handcrafted bindings. Yes there are overrides in there, but then you essentially write bindings in JSON instead of ReScript which is not efficient at all.

Some bindings like the Themes are much easier to maintain by hand than to generate them at all.

In the future, hopefully we can utilize the ReScript branch of ts2ocaml.

fhammerschmidt commented 1 year ago

The generator tools will be kept around on the binding-generator branch.

Work on bindings will continue on the mui-v5 branch

fhammerschmidt commented 1 year ago

I finished writing about 80 % of all the bindings for @mui/material, so I will release a @rescript-mui/material 5.0.0-alpha.1 soon.

Yes, new package names to align better with the new packages in MUI v5. Also we skip version number 4 to align at least with the major number.

New bindings already landed on master branch.

fhammerschmidt commented 1 year ago

And there it is: https://www.npmjs.com/package/@rescript-mui/material!

LukasDeco commented 1 year ago

@fhammerschmidt thanks for all your hard work on this. I just had a question. What's your recommendation for a css-in-rescript solution that allows injecting the theme provided by MUI? Like this: https://github.com/mui/material-ui/blob/master/packages/mui-codemod/README.md#jss-to-styled

I noticed that the css styling options in the documentation doesn't specify this(at least not that I could find). Any guidance? Also if there is something I would be happy to update documentation. Thanks again!

fhammerschmidt commented 1 year ago

To be quite honest, I don't use any of those CSS-in-JS solutions anymore, We use tailwind nowadays, and for that it is sufficient to use the StyledEngineProvider with injectFirst=true.

That is also why I set the progression for rescript-mui to 99 % instead of plain 100 %. If you find out what is the optimal way to use such means in rescript-mui, please let me know. (And don't hesitate to contribute a PR).

LukasDeco commented 1 year ago

@fhammerschmidt these are the quick bindings I did for my purpose:

@module("@mui/system")
external styledElement: string => {..} => React.component<JsxDOM.domProps> = "styled"

@module("@mui/system")
external styledElementWithTheme: string => (Mui.Theme.t => {..}) => React.component<
  JsxDOM.domProps,
> = "styled"

@module("@mui/system")
external styledComponent: React.component<'a> => {..} => React.component<'a> = "styled"
@module("@mui/system")
external styledComponentWithTheme: React.component<'a> => (Mui.Theme.t => {..}) => React.component<
  'a,
> = "styled"

I noticed in the repothis would possibly warrant a separate package for the @mui/system stuff? Do you think that would make sense?

LukasDeco commented 1 year ago

Also, I use it like this:

module Button = {
  let make = MuiSystem.styledComponentWithTheme(Mui.Button.make)(theme =>
    {
      "margin": "4px",
      "marginTop": "16px",
      "display": "flex",
      "alignItems": "center",
      "flexWrap": "wrap",
      "color": theme.palette.text.primary,
    }
  )
}
fhammerschmidt commented 1 year ago

Yes, I guess we would want that.

If I created a scaffolding for System, would you contribute to it?

LukasDeco commented 1 year ago

@fhammerschmidt absolutely!

fhammerschmidt commented 1 year ago

Done! https://github.com/cca-io/rescript-mui/commit/1cb1860704fba856954af51bbaa63c2160c239e5

Thank you in advance!