react-modal icon indicating copy to clipboard operation
react-modal copied to clipboard

Add support for React & Node 18 (as easy as adding startTransition() on each setState)

Open AnderUstarroz opened this issue 2 years ago • 18 comments

Summary:

Current react modal doesn't work with latest version of React + Node v18

Steps to reproduce:

  1. Use react-modal with latest React's and Node v18

Expected behavior:

Should work normally

Example of issue:

Breaks with the following error message: Screenshot 2023-05-08 at 15 42 54

Solution:

Simply add startTransition() wrapping each of the calls for setting the state. startTransition(() => setState("something))

AnderUstarroz avatar May 08 '23 06:05 AnderUstarroz

do you think someone supports this library?

multivoltage avatar Jan 24 '24 15:01 multivoltage

@AnderUstarroz It seems easy. Would you like to make a contribution?

diasbruno avatar Jan 25 '24 12:01 diasbruno

@AnderUstarroz can you provide an eample? I installed correctly a basic example with react 18 yesterday

multivoltage avatar Jan 25 '24 13:01 multivoltage

@multivoltage Would you like to make a contribution?

diasbruno avatar Feb 01 '24 14:02 diasbruno

Maybe it's related to the weird <suspense /> component.

Probably, something like this:

<suspense fallback="...">
  <modal />
</suspense>

will trigger this behavior.

diasbruno avatar Feb 01 '24 14:02 diasbruno

I'm guessing to fix this isse the solution is to wrap the setIsOpen() (how people call it). But it's application specific, so we don't break older reaect versions.

function toggleModal() {
    startTransition(() => {
      setIsOpen(!isOpen);      
    });
}

diasbruno avatar Feb 01 '24 14:02 diasbruno

Is it only necessary to set setIsOpen? I would like to be able to carry out this task to contribute to the project

itsjesusmacias avatar Feb 15 '24 13:02 itsjesusmacias

@diasbruno yes but as I said before in my example all work great so... @itsjesusmacias yes but using start-transiction means a major release since startTransition is only react > 18

multivoltage avatar Feb 15 '24 14:02 multivoltage

Thanks, @multivoltage. @itsjesusmacias Give it a try...I'm hoping something like this would help, otherwise we need to investigate further this issue.

As @multivoltage said, this feature it released on react 18+, so if we need to make a fix for it, we need to be careful to not break for older versions.

diasbruno avatar Feb 15 '24 20:02 diasbruno

Maybe we can add "peerDependencies" react >=18 ?

multivoltage avatar Feb 15 '24 20:02 multivoltage

It's already a peer dependency.

diasbruno avatar Feb 15 '24 20:02 diasbruno

@AnderUstarroz please can you provide a demo? I tried a 2nd time to reproduce it but with my stack;

  • node 18/20
  • vite js

Using demo example on homepage here https://www.npmjs.com/package/react-modal seem to work without errors

multivoltage avatar Feb 15 '24 21:02 multivoltage

@multivoltage with respect to your comment:

Maybe we can add "peerDependencies" react >=18 ?

To build on what you and @diasbruno have been saying, this would require all projects using react-modal to be on React v18+, no? This would generally be considered a breaking change requiring a new major version release as you pointed out above. :)

Given the popularity of this project, I don't think supporting only the latest version of React is a reasonable peer dependency even if there was a major release.

doeg avatar Feb 16 '24 17:02 doeg

@doeg How I said before when there is a real demo project with bug I'll happy to investigate more :) but talking about your idea I think:

  • projects with react < 18 will use react-modal 3.x
  • projects with react >= 18 will use react-modal 4.x

In my experience this is normal. When this happends the readme should include a row like: from react-modal 4 only react > 17 is supported. Migrate or keep going to use v3

People who install for some reason 4.x in a project with react 17 (or 16) will read a red warning in the console after npm i react-modal

multivoltage avatar Feb 16 '24 18:02 multivoltage

@multivoltage I opened a separate issue here to address which versions of React should be supported: https://github.com/reactjs/react-modal/issues/1041

doeg avatar Feb 16 '24 18:02 doeg

I've spent some time in CodeSandbox trying to reproduce the error noted above:

A component suspended while responding to synchronous input. This will cause the UI to be replaced with a loading indicator. To fix, updates that suspend should be wrapped with startTransition.

This issue in the React repo goes into some detail about the error: https://github.com/facebook/react/issues/25629

The simplest reproduction I've gotten so far is attempting to lazy load a component and render it within a modal without wrapping it in a Suspense block.

CodeSandbox link: https://codesandbox.io/p/sandbox/react-18-react-modal-suspense-error-vq79xg

import "./styles.css";
import ReactModal from "react-modal";
import { useState, lazy, Suspense } from "react";

const ExampleComponent = lazy(() => import("./ExampleComponent"));

export default function App() {
  const [showModal, setShowModal] = useState(false);

  return (
    <div className="App">
      <button onClick={() => setShowModal(true)}>Open Modal</button>

      <ReactModal isOpen={showModal} onRequestClose={() => setShowModal(false)}>
        {/* Uncomment the Suspense boundary below to resolve the error */}
        {/* <Suspense fallback={<div>Loading...</div>}> */}
        <ExampleComponent />
        {/* </Suspense> */}
      </ReactModal>
    </div>
  );
}

image

In this case, adding a Suspense boundary around the problematic component is enough to solve the issue:

image

I've yet to find a way to reproduce the A component suspended while responding to synchronous input error that points to the issue being within react-modal itself.

I've also yet to find anything that would suggest React Modal won't work with React v18, though as mentioned in https://github.com/reactjs/react-modal/pull/1040 I'm going to do some experimentation with React Modal + React v18 in our very complex codebase next week. :)

The only other potentially related issue with React Modal + Suspense could be https://github.com/reactjs/react-modal/issues/982.

@AnderUstarroz as others have mentioned, it would be super useful to get a reproducible example of what's producing this error for you. 🙇 Otherwise, @diasbruno, it seems like this one could perhaps be closed out as "not a bug"...?

doeg avatar Feb 16 '24 20:02 doeg

Maybe README can be edited with some details to help developer in this use-case?

multivoltage avatar Feb 16 '24 21:02 multivoltage

Bom trabalho, @doeg. 👍

Some notes:

  • The content of the modal is always rendered - <ExampleComponent />, even though it's not attached to the tree yet (isOpen starts false) (probably this explains why we need to add Suspense (?), but why the fallback <div /> is ok?)
  • Why adding startTransition work for @AnderUstarroz? Maybe the project already uses suspense for this?

@itsjesusmacias Feel free to jump in anytime. The more people, more ideas.

diasbruno avatar Feb 16 '24 23:02 diasbruno