deps: upgrade deps to allow react@^17

[williaster]

react@17 is out and doesn't contain breaking features so we should update semantic version ranges for react peerDependencies to allow an upper range of ^17

[hshoff]

No new features, but there are breaking changes to be mindful of:

• https://reactjs.org/blog/2020/08/10/react-v17-rc.html#fixing-potential-issues
• https://reactjs.org/blog/2020/08/10/react-v17-rc.html#other-breaking-changes

Other related issues that could be separate issues or tagged on to this one:

1. Fix strict mode warnings
2. Drop React 15 support

[hshoff]

v17 release announcement: https://reactjs.org/blog/2020/10/20/react-v17.html

[m0t0r]

@williaster @hshoff I would like to help with this issue, is this something still available or it's already being worked on ? Also, could you provide some details what exactly we expect to close this issue ? Is it only adding "react": "^15.0.0-0 || ^16.0.0-0 || ^17.0.0" or something else ?

[sarathps93]

@williaster @hshoff I would like to work on this issue. Shall I pick it up?

[hshoff]

Hi @sarathps93, @m0t0r has first dibs. Will leave it up to him if he wants to continue working on this.

---

is this something still available or it's already being worked on?

yes, still available. you have first dibs.

Also, could you provide some details what exactly we expect to close this issue ? Is it only adding "react": "^15.0.0-0 || ^16.0.0-0 || ^17.0.0" or something else?

that's a good start, it would be || ^17.0.0-0". There would be some additional tasks related to this.

Additional tasks:

1. address v17 breaking changes: https://reactjs.org/blog/2020/08/10/react-v17-rc.html#fixing-potential-issues, https://reactjs.org/blog/2020/08/10/react-v17-rc.html#other-breaking-changes
2. bump react-dom
3. bump associated @types deps
4. drop react 15 support
5. identify and fix react strict mode warnings
6. verify tests pass with v17
7. verify @visx/demo runs without issue on v17

[m0t0r]

Thanks @hshoff I will work on this issue. Sorry @sarathps93

[sarathps93]

No issues at all @m0t0r :)

[m0t0r]

Upgrading to React v17 made 34 tests to fail with same error:

    Error name:    "TypeError"
    Error message: "Cannot read property 'child' of undefined"

I found out that it's related to enzyme is not yet supporting React 17. There is an open issue. We would have to wait until they release enzyme-adapter-react-17 which according to this comment is still in the works.

[yigitlevent]

Any updates on this? I've been using React 17 for a while now in a project and really want to try visx.

[boy51]

Any updates on this? I've been using React 17 for a while now in a project and really want to try visx.

I'm running react 17 with visx and installing on npm 7 using --legacy-peer-deps which I assume installs react 16.8 as a peer dep in package lock and it works fine.

I too would appreciate if the peer dep is updated though, i don't think much changed in react 17...

[smallnamespace]

FYI, I've been using visx + React 17 for the last few months now without issue.

[raix]

@m0t0r should not need to update the development dependencies them selfs only open up the peer dependency to "16.x || 17.x"
(having chained dependencies on enzyme would really lock things down, from the history of enzyme - one of the reasons why developers are moving to RTL - along with better test patterns)

[williaster]

@m0t0r looks like there's an unofficial enzyme-adapter-react-17, not sure if it'd be worth trying that to see if fixes the failing tests you saw.

[danantal]

@m0t0r should not need to update the development dependencies them selfs only open up the peer dependency to "16.x || 17.x"
(having chained dependencies on enzyme would really lock things down, from the history of enzyme - one of the reasons why developers are moving to RTL - along with better test patterns)

I will second this. You don't need to update the development dependencies themselves - but you could still open up for the 17 peer dep :)

[maxphillipsdev]

Running with react 17 and next.js 10.2, can confirm it still works if you install with --legacy-peer-deps

[williaster]

Just an update here that we're going to try to prioritize this in Q3/soon, thanks for all the nudges to keep this a priority.

@gazcn007 and I will try to

• update everything to allow react@17
• (since the official react-17-adaptor is not released as of this time) update failing tests to react-testing-library (so, we will have a mix of tests which we're okay with; over time we'll likely try to migrate to RTL)

We'll also trying to coordinate the breaking changes of the 2.0.0 release with the react-spring@9 upgrade in #1218 .

@boy51, if you're interested in helping maybe we can circle back with a branch and specific tests that we need to convert to RTL?

[boy51]

Sure, I'm not super familiar with enzyme but I'd be glad to contribute

[jackw]

@williaster If you or any of the other contributors require any help here please let me know as I'm more than happy to assist.

[gazcn007]

hi folks, this PR is ready for review. I upgraded react to support v16 || v17, rewrote enzyme tests to RTL for ones that use mount which is not supported in React v17.
Please ignore visx-react-spring and visx-xychart, because those two are dependent on the react-spring v8 to v9 upgrade, which is addressed in this PR: #1277

@williaster @m0t0r @boy51 @jackw @keithyang

[gazcn007]

React 17 upgrade PR is green and waiting for a stamp
#1268

With regard to Harry's comment:

• upgraded all packages to use react v16 || v17
• address v17 breaking changes: https://reactjs.org/blog/2020/08/10/react-v17-rc.html#fixing-potential-issues, https://reactjs.org/blog/2020/08/10/react-v17-rc.html#other-breaking-changes
  • for document.addEventListener(...) and e.stopPropagation(), i don't see a scenario where you want document events firing when e.stopPropagation() is called
  • for other breaking changes: also did not find an issue or bug related to those as far as my investigation goes
• bump react-dom
• bump associated @types deps
• drop react 15 support
• identify and fix react strict mode warnings:
  • not sure if that needs to be fixed in this PR
• verify tests pass with v17
• verify @visx/demo runs without issue on v17

[williaster]

closed by #1268