React-redux: Error when using HMR

[theverything]

I recently updated [email protected]. After doing so I noticed that after every code change HMR would cause a full page refresh do to an error.

I looked through the stack trace and the error comes from react-redux.

Subscription.js:67 Uncaught (in promise) TypeError: this.subscribe is not a function
    at Subscription.trySubscribe (Subscription.js:67)
    at Subscription.addNestedSub (Subscription.js:53)
    at Subscription.trySubscribe (Subscription.js:67)
    at ProxyComponent.componentWillUpdate (connectAdvanced.js:246)
    at ProxyComponent.componentWillUpdate (createPrototypeProxy.js:44)
    at react.js:6605
    at measureLifeCyclePerf (react.js:5971)
    at ReactCompositeComponentWrapper._performComponentUpdate (react.js:6604)
    at ReactCompositeComponentWrapper.updateComponent (react.js:6539)
    at ReactCompositeComponentWrapper.receiveComponent (react.js:6441)

I reverted back to [email protected] and the error went away and HMR started working correctly again.

| software | version |
| --- | --- |
| react | 15.3.2 |
| webpack | 2.1.0-beta.25 |
| redux | 3.6.0 |
| node | 6.7.0 |
| npm | 3.10.3 |

[jimbolla]

@theverything I'm not exactly sure how to reproduce this. Any chance you can provide a repo?

[theverything]

I can't share the repo that the error is happening with, but I am trying to create an other one to reproduce the issue.

[theverything]

I haven't been able to create a project to reproduce the error. I did try the beta again in my current project and the error still persists. Let me know if there is any way I can help.

[jimbolla]

Is there any way you can share your project, even a stripped down one with me the demonstrates the problem? With that I could reduce the code into a test case.

[theverything]

I tried that but there was no error. 😞

[jimbolla]

Maybe in stripping it down, you removed some key part that's causing react-redux to have the problem.

[theverything]

Thats what I'm thinking.

[jimbolla]

@theverything Anything I can do to help you with this? I'd love to resolve this one to help v5 make it to release. If you can't share your code with me, what about a screen-sharing session? Maybe from that I can see enough to make a repro without it.

[timdorr]

Without a repro, there isn't really anything to be done here. I'll reopen if there is something that can be reproduced, but HMR itself is known to have issues and may be the culprit here.

[patrikholcak]

@timdorr Recently discovered the same error — here’s the project. If you try to HMR ./src/routes/counter.js you’ll see the exact same error. Downgrading to react-redux@^4.4.6 fixed it

[restrry]

Hi. guys. I ran into the same problem. Seems the reason is that connectAdvanced doesn't update link to subscription after updating. So I added my updating mechanism and it works for my project.
If it's okay for you, I could add some tests and send PR

[jimbolla]

@restrry We'll have to make some changes to get it to merge with PR #540 but this is very helpful. Thanks.

[jimbolla]

Does anyone, @patrikholcak, @restrry, know how to simulate HMR in a test?

[timdorr]

You might look at how webpack does it. But it is pretty complex and I don't know if adding that to our infrastructure would be a good use of time. Better to spot test that on your own.

If it's of any comfort, it's not an end-user feature anyways. So, the impact of fixing it does have a relatively limited audience. Not to say it's not important, just that you're not going to break everyone's app/site because of it :)

[restrry]

agree, we needn't such complex check, since it was already tested in webpack. but react-redux uses internal mechanism for detecting re-loading. doesn't it? detecting is based on version comparison, but testing it could be quite hard. I thought to inherit from Connect class, add some methods for changing version and pass it to createConnect factory, but it seems to be overhead and fragile approach

[jimbolla]

If you have a ref to the component, you should be able to do component.version = -1 to make the reinitialization logic happen, but I think the part that's lost is how HMR affects the component's lifecycle.

I'm gonna spend some time on this in a couple weeks when I'm on extended vacation. I think the test is valuable because otherwise it might break again. The problem with having a bunch of really great tests is then things that aren't covered by them tend to be forgotten.

[jimbolla]

I just added PR #567 which fixes this. Thanks @patrikholcak and @restrry.

[markerikson]

Sweet! Can you recap what the problem was, and how the fix works?

[jimbolla]

Basically, Subscription.tryUnsubscribe was clearing this.subscribe so then it was null when the component tried to resubscribe when HMR was detected. I removed the clearing and refactored a little for clarity.

[theverything]

On Monday I'll check out your branch and try it with my app