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

🔥Not all methods could|should be updated #1001

Closed
theKashey opened this issue Jun 4, 2018 · 4 comments
Closed

🔥Not all methods could|should be updated #1001

theKashey opened this issue Jun 4, 2018 · 4 comments

Comments

@theKashey
Copy link
Collaborator

Each component consist of 2 different pieces:

  1. Class methods. They exists on prototype and could be updated just by replacing the old prototype by a new one.
  2. Class properties. Their initiation occurs in class constructor.

To update second ones RHL creates 2 components - old and new, then compares all descriptors.
Comparison ignores "objects", thus keeps state untouched, and updates functions and simple variables.

But that does mean "updates functions"?
It literally cast function toString and then eval that code in the context of Class.

The problem

Casting function toString defines what is it, not how it was created.
Before, RHL did update, only if "text" got changes, but it was changed #949 and now they always got updated.

This causes #995, #978, #969, #984

RHL have rights only to update "own" functions, but could not detect them.

Solution

  1. Revert v4 does not use updated non-React modules #949
  2. Update "arrow" functions only if text got changed. Or if "this" keysword is found inside (probably should fix v4 does not use updated non-React modules #949 back)
  3. Update bound class methods. Ie those ones, which names exists on prototype.
@theKashey
Copy link
Collaborator Author

Look like it is impossible to properly "hot" update class, and even heavy babel magic will not work.
We should not try to find a way to fix the problem, which does not exists.

The right way:

  1. Create a new class
  2. Copy state and all variables you have to keep from old component to the new one
  3. Switch to the new component.

Not the change current component, but adopt a new one

This require a one line change in React-Fiber - https://github.com/facebook/react/blob/dd5fad29616f706f484938663e93aaadd2a5e594/packages/react-reconciler/src/ReactChildFiber.js#L375
The similar changes as (not yet) was made to Preact - preactjs/preact#1120

@theKashey
Copy link
Collaborator Author

fixed in v4.3.0

@xinggangling
Copy link

nice

@theKashey theKashey changed the title 🔥Not all methods could be updated 🔥Not all methods could|should be updated Dec 17, 2018
@theKashey
Copy link
Collaborator Author

To be more concrete - this issue is NOT CLOSED.

This issue addresses the problem when class methods should not be updated, or this action would break your code - as a result, some methods will not be updated, while and could, and should.
Today this leads to partial incompatibility with React-Redux v6.

maxgabut added a commit to maxgabut/react-hot-loader that referenced this issue Jan 29, 2020
Use the full URL to link to issue gaearon#1001 in the readme.
I think the short form that was used (`gaearon#1001`) only works in comments and PR messages.
theKashey pushed a commit that referenced this issue Feb 11, 2020
Use the full URL to link to issue #1001 in the readme.
I think the short form that was used (`#1001`) only works in comments and PR messages.
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

2 participants