facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.28k stars 46.96k forks source link

React Root Index with Math.random() causes Invariant Violations with React compiled as V8 Snapshot #6192

Closed virgofx closed 7 years ago

virgofx commented 8 years ago

Using V8 as a backend with snapshots enabled. To optimize performance, it is beneficial to create snapshots to improve server side rendering performance. After creating snapshot with React.js + ReactServer.js, the resulting server side component rendering errors as follows:

Invariant Violation: Trying to release an instance into a pool of a different type.

The logic that is causing the error is the Math.random() function, which has issues with snapshots and V8. There are 5 areas where this function is used. At the moment, the biggest problem for server side rendering is the following location; however, the other 4 will also impact if code reached the Math.random() at runtime.

/**
 * Size of the reactRoot ID space. We generate random numbers for React root
 * IDs and if there's a collision the events and DOM update system will
 * get confused. In the future we need a way to generate GUIDs but for
 * now this will work on a smaller scale.
 */
var GLOBAL_MOUNT_POINT_MAX = Math.pow(2, 53);

var ServerReactRootIndex = {
  createReactRootIndex: function () {
    return Math.ceil(Math.random() * GLOBAL_MOUNT_POINT_MAX);
  }
};

Would it be possible to fix this such that server side heap snapshots can be generated without having to worry about issues with server Math.random()? Could some sort of algorithm that doesn't rely on Math.random() be used to generate the nodes, root indexes, etc.?

OS: Ubuntu 14.04 x64 V8: 5.0.104 React: 0.14.7

DevTeamMember commented 7 years ago

Hey did u find the solution for this?

virgofx commented 7 years ago

Currently using a polyfill in V8Js that overwrites Math.random(). I know V8 engine made some changes. I have not tested without using a polyfill for the Math.random() that React uses when doing server side rendering with snapshots.

DevTeamMember commented 7 years ago

That's cool. Have you tried new Date().getTime() for random number? It should be safe on the server side.

virgofx commented 7 years ago

Well, the issue is that React uses Math.random() so I'm not sure if they've switched?

aweary commented 7 years ago

The Chromium bug has been labeled as fixed, so I'm going to assume this has been resolved. In any case, there's nothing semantically wrong with using Math.random() so it would be unlikely that we'd include a change just to address a bug in one platform.

Thanks!