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

this.state becoming null in mobx-react observer-wrapped component #1235

Open
yang opened this issue Apr 26, 2019 · 9 comments
Open

this.state becoming null in mobx-react observer-wrapped component #1235

yang opened this issue Apr 26, 2019 · 9 comments
Labels

Comments

@yang
Copy link

yang commented Apr 26, 2019

Description

What you are reporting:

Bug (or my own misunderstanding). When I edit a stateful class component that is wrapped in an observer() from mobx-react, this.state becomes null at some point, during a render attempt.

Expected behavior

What you think should happen: this.state should never change to null.

Actual behavior

What actually happens: this.state changes to null.

Environment

React Hot Loader version:

Run these commands in the project folder and fill in their results:

  1. node -v: 10.15.1
  2. npm -v: 6.8.0
  3. yarn -v: 1.13.0

Then, specify:

  1. Operating system: MacOS 10.14.4
  2. Browser and version: Chrome 73.0.3683.103

Reproducible Demo

I just did create-react-app, eject, and added hot-loader + mobx-react, before reproing the bug:

https://github.com/yang/hot-cra-observer-repro

Video of reproing the error: you must first click to toggle off the Widget, then trigger a hot-reload in the Widget, and then click to toggle the Widget back in.

https://youtu.be/47AaO8IEwqo

@theKashey theKashey added the bug label Apr 26, 2019
@theKashey
Copy link
Collaborator

100% due #1207

@theKashey
Copy link
Collaborator

Running your example right now, and it's working without any issues.

@yang
Copy link
Author

yang commented May 15, 2019

Hm, that's odd. I just tried this repo on a different host, and am still seeing the same issue.

Do you have a similar video of your interaction? Are you doing the steps of: toggle, hot-reload, toggle? Are you using all the same platform versions?

Let me know also if I can submit more videos, if I can do any debugging on your behalf, or if there's anything else I can do from my end to help with this issue report. Thanks for taking the time to look into this.

@theKashey
Copy link
Collaborator

Tried again in the "right" order and it got broken :)

@theKashey
Copy link
Collaborator

You know - we had almost the same issue before, I clearly remember it.
But right now I could not understand what's happening

Screen Shot 2019-05-15 at 5 56 24 PM

Look like Symbols are different.... oh! It was the same issue before! - mobxjs/mobx-react#522

@theKashey
Copy link
Collaborator

Oh, holy 💩! I make a wrong PR! That's what's happening without tests.

It uses Symbol(name), while it shall use Symbol.for(name). Could I gently ask you to fix my mistake and open PR agains mobx-react?

@theKashey
Copy link
Collaborator

Another approach is to use newSymbol instead of createSymbol - mobxjs/mobx-react@98396f0

@theKashey
Copy link
Collaborator

....and that's already made?

@theKashey
Copy link
Collaborator

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

No branches or pull requests

2 participants