Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

React Konva doesn't work with React 16.3 Context API #188

Closed
Guria opened this issue Apr 7, 2018 · 26 comments
Closed

React Konva doesn't work with React 16.3 Context API #188

Guria opened this issue Apr 7, 2018 · 26 comments

Comments

@Guria
Copy link

Guria commented Apr 7, 2018

I am trying to use React 16.3 Context API based on render props with React Konva:

import React from "react";
import { Layer, Stage, Circle, Group, Line } from "react-konva";

const { Consumer, Provider } = React.createContext({ width: 0, height: 0 });

const ToolsLayer = () => (
  <Consumer>
    {({ height, width }) => (
      <Layer>
        <Group offsetY={-height} y={-42}>
          <Line
            points={[0, 0, width, 0, width, 42, 0, 42]}
            closed
            stroke="black"
          />
          <Circle radius={11} fill="red" stroke="black" x={21} y={21} />
          <Circle radius={11} fill="green" stroke="black" x={21 + 42} y={21} />
          <Group />
        </Group>
      </Layer>
    )}
  </Consumer>
);

export default function Canvas({
  width = window.innerWidth,
  height = window.innerHeight
}) {
  return (
    <Provider value={{ width, height }}>
      <Stage width={width} height={height}>
        <ToolsLayer />
      </Stage>
    </Provider>
  );
}

And I get runtime error:

Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: object.

Check the render method of `ToolsLayer`.

Reproducible demo: https://codesandbox.io/s/2o9j1r6l30

@lavrton
Copy link
Member

lavrton commented Apr 12, 2018

I published v 1.7.2. Should work ok now.

Updated: currently it doesn't work directly, you have to use a workaround, see #188 (comment)

@lavrton lavrton closed this as completed Apr 12, 2018
@jeremypenner
Copy link

This is still broken on v1.7.2. You can see in the reproducible demo linked above that it's picking up the default value defined by the initial creation of the context ({width: 0, height: 0}) rather than the values provided by Provider.

@lavrton
Copy link
Member

lavrton commented May 14, 2018

Tracking here: facebook/react#12796

@lavrton lavrton reopened this May 14, 2018
@gaearon
Copy link

gaearon commented Aug 7, 2018

facebook/react#13336

@lyleunderwood
Copy link

Here's an example of a quick workaround I put together to try and sort of contain this.

@Guria
Copy link
Author

Guria commented Sep 28, 2018

Any updates here?

@gaearon
Copy link

gaearon commented Sep 28, 2018

Usually when there are updates on an issue, they happen on the issue itself. :-)

But since you're asking, facebook/react#13728 will likely help. It will be in the next React release. That would help you "bridge" the context you care about down.

@lyleunderwood
Copy link

@Guria basically facebook/react#13728 is an API change which will make the necessity of re-introducing your context into the hierarchy at a point below the point at which the context barrier happens (the Stage in the example of react-konva) less ugly. So instead of:

<MyConsumer>
  {value =>
    <Stage>
      <MyContext.Provider value={value}>
        <Stuff />
      </MyContext.Provider>
    </Stage>
  }
</MyConsumer>

you have:

// or whatever your top level react-konva component where you care about context is
class MyStage extends React.Component {
  static contextType = MyContext;
  render() {
    return <Stage>...</Stage>;
  }
}

// ...

<MyStage>
  <Stuff />
</MyStage>

Because facebook/react#13728 doesn't implement any kind of introspection of context solution, I don't think that it offers a fix that react-konva could implement, and I think that facebook/react#13332 would be a potential direction for solving that problem.

Does this sound about right @gaearon ?

@gaearon
Copy link

gaearon commented Sep 28, 2018

Yea

@Zik42
Copy link

Zik42 commented Dec 28, 2018

Is the way to make it work with react-konva? My consumer returns default value of context

@drcmda
Copy link

drcmda commented Feb 28, 2019

Maybe allowing for something like this:

function useForeignContext(context) {
  const { subscribeContext } = useContext(ROOTCONTEXT)
  const [wrappedContext, unsubscribe] = useMemo(
    () => subscribeContext(context)
  , [context])
  useEffect(() => unsubscribe, [context])
  return useContext(wrappedContext)
}

Which would transport context into the reconciler tree. The root component that still belongs to the dom (Stage in konvas case) could provide the subscribeContext function, which adds a dom-tied context into a list and returns a new context that provides changes values into the reconciler tree.

@lavrton
Copy link
Member

lavrton commented Mar 31, 2019

Demo how to "bridge" contexts into react-konva tree:

import React, { Component } from "react";
import Konva from "konva";
import { render } from "react-dom";
import { Stage, Layer, Rect } from "react-konva";

const ThemeContext = React.createContext("red");

const ThemedRect = () => {
  const value = React.useContext(ThemeContext);
  return (
    <Rect x={20} y={50} width={100} height={100} fill={value} shadowBlur={10} />
  );
};

