Fix bug that blocks plugin from using versions of React-Redux >= v6

[Choongkyu]

• Address bug described here: React-Redux 6.0.0 package breaks dispatch #166
• Update dependencies

[CLAassistant]

Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

Choong Kim seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[rtsao]

Nice, thanks for working on this.

I think it might be nice do this a non-breaking change.

One option for this might look something like:

1. Change react-redux peer dependency from 5.x to 5.x || 7.x
2. Make contextTypes.store non-required.
3. If ReactReduxContext is exported from react-redux, use it, otherwise fall back to context.store (but in development, log a warning that react-redux@5 is deprecated).

Then in the next major version, we can drop support for react-redux@5 entirely.

Thoughts?

[Choongkyu]

All of that sounds great. I'll update to reflect how I understood your implementation advice.

[Choongkyu]

@rtsao What're your thoughts on including React 16.8 as a requirement so that useContext can be used to make the code a bit cleaner?

EDIT: I decided against doing this in the interest of making this a non-breaking change.

[KevinGrandon]

Nice work here, but looks like a few linter and testing errors. Would you like to resolve these? I can also have our team try to take a look and use this PR as a starting point, but it might take a bit longer. Thanks for the work on this PR.

[rtsao]

It seems this plugin already depends on [email protected], which has a peer dependency of React 16.8 already. This was changed in #196

So I suppose this breaking change was already made?

cc @micburks

[micburks]

I suppose the peer version should have just been relaxed to >=1.3.4. Might be a good idea to revert that since there's no strict reason for forcing fusion-react 2+

[Choongkyu]

Nice work here, but looks like a few linter and testing errors. Would you like to resolve these? I can also have our team try to take a look and use this PR as a starting point, but it might take a bit longer. Thanks for the work on this PR.

Thanks. I was able to find one linting issue which I resolved by disabling the console rule. However, I'm currently getting this page when I try to get more intel on the errors and was wondering how I might be able to remedy them. https://imgur.com/a/TqGn89Y

[KevinGrandon]

Thanks. I was able to find one linting issue which I resolved by disabling the console rule. However, I'm currently getting this page when I try to get more intel on the errors and was wondering how I might be able to remedy them. https://imgur.com/a/TqGn89Y

Thanks for the updates.

You can find a public build log here: http://uber-open-source-gate.herokuapp.com/buildkite_public_log?https://buildkite.com/uberopensource/fusion-plugin-rpc-redux-react/builds/374

I believe this should manifest locally with the yarn test command.

[KevinGrandon]

Current error for browser tests is

# browser plugin integration test withRPCRedux
ok 1 dispatches expected action type
ok 2 dispatches expected action payload
TypeError: Cannot read property 'ReactReduxContext' of undefined

Were you able to see this with yarn test?

[codecov]

Codecov Report

Merging #200 into master will decrease coverage by 46.42%.
The diff coverage is 43.47%.

@@            Coverage Diff            @@
##           master   #200       +/-   ##
=========================================
- Coverage   96.42%    50%   -46.43%     
=========================================
  Files           2      2               
  Lines          28     40       +12     
  Branches        5      9        +4     
=========================================
- Hits           27     20        -7     
- Misses          0     18       +18     
- Partials        1      2        +1

Impacted Files	Coverage Δ
src/plugin.js	22.22% <0%> (-77.78%)	⬇️
src/hoc.js	58.06% <45.45%> (-36.68%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 3d241b1...44ef0ac. Read the comment docs.

[Choongkyu]

Current error for browser tests is

# browser plugin integration test withRPCRedux
ok 1 dispatches expected action type
ok 2 dispatches expected action payload
TypeError: Cannot read property 'ReactReduxContext' of undefined

Were you able to see this with yarn test?

Thanks, Kevin! I wasn't able to see that error with yarn test. This is what I see:

TAP version 13
# plugin
ok 1 should be equal
ok 2 should be equal
# mock plugin
ok 3 should be equal
ok 4 should be equal
# withRPCRedux hoc
ok 5 should be equal
ok 6 should be equal
ok 7 passes the handler through to props
ok 8 should be equal
ok 9 should be equal
ok 10 should be equal
ok 11 should be equal
# withRPCReactor hoc
ok 12 should be equal
ok 13 should be equal
ok 14 passes the handler through to props
ok 15 should be equal
ok 16 should be equal
ok 17 should be equal
ok 18 should be equal
# ResponseError
ok 19 should be truthy
# interface
ok 20 default export function
ok 21 createRPCReducer function export
ok 22 mock function export
ok 23 withRPCRedux function export
ok 24 withRPCReactor function export
ok 25 should be equal
ok 26 should be equal
ok 27 should be equal
ok 28 should be equal

# tests 28
# pass  28

# ok

[KevinGrandon]

You might be able to reproduce this locally with the docker workflow. (This would be the same workflow Buildkite uses)

docker-compose build
docker-compose run fusion-plugin-rpc-redux-react yarn test

[Choongkyu]

docker-compose build

I'm able to build just fine but get this message when trying to run yarn test with docker:

 [email protected] build-test /fusion-plugin-rpc-redux-react
> rm -rf dist-tests && cup build-tests

rm: cannot remove 'dist-tests': Device or resource busy
npm ERR! code ELIFECYCLE
npm ERR! errno 1
npm ERR! [email protected] build-test: `rm -rf dist-tests && cup build-tests`
npm ERR! Exit status 1
npm ERR! 
npm ERR! Failed at the [email protected] build-test script.
npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

npm ERR! A complete log of this run can be found in:
npm ERR!     /root/.npm/_logs/2019-05-10T02_31_36_659Z-debug.log
error Command failed with exit code 1.                                                                                                                                 
info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command.