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

Documentation clarification around code splitting #1368

Closed
tstirrat15 opened this issue Oct 28, 2019 · 8 comments · Fixed by #1369
Closed

Documentation clarification around code splitting #1368

tstirrat15 opened this issue Oct 28, 2019 · 8 comments · Fixed by #1369

Comments

@tstirrat15
Copy link
Contributor

Looking at this section in under the code splitting header:

To support any code splitting library RHL uses a special tail update detection logic - if some components would be requested after initial HMR event - Application would be updated yet again to apply possible change. In this case you will see a message React-Hot-Loader: some components were updated out-of-bound. Updating your app to reconcile the changes..

I don't understand what this paragraph means. When does the warning show? Does it mean that something is wrong, or does it just indicate that a component on the other side of a code-splitting boundary has been updated?

I'm happy to submit an update to this section once I feel like I understand it.

@theKashey
Copy link
Collaborator

There was (and some issues says still is) an issue, which would be better described as I could see my update only after anther update.
Basically

  • the client receives an update message
  • it replaces old components by a new one
  • it force-updates everything
  • you still not see your update
  • 😭🤔
  • you do another update
  • and now you see it - your first one

It could be caused by different reasons, but in terms of code splitting it was due the non-instant effect from dynamic import - "the new" component would be loaded after RHL force-updates everything and exits. As long as the call to that import would be caused by the force-update.

There was a special logic to handle React.lazy, some libraries were importing their stuff in their constructor... until "that change", which is just checking "is something updated" after "the update".
When I was rewriting imported-components I've failed to reimplement this logic for the custom React.lazy component, and had to stop using it to be RHL-compliant. Then I decided to fix the root cause, and here we are.

So, if RHL detects that something was updated after update just finished - it updates everything again. This change initial process:

  • 😭🤔
  • you do another update <--- it does this
  • and now you see it - your first one 🥳

However, code splitting library still should detect HMR and call the importer again. Not all are doing that.

I hope now you have enought context to reword this sentence.

@tstirrat15
Copy link
Contributor Author

I think so. Lemme submit a PR and see if I've got it. Thank you!

@brandon-pereira
Copy link

It would be nice if we could disable the warning, is this possible?

@theKashey
Copy link
Collaborator

Or "report it once"?

@brandon-pereira
Copy link

Well, the docs say "The warning is informational - it is a notice that this tail update logic is triggered, and does not indicate a problem in the configuration or usage of react-hot-loader.". I have a team of developers working with me, I'm managing the SSR/HMR setup. I don't think every developer should need to know about the inner works of a HMR. Ideally, I can set setConfig({ trackTailUpdates:true }). to suppress this warning since it's just informational and not an issue with the configuration. I'd like to keep our console logs to a minimal.

If this makes sense, I can try and see if I can implement the change.

@tstirrat15
Copy link
Contributor Author

The idea being that if you're specifying the behavior, it shouldn't tell you that it's doing it? That makes sense to me.

@theKashey
Copy link
Collaborator

Sounds right

@brandon-pereira
Copy link

Opened #1386 where I'll implement this!

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

Successfully merging a pull request may close this issue.

3 participants