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

Upgrading to @rescript/react is incompatible with most of my app’s dependencies based on reason/react #6

Closed
vasco3 opened this issue Feb 11, 2021 · 9 comments

Comments

@vasco3
Copy link

vasco3 commented Feb 11, 2021

Some scripts that I use in my app are breaking. For example:

  1. rescript-recoil
  2. bs-reach
  3. re-formality

is there a way to fix this other than waiting for the third-party libs to catch up?

example of error

It's possible that your build is stale.
Try to clean the artifacts and build again?

Here's the original error message
The files /code/sobras/node_modules/rescript-recoil/lib/ocaml/recoil.cmi
and /code/sobras/node_modules/@rescript/react/lib/ocaml/react.cmi
make inconsistent assumptions over interface React

example 2

It's possible that your build is stale.
Try to clean the artifacts and build again?

Here's the original error message
The files /Users/jorgec/Downloads/code/sobras/node_modules/@rescript/react/lib/ocaml/react.cmi
and /Users/jorgec/Downloads/code/sobras/node_modules/re-formality/lib/ocaml/formality.cmi
make inconsistent assumptions over interface React

example 3

It's possible that your build is stale.
Try to clean the artifacts and build again?

Here's the original error message
The files /Users/jorgec/Downloads/code/sobras/node_modules/@rescript/react/lib/ocaml/reactDOMRe.cmi
and /Users/jorgec/Downloads/code/sobras/node_modules/bs-reach/lib/ocaml/slider-Reach.cmi
make inconsistent assumptions over interface React
@ryyppy
Copy link
Member

ryyppy commented Feb 12, 2021

Hm, I think this is the moment where third party packages need to create a new npm package that does align with the new rescript & rescript-react ecosystem.

Not only because it's not possible to drive two different packages with the same exposed toplevel module names, but also because the package names like bs-xyz, and the module source code are inconsistent with ReScript's recommendations.

As a quick workaround, I'd copy the bs-reach / re-formality libraries into your app code and remove the dependency. bs-reach hasn't gotten any updates for quite some time, so I'd actually argue that you should copy it anyways, since you get more control over the code when you find interface mismatches.

I can't find any details about ˋrescript-reasonˋ... was this a typo?

@vasco3
Copy link
Author

vasco3 commented Feb 12, 2021

@ryyppy it was a typo! I meant rescript-recoil. I edited my initial comment.

Since re-formality depend on ppx, is it straightforward to just copy the code? I've never built a ppx

@bloodyowl
Copy link
Contributor

Migrated my projects:

@ryyppy
Copy link
Member

ryyppy commented Feb 12, 2021

@vasco3 huh that's weird... just checking re-formality, it doesn't look like it has a dependency to reason-react?

So as long as the library + ppx don't use any of the old ReasonReact toplevel modules, or some funky bindings from ReactDOMRe that have been deprecated for a while, it should actually work fine. The problems arise when you have a package that has a dependency on reason-react, like bloodyowl had.

\cc @alexfedoseev just to verify what reason-react apis are being used, because I can't find any hints in the source code so far

@alex35mil
Copy link

@ryyppy Current published package contains 2 versions of the lib:

  1. PPX: current
  2. Non-PPX: kept for backward compatibility to make transition to the current one easier

PPX version uses:

  • React.useReducer
  • React.useEffect*
  • React.useMemo*

Non-PPX version uses:

  • React.useReducer
  • React.useEffect*
  • React.useMemo*
  • ReactEvent.Form.*

Package has only one dependency and it's not reason-react.

@vasco3
Copy link
Author

vasco3 commented Feb 12, 2021

@alex35mil
Copy link

@vasco3 @ryyppy I found a culprit: https://github.com/MinimaHQ/re-formality/blob/master/lib/bsconfig.json#L5

The package doesn't have reason-react as an npm dependency but it's included in bs-dependencies. B/c all users had reason-react installed, it was working. Now the package name changed and it causes issues. I will release a fix this weekend.

@alex35mil
Copy link

Updated:

  • re-formality@4.0.0-beta.11
  • re-dnd@4.0.0-beta.1

@cknitt
Copy link
Member

cknitt commented Sep 26, 2022

After more than one and a half years, all projects that are still active will already have updated the dependency to @rescript/react.

If there is still anything missing, please open an issue on that project's issue tracker.

@cknitt cknitt closed this as completed Sep 26, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants