react-art + React Context API is not working correctly

[lavrton]

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

Bug

What is the current behavior?

When you use react-art rendrer, the Consumer is picking up the default value defined by the initial creation of the context rather than the values provided by Provider.

Actual behabiour: Shape has y = 10 (initial value).

import React from "react";
import { render } from "react-dom";
var ReactART = require("react-art");
var Group = ReactART.Group;
var Shape = ReactART.Shape;
var Surface = ReactART.Surface;

var RED_DOT_PATH =
  "M12.5,17 C16.0898511,17 19,14.0898511 19,10.5 C19,6.91014895 16.0898511,4 12.5,4 C8.91014895,4 6,6.91014895 6,10.5 C6,14.0898511 8.91014895,17 12.5,17 Z M12.5,17";

const { Consumer, Provider } = React.createContext({ x: 0, y: 10 });

const App = () => (
  <Provider value={{ x: 0, y: 100 }}>
    <Surface width={700} height={700}>
      <Consumer>
        {({ x, y }) => {
          return <Shape x={x} y={y} fill="#D97B76" d={RED_DOT_PATH} />;
        }}
      </Consumer>
    </Surface>
  </Provider>
);

render(<App />, document.getElementById("root"));

Demo: https://codesandbox.io/s/llx6kv6527

What is the expected behavior?

Shape should have y = 100 (provided value).

v16.3.2 for react, react-dom and react-art

[gaearon]

Have you tried on master? I think #12779 fixed this (although the solution is a bit gross). For it to work you’ll need to specify whether your renderer is “primary” or “secondary” (and more than two won’t work).

[gaearon]

I’ll assume it’s fixed but if not let me know.

[lavrton]

@gaearon
The issue is not fixed with new v16.4.0. I updated deps on the demo, but the result is the same: https://codesandbox.io/s/llx6kv6527

[aweary]

@gaearon I looked into this. ReactART.Surface renders its surface element (canvas in this case) and then calls ARTRenderer.updateContainer when the component mounts, so ReactDOM commits before the Consumer is rendered. completeWork walks up the tree and pops the Context provider. When the Consumer is eventually rendered it falls back to the default value, since the Provider was popped.

It seems like popping the context Provider isn't safe if a child might call a secondary renderer after the primary renderer commits.

[gaearon]

@acdlite Was #12779 only for Fabric?

[aweary]

@gaearon the context is being shared successfully between the renderers, with ReactDOM being the primary renderer. I believe the reason this wasn't caught was because the test added for ReactART renders the Provider inside the Surface component that's managing the ARTRenderer, so the Provider's value isn't being passed through the renderer boundary.

The example @lavrton renders the Provider outside of the Surface component. If we change it so that the Provider is rendered as a child it works.

[gaearon]

I'll close this in favor of #13332 (which I think is the real issue here).

[aweary]

@gaearon is react-art using createPortal? I didn't think it was

[gaearon]

No, because createPortal doesn't support cross-renderer portals. But that was the initial vision for portals. We just never implemented it.

Portals would be the mechanism by which you put one renderer into another.

[gaearon]

I also filed #13336 which is a bit more concrete about this use case.