search

cca-io / rescript-mui

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

Fixes rescript 10.1 compilation #190

Closed illusionalsagacity closed 1 year ago

illusionalsagacity commented 1 year ago

Fixes a compilation issue with rescript@^10.1.2, where the Box module's optional named parameters were option<option<'a>> rather than option<'a>

I need to test this some more with more permutations of rescript+@rescript/react and bsconfig, not sure if this is fully working

needs node >14

ppx definitely doesn't work in 18:

Uncaught Error: Invalid hook call. Hooks can only be called inside of the body of a function component. This could happen for one of the following reasons:
1. You might have mismatching versions of React and the renderer (such as React DOM)
2. You might be breaking the Rules of Hooks
3. You might have more than one copy of React in the same app
See https://reactjs.org/link/invalid-hook-call for tips about how to debug and fix this problem.
    React 2
    useTheme useTheme.js:4
    useStyles makeStyles.js:222
    app curry.js:14
    _1 curry.js:62
    NewImplementation NewImplementation.bs.js:26
    React 13
    jBOC0 Index.bs.js:25
    newRequire index.3ff66f6a.js:71
    <anonymous> index.3ff66f6a.js:120
    <anonymous> index.3ff66f6a.js:143
react.development.js:1476:12
    React 2
    useTheme useTheme.js:4
    useStyles makeStyles.js:222
    app curry.js:14
    _1 curry.js:62
    NewImplementation NewImplementation.bs.js:26
    React 8
    performSyncWorkOnRoot self-hosted:1406
    React 5
    jBOC0 Index.bs.js:25
    newRequire index.3ff66f6a.js:71
    <anonymous> index.3ff66f6a.js:120
    <anonymous> index.3ff66f6a.js:143

test cases

  1. disconnect ppx-test from monorepo by removing it from package.json workspaces, this allows a separate version of rescript & @rescript/react to be tested.
  2. in public/rescript-material-ui, public/rescript-material-ui-lab, and public/rescript-material-ui-ppx run yarn link
  3. in ppx-test run: 
    yarn link rescript-material-ui
yarn link rescript-material-ui-lab
yarn
yarn run clean
yarn run build
yarn run serve
fhammerschmidt commented 1 year ago

It should only be the PPX that is not working, right? Because I worked with 10.1 for some time now already, and had no issues.

When we adopted this repository, we stated that we won't maintain the PPX if it breaks, as we don't use it. So we could get rid of it, or do you use it?

illusionalsagacity commented 1 year ago

I think none of the CI will work on material-ui v4 because a package vrtest-mui seems to have been unpublished.

It should only be the PPX that is not working, right?

Yeah it was some combination that led to that rules of hooks error in the PR description. I should've written it down as it seems to work with:

It may have been on react 18 that it was broken.

When we adopted this repository, we stated that we won't maintain the PPX if it breaks, as we don't use it. So we could get rid of it, or do you use it?

I do use it, but have been planning on moving away for some time since V5 encourages using emotion instead.

illusionalsagacity commented 1 year ago

I went through the test cases listed in the PR description and couldn't reproduce that error, odd. As far as I can tell this is working normally, aside from the CI not being able to be run anymore.

edit: I will try linking this with autobooks' app tomorrow and making sure that works

illusionalsagacity commented 1 year ago

Seems to be working fine in our app with

"jsx": { "version": 3, "mode": "classic" },
"bsc-flags": ["-open ReactV3"],

Yeah the PPX doesn't work in JSXv4:

FAILED: Foo.cmj

  We've found a bug for you!
  Foo.res

  The value jsx can't be found in ReactDOM

I'll indicate in the README for it when I get a chance

illusionalsagacity commented 1 year ago

This might not be necessary, seems to be compiling on rescript@10.1.3