Update react-redux to use forwardRef syntax

[mvanlonden]

WIP DNM

Proposal for react-redux v6 contains breaking changes. This PR is a WIP to address those changes.

reduxjs/react-redux#995

cc @markerikson

[markerikson]

React-Redux 6.0 beta is now available, so this will need to be pushed forward soon. Please see https://github.com/reduxjs/react-redux/releases/tag/v6.0.0-beta.1 for our release notes, and reduxjs/react-redux#1000 for the actual changes .

[markerikson]

Hmm. The tests look to be completely busted for everything related to refs.

@mvanlonden , got time to revisit this?

[erikras]

I'm ready to merge when the tests pass.

Seems like package.json would need to change, no?

[markerikson]

I briefly tried cloning the repo and pulling this PR. The initial issue, as Erik said, is that the package needs to have the React-Redux version bumped.

After I did that, though, I got about 80 test failures, for a variety of reasons, and I'm not comfortable with this codebase to be able to dig into them myself.

Hopefully someone can dig into things soon.

[mvanlonden]

Took a quick pass but ran into the test failures. This was for compatibility with reduxjs/react-redux#995 but might not matter since the other PR also uses forwarded refs?

I'm also not very familiar with this codebase and am migrating to a new form library.

[markerikson]

PR 1000 was what we finally went with, but they should have ultimately had the same public API.

If I upgrade the peer dep and dev dep to react-redux@6.0.0-beta.3, the test failures I'm seeing are stuff like:

  ● Fields.immutable › should rerender when sync error is cleared

    expect(jest.fn()).toHaveBeenCalledTimes(1)

    Expected mock function to have been called one time, but it was called two times.

      1400 |       // username input rendered
      1401 |       expect(usernameInput).toHaveBeenCalled()
    > 1402 |       expect(usernameInput).toHaveBeenCalledTimes(1)
           |                             ^
      1403 |
      1404 |       // username field has error
      1405 |       expect(usernameInput.mock.calls[0][0].username.meta.valid).toBe(false)

      at Object.<anonymous> (src/__tests__/Fields.spec.js:1402:29)

Lots of stuff like that.

[markerikson]

Oh. Dear.

I just realized that Redux-Form uses legacy React context internally.

That may explain why things appear to be very broken atm - I've seen a bunch of issues filed against React saying bad things happen when you mix old and new context.

Well, I tried poking around at the tests, but I think this is going to require a lot more work than I'd initially thought. @erikras , this may require some focused attention from you once you're back from your vacation.

(Also, today's lovely discovery: WebStorm doesn't recognize tests correctly when they're defined in nested functions, which is how they're done in this repo so they can be parameterized. Oops.)

[harrythedev]

I'm switching to formik. 👋

[erikras]

Back from vacation. Working on this...

[cereallarceny]

@erikras Do we have any rough time estimate on this?

[erikras]

Put in about six hours yesterday. Gonna throw some more at it today. Lots of broken tests still.

End of this week, early next week?

[pogran]

@erikras , hi. The problem was fixed?

[cereallarceny]

It's not my issue @pogran 😄

[erikras]

Published fix in v8.0.0.

[Ogala]

Thanks @erikras. Fix works well so far! v8.0.1

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.