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

Wrapping the app in React.StrictMode causes it not to update #1068

Closed
japhib opened this issue Apr 13, 2022 · 5 comments · Fixed by #1104
Closed

Wrapping the app in React.StrictMode causes it not to update #1068

japhib opened this issue Apr 13, 2022 · 5 comments · Fixed by #1104

Comments

@japhib
Copy link

japhib commented Apr 13, 2022

Hi! This framework is awesome and I'm excited to start using it.

I was following along with the tutorial which is great, and used create-react-app to create the app (as recommended). It worked fine for the most part, BUT with one major problem: the React components did not re-render when the game state updated. I did some investigation and it appears to be a problem with the default create-react-app template.

You can see the issue here: https://codesandbox.io/s/boardgame-io-react-typescript-demo-forked-undhfv
This is a fork of the provided Typescript-React codesandbox that demonstrates the problem.

Changes from the original codesandbox:

  • update react, react-dom, @types/react, and @types/react-dom from version 17 to 18
  • change index.tsx to look like this:
import React from 'react';
import ReactDOM from 'react-dom/client';

import App from './App';

const root = ReactDOM.createRoot(document.getElementById('root')!);
root.render(
  <React.StrictMode>
    <App />
  </React.StrictMode>
);

^^^ Note that this is the default index.tsx that gets generated by the latest create-react-app.

FIX: if you comment out the <React.StrictMode> wrapping <App />, the DOM starts rendering again as the game state updates.

Note: the issue doesn't happen unless using React.StrictMode and ReactDOM.createRoot() / root.render rather than just plain render.

Obviously it's not a huge deal since the fix is pretty straightforward; but probably still worth fixing so it doesn't trip up newbies like me.

@delucis
Copy link
Member

delucis commented Apr 13, 2022

Hi thanks for the report! This looks like it's a new bug with React 18, right? I had no problem with create-react-app in the past.

Any idea why this might be happening? Haven't had a chance to dive into React 18 yet so I'm not fully on top of the changes from 17. If you can figure out the source of the issue, I'd love to take a look at a PR!

@Blarc
Copy link

Blarc commented Apr 18, 2022

I had the same problem. Thanks for the fix.

@japhib
Copy link
Author

japhib commented Apr 18, 2022

@delucis Honestly I have no idea what the issue could be. I'm not familiar enough with the internals of boardgame.io or React to even know where to start. I also don't have enough bandwidth to look into this atm but I figured I'd put this here for reference in the meantime.

@Blarc glad to help!

@delucis
Copy link
Member

delucis commented Apr 18, 2022

No worries — if someone knows the answer, let me know! Otherwise I’ll try to make some time to investigate. The React client is relatively simple if someone wants to take a look: https://github.com/boardgameio/boardgame.io/blob/main/src/client/react.tsx

@alexjurkiewicz
Copy link
Contributor

Removing the componentWillUnmount function of WrappedBoard (in react.tsx) fixes this problem:

    componentWillUnmount() {
      this.client.stop();
      this.unsubscribe();
    }

I'm also not clear why it helps, sadly. I think due to Strict Mode's changes to help detect unexpected side effects, which double invoke all components. It seems like two copies of the component are created, one is mounted and unmounted from the React tree, then the second is mounted. So there must be some logic assuming only a single copy of the component would be created. Possibly related to the debug panel ClientManager..?

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

Successfully merging a pull request may close this issue.

4 participants