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.

Sign up for GitHub

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

Jump to bottom

Stage is killing the Context #349

Closed
macabeus opened this issue Mar 30, 2019 · 7 comments
Closed

Stage is killing the Context #349

macabeus opened this issue Mar 30, 2019 · 7 comments

Comments

@macabeus
Copy link

macabeus commented Mar 30, 2019 •
edited
Loading

I'm developing a project using provider pattern. So I'm passing informations between my components using context.

Then, at Map component I have this:

const Map = () => {
  const { ... } = useContext(VisionContext).vision // I can use VisionContext here

  return (
    <Fragment>
      <Stage>
        <TilemapLayer />
    ...
  )
}

And for some unknown reason, I have a problem at TilemapLayer because the context didn't arrive at this component:

const TilemapLayer = ({ setSelectedPointInfos }) => {
  const { ... } = useContext(VisionContext).vision // Error! "useContext(VisionContext)" returns "undefined" here!

Debugging it I replaced the Stage to div at return of Map:

const Map = () => {
  ...

  return (
    <Fragment>
      <div>
        <TilemapLayer />
    ...
  )
}

And with this code, I can use useContext(VisionContext) at TilemapLayer. I tried replacing div for others components as well, and it's worked.

Because of it, I think that Stage is killing the Context - what is a bug.
@lavrton
Copy link
Member

lavrton commented Mar 31, 2019

react-konva is a custom renderer. At the current moment, it can't have access to contexts of the parent tree. I can't find a way to may it work automatically.

So react-konva reset all the stack. The good news is that you can "bridge" contexts manually. The idea is simple. Consume the context BEFORE Stage component. Then provide it again (with the same value) INSIDE Stage:

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>
  );
};

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

@lavrton lavrton closed this as completed Mar 31, 2019
@ribx
Copy link

ribx commented Jun 2, 2019 •
edited
Loading

I was checking for this issue and saw that other libraries use the getPublicRootInstance function, when they render their root element:

https://github.com/facebook/react/blob/5763f1d4c458b9d9a769a28b9d92b698bc94533f/packages/react-dom/src/client/ReactDOM.js#L584

while react-konva uses

react-konva/src/ReactKonvaCore.js

Lines 89 to 99 in f1de915

return (
<div
ref={ref => (this._tagRef = ref)}
accessKey={props.accessKey}
className={props.className}
role={props.role}
style={props.style}
tabIndex={props.tabIndex}
title={props.title}
/>
);

to render the stage into the div given by the ref.

Do I understand this correctly?

@ribx
Copy link

ribx commented Jun 2, 2019 •
edited
Loading

Maybe this implementation would help:

https://github.com/michalochman/react-pixi-fiber/blob/3a6fce8e53adf564f7395e314542a12a935a2f19/src/Stage.js#L59-L74

@lavrton
Copy link
Member

lavrton commented Jun 3, 2019

@ribx how it may help? react-pixi-fiber supports fully contexts?

@ribx
Copy link

ribx commented Jun 3, 2019

I thought it does. I found it only because of this PR:

https://github.com/michalochman/react-pixi-fiber/pull/84/commits

But maybe things changed again in react-reconciler. Many projects using it, but there is no documentation provided by facebook yet.

So I will go for replicating all my providers. Thanks.

@ejarzo ejarzo mentioned this issue Mar 12, 2020
Merged
heat1q added a commit to boardsite-io/boardsite that referenced this issue Jan 28, 2021
@heat1q
Explicitly provide Redux store context to Layers
446e1c0 
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.
@lavrton
Copy link
Member

lavrton commented Sep 23, 2022

Update on this issue. From [email protected], context bridge should work by default. It will be really cool if someone can try it and provide feedback.

@BryceDalbey
Copy link

I've just tried it out with [email protected] and had no problems. I also bridged into the <Html> wrapper with no issues, though I haven't tested that as much.

Not sure this is the right place for this but just in case others are doing the same as me: I originally bridged into the stage to use a customization menu from outside the stage but once I had enough elements inside the stage (~15 1000x500px partly transparent images with RGB filters) the latency became noticeable (since every component in the stage would run on each state update). So I moved the menu inside the stage and separated it out so I could wrap each part and the layers it interacts in a separate context (hence having to bridge into html wrappers). This way makes organizing the menu a pain since each part is inside a separate html wrapper but the performance is obviously much better.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@lavrton @ribx @macabeus @BryceDalbey