search

floating-ui / react-popper

🍿⚛Official React library to use Popper, the positioning library
https://popper.js.org/react-popper/
MIT License
2.5k stars 226 forks source link

Upgrade dependencies #439

Closed nikolasrh closed 2 years ago

nikolasrh commented 2 years ago

All dependencies that support React 17 have been updated. See commit messages for details about deprecated packages or link to upgrade guides.

React 18 has been added as peer dependency, but it has not been tested. I have tested the build and the demo application, but not the built JavaScript files in the dist folder.

This PR doesn't include a version change.

Yarn status

$ yarn upgrade-interactive --latest
yarn upgrade-interactive v1.22.18
info Color legend : 
 "<red>"    : Major Update backward-incompatible updates 
 "<yellow>" : Minor Update backward-compatible features 
 "<green>"  : Patch Update backward-compatible bug fixes
? Choose which packages to update. (Press <space> to select, <a> to toggle all, <i> to invert selection)
 devDependencies
   name                    range   from         to      url
❯◯ @testing-library/react  latest  12.1.5    ❯  13.0.1  https://github.com/testing-library/react-testing-library#readme
 ◯ @types/react            latest  16.14.24  ❯  18.0.2  https://github.com/DefinitelyTyped/DefinitelyTyped/tree/master/types/react
 ◯ react                   latest  17.0.2    ❯  18.0.0  https://reactjs.org/
 ◯ react-dom               latest  17.0.2    ❯  18.0.0  https://reactjs.org/
 ◯ react-test-renderer     latest  17.0.2    ❯  18.0.0  https://reactjs.org/

Upgrading dev dependencies to React 18

This needs to be resolved in order to upgrade to React 18:

atomiks commented 2 years ago

To properly support React 18 we need to use the flushSync API. https://reactjs.org/docs/react-dom.html#flushsync

Wrap the setState call with this:

import {flushSync} from 'react-dom';

https://github.com/floating-ui/react-popper/blob/fcd81bca90e91e1fe9a2ce1a10dcac9b05f9aed0/src/usePopper.js#L77-L84

Also I guess we need to do it in the render props API too

nikolasrh commented 2 years ago

Will this break support for React 16.8 and 17?

Do you know if flushSync is available in previous versions of react-dom? And react-dom should be added as a peer dependency?

atomiks commented 2 years ago

flushSync existed since React 16.8 apparently (legit had no idea lol, I thought it was part of React 18)

You can check the exports for React 16.8 on unpkg to see (that's what I did)

nikolasrh commented 2 years ago

Maybe we should split it up in seperate PRs? I can drop the commit about React 18 as peer dependency.

nikolasrh commented 2 years ago

I just discovered floating-ui 😁

Maybe that's where I should be heading for React 18 support?

nikolasrh commented 2 years ago

Dropped commit for React 18 support.

atomiks commented 2 years ago

A lot of projects have a dependency on react-popper so it would be good to support React 18 (and it's not too difficult). You can make a new PR with just that setState and peer dep change

But yeah! Floating UI is actively being developed while Popper is not. Therefore for new projects it's a good choice

nikolasrh commented 2 years ago

@FezVrasta Will you have time to review this?

FezVrasta commented 2 years ago

How's this different than https://github.com/floating-ui/react-popper/pull/440?

nikolasrh commented 2 years ago

440 upgrades React to 18. This PR upgrades everything else.

I will not be following up this PR since we are switching to floating-ui. Do with it as you want. As a last change I have rebased master to this branch to integrate with #440 and #441 and removed @testing-library/react-hooks.

440 upgraded @testing-library/react to 13, but still kept @testing-library/react-hooks which is now available through @testing-library/react. It would be nice if someone with more experience with act and waitFor would look over the changes in https://github.com/floating-ui/react-popper/pull/439/commits/23d553e87b6383b260d8f727934bb85503c653be

After upgrading to React 18, yarn install gives a peer dependency warning on @react-spring/web, but the animations in yarn run demo:dev still seems to be working fine.

yarn install v1.22.18
[1/4] Resolving packages...
[2/4] Fetching packages...
[3/4] Linking dependencies...
warning " > @react-spring/web@9.4.4" has incorrect peer dependency "react@^16.8.0  || ^17.0.0".
warning " > @react-spring/web@9.4.4" has incorrect peer dependency "react-dom@^16.8.0  || ^17.0.0".
warning "@react-spring/web > @react-spring/animated@9.4.4" has incorrect peer dependency "react@^16.8.0  || ^17.0.0".
warning "@react-spring/web > @react-spring/core@9.4.4" has incorrect peer dependency "react@^16.8.0  || ^17.0.0".
warning "@react-spring/web > @react-spring/shared@9.4.4" has incorrect peer dependency "react@^16.8.0  || ^17.0.0".