React component state gets lost when jsx tag below gets removed

[hoschi]

Description

React component state gets lost when jsx tag below gets removed, when one above gets removed it works fine.

Steps:

• fetch demo repo: https://github.com/hoschi/bare-minimum-react-hot-rr4-redux/tree/loosing-state
• start dev server and open site in browser: http://localhost:8080/page3
• use spinner to change number to e.g. 6, which is saved in component state of Page1

Expected behavior

• remove 'above': value of in put should be still '6'
• remove 'below': value of in put should be still '6'

Actual behavior

• remove 'above': value of in put is still '6' (works)
• remove 'below': value of in put is '1' (doesn't work)

Environment

React Hot Loader version: 3.0.0-beta.6
https://github.com/hoschi/bare-minimum-react-hot-rr4-redux/blob/559a9bfce52d97fa48a314f404e8255f74ec81d5/package.json#L19

Run these commands in the project folder and fill in their results:

1. node -v: 6.3.1
2. npm -v: 3.10.3

Then, specify:

1. Operating system: Linux (Arch)
2. Browser and version: Chrome, 56.0.2924.87 (64-bit)

Reproducible Demo

https://github.com/hoschi/bare-minimum-react-hot-rr4-redux/tree/loosing-state

[gregberge]

Working on it.

[gregberge]

@theKashey https://github.com/neoziro/bare-minimum-react-hot-rr4-redux/tree/loosing-state this looks weird, state is lost.

[theKashey]

Above..
There is 2 issues with code itself:

1. Page1 starts timers on construction, but should on mount.
2. by the time page1-3 were executed react-hot-loader is NOT (in RootApp).

This is a quite important moment, and we should point on it in readme, and maybe add some reporting via babel plugin.
__ you have to import react-hot-loader before any other component __ Or REACT_HOT_LOADER variable, all components will register into will not exists.
By fact - on the first change you will got all components remounted, cost they all were auto-wrapped before, and now they are registered ones.

There is also an issue with React itself:

As long prev and next elements are completely different - it will unmount/mount all the things.

To make it work one has to add keys to rendered elements, letting React know that's happening.

<p key="above">above, remove this and Page1 looses statrkke</p>
<Page1 key="page"/>
<p key="below">below, remove this and Page1 keeps state</p>

Result: there is nothing to be fixed in RHL, but there is one moment to improve.

[gregberge]

@theKashey

• I tried to use keys and it didn't solve it
• Timer was just a test, I clean it up using didMount / willUnmout but it is not the point
• So for you this is "normal" and we should just mention it in README?

[gregberge]

OK I retried it with last version, two problems:

• Removing "above" cause a unmount / mount, this is React behaviour, using key no problem
• Hot Reloading does not work the first time, and we can fix it

Instead of ignoring it if __REACT_HOT_LOADER__ is not present we could create a __REACT_HOT_LOADER__ temp in Babel and replace it by the true one when patch.js is loaded. This is exactly the strategy of Google Analytics with global ga.

react-hot-loader/packages/react-hot-loader/src/babel.js

Lines 30 to 40 in 4152093

	// because it otherwise breaks the output of the babel-node REPL (#359).
	const buildTagger = template(
	`
	var UNUSED = (function () {
	if (typeof __REACT_HOT_LOADER__ === 'undefined') {
	return;
	}
	REGISTRATIONS
	})();
	`,

@theKashey what do you think? I think it is better than including patch.js by ourself because we do not know where we are with Babel.

[gregberge]

Described this in a separated issue #738.

[hoschi]

Removing "above" cause a unmount / mount, this is React behaviour, using key no problem

You mean that bug is expected behaviour, because there are two p tags without keys?

[gregberge]

This is not a bug, this is exactly the behaviour of React, React uses index as key by default to changing the order triggers a unmount / mount.