gaearon / react-hot-loader

Tweak React components in real time. (Deprecated: use Fast Refresh instead.)
http://gaearon.github.io/react-hot-loader/
MIT License
12.26k stars 801 forks source link

RHL should detect wrong HRM #756

Open theKashey opened 6 years ago

theKashey commented 6 years ago

Based on #753 If user has an async loaded component, and that component got not updated in terms of RHL(proxy) before module.hot.accept fired - RHL should detect and highlight the problem.

Current behavior

The element will be not updated on HRM. May be updated(or remounted) on page redraw (including next HMR event).

How

Store the current generation from AppContainer in each instance during construction/hotReplacementRender. Compare with actual value in ProxyComponent.render.

theKashey commented 6 years ago

So - when the hot-exported module got updated and reaches apply state - store the current generation as a minimal value. Next, store this variable in AppContainer's context. Next, all components shall have generation not less than that value.

The problem - currently ProxyComponent stores the number of times it got updated, and lots of components (from packages) will be never be updated. As long there is no way to understand shall some component be updated on HRM even or not - it is impossible to detect wrong HRM. Look like we shall search the another solution.

I've tried to spike a solution for webpack, but failed - there is no way to detect using hot api, that some module were not updated.

The only way I've found is no use module.hot.addStatusHandler

var secret = Date.now();
 module.hot.addStatusHandler ( status => {
    registerSecret(module.id, status, secret);
});
....
const registerSecret = (fileId, state, secret) => {
  files[fileId][state]=secret;
  if(state === 'idle') {
    setTimeout(function() {
      if(  files[fileId]['apply']==files[fileId]['idle']) {
        // throw NOW UPDATED!
      }
   });
  }
}

Actually - this handler could be added only once, to the first instance of a module, and new files could just call some endpoints during the execution.

Anyway - we will have to add this to all files via babel-plugin, and this logic is to far away from our business.

There must be a better way.

theKashey commented 6 years ago

After yet another coastal hike I've understand - this is our business. As long we prevent a whole page refresh and willing to smoothly update the page - we have to track changes like this. It is not hard to track files updates, which were not applied. It is close to impossible to make them - we have to ask user to fix his code - use another loader or mark exports as hot. Literally - just "ask", we cant fix this from inside.

@neoziro - what're your thoughts?

gregberge commented 6 years ago

I think detecting wrong is worst than not detecting. So if we have a 100% sure solution, let's do it, else I think we should not warn.

theKashey commented 6 years ago

I can do it only for webpack (99%?). But not for parcel, as long there is no other API, except accept.

The other (better?) way it to use some your experience from loadable, and analyze how the package was build, asking to wrap all endpoints with how. Maybe patch import in some way. Also - webpack only solution.

PS: You have to wrap endpoints with hot, or reconciler will not work. Ie all the components, extracted as top-level variables will work, but all the hidden ones - will not, as long replacementRender called only once.

gregberge commented 6 years ago

@theKashey I propose to work on "loadable-components" to make it support React Hot Loader out of the box. So users would have a working zero-config solution to use Code Splitting + Hot Reloading.

loadable-components have still some issues with the strategy used to register components (server-side rendering). So I have to also fix them.

What do you think?

theKashey commented 6 years ago

As it mentioned in readme - be aware of HMR is not a business for a "loader". As long react-loadable will ever support HMR/RHL, as long we could make loadable-components more HMR-friendly, and as long react-imported-component already is - it is better not to limit user with some Loader he could use. There is dozen of them, for any case and taste.

In a hour I'll will open a PR for webpack case, to track files user have to mark with hot.

theKashey commented 6 years ago

Bad news, @neoziro - this is not possible yet. It is quite easy to detect, that some file got updated, but there is no way to detect which file should be updated. This is possible with pure webpack's module.hot.check({onAccept}), but it actually is not bound to the module, and uses only the last used option, which is global. And the last used option will be true, provided by webpack-dev-server.

Currently - there is no way to solve this task, except indicating the files which were updated in real, and the ones, webpack thinks were updated.

The other tools, not HRL should be involved here.

gregberge commented 6 years ago

OK let's keep it opened.