Compatibility issues with mobx-react

[Venryx]

This library appears to have conflicts with mobx-react.

Here is the thread where I discovered the issue, along with analysis of the cause: mobxjs/mobx-react#797

Summary: Because react-class-hooks replaces this.render after mobx-react has already created a reaction object (attached to the original render function), whenever MobX detects a change, it only calls the original render function -- not the wrapper that react-class-hooks added late (onto the instance).

What problems does it cause?

1. It causes mobx-react to never clean up the reaction, causing a memory leak and potential drop in performance (from the reaction continuing to update, even past the component's unmounting).
2. It likely causes react-class-hooks to not store/retrieve its data properly when re-renders are triggered by MobX. This is because the this.render override by react-class-hooks would not be called, meaning the stack counter would not be reset, leading to it just keeping on adding more cells instead of reading from existing ones.

[Venryx]

I've come up with a userland-only fix for this issue. However, brace yourself as it is not pretty.

Behold my masterpiece of clean code: 😆

export function ClassHooks(targetClass: Function) {
	const componentWillMount_orig = targetClass.prototype.componentWillMount;
	targetClass.prototype.componentWillMount = function() {
		const MAGIC_STACKS = GetMagicStackSymbol(this);
		// by initializing comp[MAGIC_STACKS] ahead of time, we keep react-class-hooks from patching this.render
		if (!this[MAGIC_STACKS]) this[MAGIC_STACKS] = {};
		if (componentWillMount_orig) return componentWillMount_orig.apply(this, arguments);
	};

	const render_orig = targetClass.prototype.render;
	// note our patching Class.render, not instance.render -- this is compatible with mobx-react
	targetClass.prototype.render = function() {
		const MAGIC_STACKS = GetMagicStackSymbol(this);
		// apply the stack-resetting functionality normally done in the on-instance patched this.render
		Object.getOwnPropertySymbols(this[MAGIC_STACKS]).forEach(k=>{
			this[MAGIC_STACKS][k] = 0;
		});
		return render_orig.apply(this, arguments);
	};
}

let magicStackSymbol_cached: Symbol;
export function GetMagicStackSymbol(comp: Component) {
	if (magicStackSymbol_cached == null) {
		const instanceKey = React.version.indexOf("16") === 0 ? "stateNode" : "_instance";
		const ReactInternals = React["__SECRET_INTERNALS_DO_NOT_USE_OR_YOU_WILL_BE_FIRED"];
		const compBeingRendered_real = ReactInternals.ReactCurrentOwner.current;

		const compBeingRendered_fake = {render: ()=>({})};
		ReactInternals.ReactCurrentOwner.current = {[instanceKey]: compBeingRendered_fake};
		useRef(); // this triggers react-universal-hooks to attach data to the "comp being rendered" (fake object above)
		//useClassRef(); // variant, if only using the underlying react-class-hooks library
		ReactInternals.ReactCurrentOwner.current = compBeingRendered_real;

		// now we can obtain the secret magic-stacks symbol, by iterating the symbols on compBeingRendered_fake
		const symbols = Object.getOwnPropertySymbols(compBeingRendered_fake);
		const magicStackSymbol = symbols.find(a=>a.toString() == "Symbol(magicStacks)");
		magicStackSymbol_cached = magicStackSymbol;
	}
	return magicStackSymbol_cached as any; // needed for ts to allow as index
}

Usage:

@observer
@ClassHooks
class MyComp extends Component {
	@observable name = "Mob Me";
	render() {
		// hurray, now we can use mobx-react
		console.log(this.name);
		// *and* react-universal-hooks (or the underlying react-class-hooks)
		const [name, setName] = useState("Hook Me");
	}
}

Note that about half of the code is just to be able to obtain the MAGIC_STACKS symbol created within react-class-hooks. If react-class-hooks were to expose the symbol in its exports, the fix above would be much shorter to implement.

Alternative ways to get the MAGIC_STACKS symbol:

1. Monkey-patch the Symbol.for function before react-class-hooks first executes, so that we can "grab" the symbol as soon as it's created. It's not quite as "plug and play" as my hack above though, since it requires that you add code in two different areas (with one having to run before the react-class-hooks module is even first run/accessed/imported).
2. Use something like string-replace-webpack-plugin to change the code of react-class-hooks to expose the MAGIC_STACKS symbol.
3. Kindly ask the creator of react-class-hooks to expose the symbol through the exports. ^_^

[Venryx]

Here are some ideas for how to fix the issue within the library directly, removing the need for the above hack:

1) Provide an official decorator to modify the componentWillMount and render functions, similar to that seen above. It, of course, would already have access to the MAGIC_STACKS symbol, reducing the complexity a lot.

Actually, it wouldn't need to override the class componentWillMount -- only render. This is because, it already has an entry point for running code before the first hook executes: just after the const self = getMagicSelf(); line in useMagicStack().

Anyway, if this route is taken, perhaps we could make it optional to use the decorator, as something you only need to do if using the library with mobx-react; elsewhere, one could continue using the current implicit way. (where you don't need a class decorator)

EDIT: You could even wrap the @observer decorator of mobx-react to auto-apply the @ClassHooks decorator, meaning all the end developer has to do is use @ObserverWithHooks instead of @observer.

2) There are probably others, but I've spent too much time on this already, so I'm leaving further ideas up to whoever else hits this issue. XD

[FrobtheBuilder]

Oh you gotta be kidding me, I bet this was the real source of that crash bug. I was using class hooks in a component decorated by mobx-react! Guess I better apply this fix to those instances.

[salvoravida]

hi, i will release a fix asap later this night. even if mobx-react think that it's the only one that can swap this.render LOL

i'm thinking about runtime patching mobx, so you have to do nothing.
if it will not be possible (i'm not an expert of mobx -> Redux Rocks! :D)
i will export MAGIC_STACK with your decorator solution.

stay tuned! 🎉

[salvoravida]

// TODO: archive this repo and merge in react-universal-hooks.