React.createContext with SSR leads to concurrency problems in environments with co-routines

[macrozone]

First some context: i am using react SSR with meteor. Meteor uses node-fibers (co-routines, see https://github.com/laverdet/node-fibers), which enables async code to run synchronously without async/await or generators. So i think that React.renderToString could be interrupted by other code and this leads to the problem, but i am totally unsure.

Given the following code:

const MyContext = React.createContext();

const AppContent = () => (
    <MyContext.Consumer>
    {props => <SomeComponent {...props} />}
    </MyContext.Consumer>
)
const App = props => (
    <MyContext.Provider value={props}>
        <AppContent />
    </MyContext.Provider>
)

Do you want to request a feature or report a bug?

Bug

What is the current behavior?

If <App /> gets rendered with different properties roughly at the same time, e.g.:

execution 1: <App foo="execution1" />

execution 2: <App foo="execution2" />

then SomeComponent might render twice with the same props (either twice execution1 or execution2

If the current behavior is a bug, please provide the steps to reproduce and if possible a minimal demo of the problem. Your bug will get fixed much faster if we can run your code and it doesn't have dependencies other than React. Paste the link to your JSFiddle (https://jsfiddle.net/Luktwrdm/) or CodeSandbox (https://codesandbox.io/s/new) example below:

See above

What is the expected behavior?

Context consumer should only respect the current tree and pick up the value from the parent provider even in environments with fibers (co-routines)

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

16.5.2

[macrozone]

This comment also describes this problem/limitation: https://github.com/facebook/react/blob/master/packages/react/src/ReactContext.js#L54

[macrozone]

This is a possible workaround for environments with node-fibers (e.g. Meteor). currently testing...

const createFiberSaveReactContext = defaultValue => {
  // see https://github.com/facebook/react/issues/13854
  const context = React.createContext(defaultValue);
  if (Meteor.isClient) {
    return context;
  }
  const getCurrentFiberContextValues = () => {
    const Fibers = require('fibers');
    if (!Fibers.current) {
      throw new Error('no fiber');
    }
    if (!Fibers.current.__reactContextValues) {
      Fibers.current.__reactContextValues = new WeakMap();
    }
    return Fibers.current.__reactContextValues;
  };

  Object.defineProperty(context, '_currentValue', {
    get() {
      return getCurrentFiberContextValues().get(context);
    },
    set(value) {
      getCurrentFiberContextValues().set(context, value);
    },
  });

  return context;
};

[aweary]

This comment also describes this problem/limitation:

The comment in ReactContext is specifically talking about cases where there are concurrent renderers (like using ReactDOM and ReactART at the same time), not actual concurrent execution environments.

React supports standard-compliant JavaScript environments, so using an environment with non-compliant concurrency primitives is going to cause problems.

I see you opened an issue on the Meteor repo. I think that's the right place to report this, and hopefully the Meteor team can look into this for you.

Thanks!

[macrozone]

@aweary while I understand that non-compliant environement might not need supported, i still think that this is a limitation of react and therefore belongs here as a feature request, or on a FR-board for react.

I created a meteor package that monkey-patches React.createContext: https://github.com/panter/meteor-fiber-save-react-context/blob/master/fiber-save-react-context.js

this of course does not belong in React, but maybe an api to facilitate such support? Maybe it could also help with the _currentValue2-workaround

[gaearon]

I think perhaps #14182 fixed this too? Try 16.6.3.

[macrozone]

@gaearon thank you! I tried to update my app to 16.6.3, but on concurrent server-renders I now get:

W20181113-11:53:44.084(1)? (STDERR) server render failed TypeError: Cannot read property 'debugElementStack' of undefined
W20181113-11:53:44.085(1)? (STDERR)     at pushElementToDebugStack (/path/to/app/node_modules/react-dom/cjs/react-dom-server.node.development.js:2474:11)
W20181113-11:53:44.085(1)? (STDERR)     at resolve (/path/to/app/node_modules/react-dom/cjs/react-dom-server.node.development.js:2709:7)
W20181113-11:53:44.085(1)? (STDERR)     at ReactDOMServerRenderer.render (/path/to/app/node_modules/react-dom/cjs/react-dom-server.node.development.js:3098:22)
W20181113-11:53:44.086(1)? (STDERR)     at ReactDOMServerRenderer.read (/path/to/app/node_modules/react-dom/cjs/react-dom-server.node.development.js:3057:29)
W20181113-11:53:44.086(1)? (STDERR)     at renderToString (/path/to/app/node_modules/react-dom/cjs/react-dom-server.node.development.js:3524:27)

this does not happen if i just do one call to the server, only when another rendering is happening.

Need to investigate more into that though.

[macrozone]

@gaearon actually this error above Cannot read property 'debugElementStack' of undefined happens since react 16.5, because pushCurrentDebugStack and popCurrentDebugStack rely on their order and use the same global array to store the stack frames.

I opened an issue on meteor about that meteor/meteor#10364, but it's not a problem that cannot be solved easily from their side, unfortunatly.

So personally, i need to stick with meteor 16.4.2 for the moment

[gaearon]

Yeah we don't really write code with the assumption that every render can be interrupted. We've started work on a new SSR that would support that (although through our own mechanism — Suspense) but it's going to be many months before it ships.

[macrozone]

@gaearon of course, i understand this ;-) but gladly this error (and the now fixed createContext bug) are the only problems i noticed so far.

[CaptainN]

Is this still an issue with react ssr and meteor?

[macrozone]

@CaptainN: no, it works so far. #13854 (comment) can still happen on develop mode, but that's not severe as it not happens on production builds.

Be aware that this concurrency problem might still arise in the future if you use fiber to yield (and yielding happens during one render) because that is a non-standard js runtime and most libraries do not expect to be run in such an environment.

[CaptainN]

I'm getting a memory leak in my Meteor/React SSR code, which seems to have started after I migrated a bunch of data stuff to use hooks instead of a HoC. Do you think it's possible something similar to this issue in the hooks implementation is causing that?

[eugle]

The problem is still there

1. React 16.13.1
2. Meteor 1.10.2

I20200601-09:21:10.637(8)? TypeError: Cannot read property 'debugElementStack' of undefined
I20200601-09:21:10.637(8)?     at pushElementToDebugStack (/Users/mozibrand/Documents/git/me/jimuku/node_modules/react-dom/cjs/react-dom-server.node.development.js:2677:11)
I20200601-09:21:10.637(8)?     at resolve (/Users/mozibrand/Documents/git/me/jimuku/node_modules/react-dom/cjs/react-dom-server.node.development.js:2953:7)
I20200601-09:21:10.638(8)?     at ReactDOMServerRenderer.render (/Users/mozibrand/Documents/git/me/jimuku/node_modules/react-dom/cjs/react-dom-server.node.development.js:3435:22)
I20200601-09:21:10.638(8)?     at ReactDOMServerRenderer.read (/Users/mozibrand/Documents/git/me/jimuku/node_modules/react-dom/cjs/react-dom-server.node.development.js:3373:29)
I20200601-09:21:10.638(8)?     at renderToString (/Users/mozibrand/Documents/git/me/jimuku/node_modules/react-dom/cjs/react-dom-server.node.development.js:3988:27)
I20200601-09:21:10.638(8)?     at process (/Users/mozibrand/Documents/git/me/jimuku/node_modules/@apollo/react-ssr/lib/react-ssr.cjs.js:36:16)
I20200601-09:21:10.638(8)?     at /Users/mozibrand/.meteor/packages/promise/.0.11.2.gh9fq7.gg3jw++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/meteor-promise/fiber_pool.js:43:40
I20200601-09:21:10.638(8)?  => awaited here:
I20200601-09:21:10.639(8)?     at Function.Promise.await (/Users/mozibrand/.meteor/packages/promise/.0.11.2.gh9fq7.gg3jw++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/meteor-promise/promise_server.js:56:12)
I20200601-09:21:10.639(8)?     at imports/startup/server/ssr.js:48:16
I20200601-09:21:10.639(8)?     at /Users/mozibrand/.meteor/packages/promise/.0.11.2.gh9fq7.gg3jw++os+web.browser+web.browser.legacy+web.cordova/npm/node_modules/meteor-promise/fiber_pool.js:43:40