const Canvas = () => {
  return (
    <ThemeContext.Consumer>
      {value => (
        <Stage width={window.innerWidth} height={window.innerHeight}>
          <ThemeContext.Provider value={value}>
            <Layer>
              <ThemedRect />
            </Layer>
          </ThemeContext.Provider>
        </Stage>
      )}
    </ThemeContext.Consumer>
  );
};

class App extends Component {
  render() {
    return (
      <ThemeContext.Provider value="blue">
        <Canvas />
      </ThemeContext.Provider>
    );
  }
}

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

https://codesandbox.io/s/ykqw8r4r21

@lavrton
Copy link
Member

lavrton commented Sep 30, 2019

I am going to close the issue. I don't think we can do anything from our side to resolve the issue and handle context bridging automatically.

@ghost
Copy link

ghost commented Dec 6, 2019

I encountered this issue with my project. After playing with it, I realize that we can skip the bridge by declaring the context consumer inside Stage. So something like this will work just fine.

import React, { Component } from "react";
import { render } from "react-dom";
import { Stage, Layer, Rect } from "react-konva";

const ThemeContext = React.createContext();

const ThemedRect = () => {
  const value = React.useContext(ThemeContext);
  return (
    <Rect x={20} y={50} width={100} height={100} fill={value} shadowBlur={10} />
  );
};

const Canvas = () =>  (
  <Stage width={window.innerWidth} height={window.innerHeight}>
    <ThemeContext.Provider value="blue">
      <Layer>
        <ThemedRect />
      </Layer>
    </ThemeContext.Provider>
  </Stage>
)

class App extends Component {
  render() {
    return <Canvas />
  }
}

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

https://codesandbox.io/s/react-konva-consume-context-demo-d5yht

@lyleunderwood
Copy link

@harryhle That seems unrelated as you're not actually bridging the gap between the two renderers there, both the provider and the consumer are on the react-konva side of the bridge, and therefore no bridging is necessary.

@ghost
Copy link

ghost commented Dec 6, 2019

@lyleunderwood That's right. I actually work around this problem by passing data between two renderers with props, instead of using the second context as bridge.

@dylinmaust
Copy link

Is there a way to bridge react-query's QueryClientProvider or Material UIs MuiThemeProvider using this same technique?

@lyleunderwood
Copy link

lyleunderwood commented Aug 31, 2021

react-query doesn't seem to expose a consumer directly. You could probably do something like:

import { useQueryClient, QueryClientProvider } from 'react-query';

const Canvas = () => {
  const queryClient = useQueryClient();

  return (
    <Stage>
      <QueryClientProvider client={queryClient}>
        <Layer>
          {/* ... */}
        </Layer>
      </QueryClientProvider>
    </Stage>
  );
};

@lyleunderwood
Copy link

lyleunderwood commented Aug 31, 2021

Similarly for MUI we should be able to do something like:

import { ThemeProvider, useTheme } from '@material-ui/core/styles';

const Canvas = () => {
  const theme = useTheme();

  return (
    <Stage>
      <ThemeProvider theme={theme}>
        <Layer>
          {/* ... */}
        </Layer>
      </ThemeProvider>
    </Stage>
  );
};

@dylinmaust
Copy link

@lyleunderwood Thanks! I actually had implemented it in such a way for the <Stage>, but was also using the <Html> component from react-konva-utils, which requires a second bridge. So I mistakenly thought it wasn't bridging correctly.

lynda0214 added a commit to lynda0214/bulletin-board that referenced this issue May 20, 2022
… api

[how]
- implement CommentThread, CommentStarter
- adopt moment.js to display timestamp
- complete remove comment and update comment thread actions
- extract canvas component
- replace prop drilling with context

[reference]
According to the following discussion, react-konva <Stage> will influence the Context API:
konvajs/react-konva#188
Redux is based on Context API, so it is influenced too
konvajs/react-konva#311
@maritaria
Copy link
Contributor

The usage of context api should probably be an example on the offical docs site, there is no mention there of this problem/pitfall and I only found this due to some google searches (which first led me in other directions).

@lavrton
Copy link
Member

lavrton commented Sep 23, 2022

Update on this issue. From react-konva@18.2.2, context bridge should work by default. It will be really cool if someone can try it and provide feedback.

@Nimbo1999
Copy link

I've just tested the new version and the context bridge its working fine on my case 👍

@bekirisgor
Copy link

Thanks for the update that works now :)

@Lucas1Chaves
Copy link

I started to get a new error on version 18.2.2
image

On 18.2.1 it worked even without Bridge component, i'm declaring recoil root on top of the component tree of my app instead of inside stage.

@lavrton
Copy link
Member

lavrton commented Oct 10, 2022

Can you make a demo?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests