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

Support of React-Redux 6.0.0 #311

Closed
bini1988 opened this issue Dec 25, 2018 · 14 comments
Closed

Support of React-Redux 6.0.0 #311

bini1988 opened this issue Dec 25, 2018 · 14 comments

Comments

@bini1988
Copy link

Hello,
I am trying to use latest react@16.7.0 and react-redux@6.0.0 modules with React Konva and I get errors:

browser.js:38 Uncaught Error: Could not find "store" in the context of "Connect(Component)". Either wrap the root component in a , or pass a custom React context provider to and the corresponding React context consumer to Connect(Component) in connect options.

It seems that React Konva goes wrong when I use redux connect function with konva shapes.
Reproducible demo: https://codesandbox.io/s/v6v2znvqql

@lavrton
Copy link
Member

lavrton commented Dec 25, 2018

The code above works with react-redux v5. react-redux is changed underlying context mechanism.

The issue should be related to #188

@TimmyGOGO
Copy link

TimmyGOGO commented Jan 14, 2019

Same problem. I have a wrapped in a <Portal> element "PopoverContainer" on <Stage>. And I wanted to transfer logic with handling events and redux actions to another file, but then got an error of:

Uncaught Error: Could not find "store" in the context of "Connect(PopoverContainer)". Either wrap the root component in a , or pass a custom React context provider to and the corresponding React context consumer to Connect(PopoverContainer) in connect options.

Is it possible to make it work if we wrap Component in <Provider store={this.props.store}>...</Provider>?

@TimmyGOGO
Copy link

I tried to pass redux store as a prop to Components, but faced next problem:

react-dom.development.js:55 Uncaught Error: Element type is invalid: expected a string (for built-in components) or a class/function (for composite components) but got: undefined. You likely forgot to export your component from the file it's defined in, or you might have mixed up default and named imports.

I suggest this is because I pass store to a decorated Component, which is exported like this:
export default connect( mapToStateProps, mapDispatchToProps )(Component);

@lavrton
Copy link
Member

lavrton commented Jan 15, 2019

The current workaround is to bridge store in Stage component.

import { ReactReduxContext, Provider } from "react-redux";


       // later in render:
      <ReactReduxContext.Consumer>
        {({ store }) => (
          <Stage width={window.innerWidth} height={window.innerHeight}>
            <Provider store={store}>
              <Layer>
                {staticTargets.map(t => (
                  <Target target={t} key={t.id} />
                ))}
              </Layer>
              <Layer name="dragging-layer">
                {movingTargets.map(t => (
                  <Target target={t} key={t.id} />
                ))}
              </Layer>
            </Provider>
          </Stage>
        )}
      </ReactReduxContext.Consumer>

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

@TimmyGOGO
Copy link

Thanks a lot, @lavrton!

@ArgonAlex
Copy link

ArgonAlex commented Feb 12, 2019

It should be noted that React Redux v6 as is, using the new Context API is being rethought. They are going back to direct subscriptions with a new implementation in another release, possibly v7.

reduxjs/react-redux#1177

@markerikson
Copy link

However, we will still be using the same context mechanism as v6 to make the store available to the components. The difference will be in how the components receive updated store state values.

@veksa
Copy link

veksa commented Apr 5, 2019

FYI:
Same problem with the context mechanism in the react-fela, and custom components with a context.
It's a little uncomfortable reuse all context providers after .

@xxlee0927
Copy link

The solution @lavrton provide may encounter the issue that child wrapped in <Provider> sometimes receive updated redux value earlier than parent. Therefore I need to perform additional condition checking to prevent bugs. Hope to have full support of context.

@lavrton
Copy link
Member

lavrton commented Sep 30, 2019

This final note here.

react-redux is using a context mechanism to make the store available for components. But in the way react-konva is implemented, the context information is lost inside <Stage> component. react-konva is creating a "fresh renderer". So it has no information about root contexts. At the current moment, I don't see any ways to bridge this automatically.
We have to do to it manually:

import { ReactReduxContext, Provider } from 'react-redux';

const Canvas = () => {
  return (
    <ReactReduxContext.Consumer>
      {/* consume store from context */}
      {({ store }) => (
        <Stage width={window.innerWidth} height={window.innerHeight}>
          {/* inject store into context */}
          <Provider store={store}>
            <SomeCanvasComponents />
          </Provider>
        </Stage>
      )}
    </ReactReduxContext.Consumer>
  );
};

As @xxlee0927 mentioned:

The solution @lavrton provide may encounter the issue that child wrapped in sometimes receive updated redux value earlier than the parent.

And I had exactly this issue in the last project. I will try to give the full example of the issue and the solution I used to fix the error.

Let us think we have a store:

{
  wallsIds: ['v1', 'v2'],
  walls: {
    v1: { fill: 'red' },
    v2: { fill: 'blue' }
  }
}

Now we have a component that creates our drawings.

const Canvas = ({ wallsIds }) => {
  return (
    <ReactReduxContext.Consumer>
      {({ store }) => (
        <Stage width={window.innerWidth} height={window.innerHeight}>
          <Provider store={store}>
            <Layer>
              {wallsIds.map(id => <Wall id={id} />)}
            </Layer>
          </Provider>
        </Stage>
      )}
    </ReactReduxContext.Consumer>
  );
};

export default connect(store => ({ wallsIds: store.wallsIds }))(Canvas);

And we have a Wall component that connected into the store and reads its full properties (by having its id):

const Wall = ({ wall }) => {
   return <Rect fill={wall.fill} />
}

export default connect((store, props) => ({ wall: store.walls[props.id] }))(Wall);

This issue is that on store update, canvas components connected inside <Stage /> will receive new state BEFORE the parent component. That means <Wall /> will know about the update before <Canvas />. And that can produce the bug when you are trying to remove the element. Like, remove a wall with id v1 from the store. Here is how the error will happen:

On remove, the store will have no information about wall v1. <Wall /> component see the update and it is trying to update itself. On update, it reads wall information from the store: { wall: store.walls[props.id] }. But it will be undefined. So in your case, we will have an error when we are trying to read wall.fill. The <Canvas/> is not updated yet, so it still render "old" data when we still have the wall.

How we can make sure that properties are in sync?

The workaround is simple. We can't use information from the store that we received in <Canvas> components to draw something inside <Stage/>. In other words, don't use this.props for components inside <Stage />. Like we used this:

{wallsIds.map(id => <Wall id={id} />)}

wallsIds is a prop we have from the connected store.

How to rewrite this code?

We can just wrap whole content inside <Stage /> into another component:

const WallsContainer = ({ wallsIds }) => {
  return {wallsIds.map(id => <Wall id={id} />)};
};

export default connect(store => ({ wallsIds: store.wallsIds }))(WallsContainer);

And then use it instead:

<Stage width={window.innerWidth} height={window.innerHeight}>
  <Provider store={store}>
    <Layer>
      <WallsContainer />
    </Layer>
  </Provider>
</Stage>

I hope I was clear in this large message. If you still have any issues with it, please add your comment.

I am going to close the issue for now because I don't think we can do anything else at the current moment.

@lavrton lavrton closed this as completed Sep 30, 2019
heat1q added a commit to boardsite-io/boardsite that referenced this issue Jan 28, 2021
This is a known issue of react-konva, see eg.
konvajs/react-konva#349
konvajs/react-konva#311

Revert some changes made to drawControl and boardControl
reducers.
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
@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.

@ierehon1905
Copy link

ierehon1905 commented Jan 8, 2023

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.

It works. Thank you!

The only issue is with hot reload during development. But we are trying to solve it on hmr side

@afdallah
Copy link

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.

Tested, It works. Thank you @lavrton

@folbo
Copy link

folbo commented Aug 11, 2023

We can't use information from the store that we received in components to draw something inside . In other words, don't use this.props for components inside

What if one needs to use values from redux store in Canvas component, for example

function MyComponent() {
  var state = useSelector(state => state.canvas);
  return (
    <Canvas onMouseDown={(e) => {
      const worldPos = state.scale * mousePos;
      console.log(worldPos);}}>
      <Provider store={store}>
        <Layer>
          <WallsContainer />
        </Layer>
      </Provider>
    </Canvas>
  );
}

?

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

10 participants