Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

v4 does not use updated non-React modules #949

Open
nwoltman opened this issue Apr 24, 2018 · 13 comments
Open

v4 does not use updated non-React modules #949

nwoltman opened this issue Apr 24, 2018 · 13 comments
Assignees

Comments

@nwoltman
Copy link

nwoltman commented Apr 24, 2018

Description

If a React module depends on a module that is not a React component and the non-React module is updated, the code is hot reloaded but the React module does not use the updated non-React module. This was working in v3 and seems like it should work automatically when using ES2015 modules.

Expected behavior

The React module should use the updated non-React module.
This was working as expected with [email protected].

Actual behavior

The React module continues to use the old non-React module.

Environment

React Hot Loader version: 4.1.2, 4.3.0+

Irrelevant version info 1. `node -v`: 10.15.1 2. `npm -v`: 6.4.1

And

  1. Operating system: Windows 10
  2. Browser and version: Chrome 72

Reproducible Demo

https://github.com/nwoltman/rhl-bug-repro

Follow the instructions in the README. When you modify the sanitizeEmail function, the console shows that HMR worked, but when you type in the text input on the page, it seems to still be using the old sanitizeEmail function.

If you change the react-hot-loader version in package.json to 3.1.3 (and uncomment 'react-hot-loader/patch' in webpack.config.js) 4.2.0 (this bug was temporarily patched in that version), it works as expected.

@theKashey
Copy link
Collaborator

The problem - RHL do have a logic to "update" bound methods, and it is straightforward - If a method was changed - update it!
Here - there is no change the method itself, only in the external variable it uses. As result - we are not updating it.

The only possible change - always update all the bound/arrow methods.

@theKashey
Copy link
Collaborator

Thank you. That was a major bug.

@nwoltman
Copy link
Author

No problem! Great to see that you got a fix ready so quickly 😃

@DorianBlues
Copy link

in v4, when I change the method in some components, the hot reload not work, but if I change its parent components methods which as props pass to it, the hot reload worked fine. I wonder is this the same problem above?

@theKashey
Copy link
Collaborator

theKashey commented Apr 27, 2018

@TSL112358 - sounds like something different. But it sounds similar to #944
If you have an easy example - please share. I need it.

theKashey added a commit that referenced this issue May 1, 2018
fix: always update bound functions. #949
@yangmingshan
Copy link

In our application, update a non-React module will cause page reload (not hot reload). Is this a right behavior? Or I'm using the wrong way.

react-hot-loader 4.1.2
node 9.11.1
chrome 66
macOS 10.13.4

@theKashey
Copy link
Collaborator

If "propagation" change bubling will not "hit" hot (or module.hot) - it will trigger will page reload.
This is very handy, when you are updating something that could not be hotreloadable, redux store, for example.
Just double check console output.

@akhayoon
Copy link

akhayoon commented May 3, 2018

@theKashey Is there any way for me to use this updated code that has the fix?

@theKashey
Copy link
Collaborator

We will release a new version today 🤞

@nwoltman
Copy link
Author

nwoltman commented May 9, 2018

Fixed in version 4.1.3 🎉

@nwoltman nwoltman closed this as completed May 9, 2018
@theKashey
Copy link
Collaborator

Sorry breaking this back.

@mike1808
Copy link

@theKashey can you say how we can help with development to fix this issue?

@theKashey
Copy link
Collaborator

In short - #1138
I just need some free time to start playing with it. Unfortunately free time is a problem.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants