Memory leak in React 15.3.0 non-production server side rendering

[franjohn21]

A memory leak appears to have been introduced to server side rendering in React 15.3.0 when NODE_ENV != production.

With NODE_ENV=development, the size of the heap grows proportionally to the number of requests received. This was confirmed in both Node v6.3.0 and v0.12.3. Also it was confirmed to work correctly in React 15.2.0.

To reproduce: Start the app configured for server-side rendering with NODE_ENV=development (or just not production). Inspect the V8 heap before and after hitting your server with a number of requests. I used heapdump to take the snapshots and wrk to hit my app with a bunch of requests. Load the snapshots into chrome dev tools profiler. Here is what I see:

The culprit appears to be the react children mutation warning. onBeforeComponentMount saves a reference to a given React Element and all of its props in the elements object, but the cleanup steps onComponentHasMounted or onComponentHasUpdated are never called on the server and thus the elements object is ever growing.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?
react - 15.3.0
react-dom - 15.3.0
node - 6.3.0 || 0.12.3
express - ^4.0.0

[zpao]

Awesome detective work. We should fix this.

cc @jimfb @spicyj @gaearon Can we thread the transaction through to the hook to determine if we should store the reference? Or is there something else (I haven't looked closely at the actual hook code if there's precedent).

[gaearon]

Hmm, why is this devtool storing elements by itself? They should already be available in ReactComponentTreeDevtool which correctly cleans up on server rendering (we have tests for this). I think this devtool needs to switch to ask ReactComponentTreeDevtool for the element.

[gaearon]

Specifically ReactComponentTreeDevtool.getElement(debugId) should provide the same information. Can you test if using that instead of a local dictionary fixes the issue?

[keyz]

The tricky part here is only onBeforeMountComponent and onMountComponent get called while doing SSR. Currently ReactChildrenMutationWarningHook asserts in onComponentHasMounted and onComponentHasUpdated, which are not called.

I think if we want to do this in the SSR code path we might move the assertion to onMountComponent (and get the element from ReactComponentTreeHook as @gaearon mentioned).