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

[DevTools Bug]: Selecting/deselecting boolean from DevTools Component props causing loss of class functions #24781

Closed
a-gehlot opened this issue Jun 23, 2022 · 4 comments · Fixed by #26522

Comments

@a-gehlot
Copy link

Website or app

https://github.com/a-gehlot/react-error

Repro steps

  1. Install webpack, react, babel with "npm install" in terminal.

  2. Bundle app with webpack via "webpack --watch --mode=developement" in terminal.

  3. Open up "index.html" in Chrome and open React DevTools. Under Components, there should be a Dog component within a Person component, where the Person is passed as a prop to the Dog.

  4. Under Dog props, there should be a person object with a value of present being true. If you click the button, the value of present should change to false, the number should switch to 0, and the checkbox should get deselected. However, if you manually select the checkbox to change between true/false, the prop seems to lose its prototype references to the original JS class, as an error stating it cannot find the function will be shown.

Screen.Recording.2022-06-23.at.1.28.54.PM.mov

How often does this bug happen?

Every time

DevTools package (automated)

No response

DevTools version (automated)

No response

Error message (automated)

No response

Error call stack (automated)

No response

Error component stack (automated)

No response

GitHub query string (automated)

No response

@a-gehlot a-gehlot added Component: Developer Tools Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug Type: Bug labels Jun 23, 2022
@mondaychen mondaychen self-assigned this Jun 28, 2022
@mondaychen
Copy link
Contributor

mondaychen commented Jun 28, 2022

Hi
This is because React needs to copy your state before modifying it. When copying a non-primitive state, React assumes it is an object or an array. See: https://github.com/facebook/react/blob/main/packages/react-reconciler/src/ReactFiberReconciler.new.js#L609
In this process, the class instance becomes an object and checkPresence is gone.
We'll need to update react for fix this issue.

@mondaychen mondaychen removed the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jun 29, 2022
@mondaychen
Copy link
Contributor

After discussion with team, we decided to make a change in devtools to stop allowing editing inside classes. Class internals should be opaque.

@official-akshayjadhav
Copy link

@mondaychen is anyone working on its resolution?

@mondaychen
Copy link
Contributor

Hi @official-akshayjadhav. Not yet.

@hoxyq hoxyq self-assigned this Mar 1, 2023
hoxyq added a commit that referenced this issue Apr 3, 2023
## Summary
Fixes #24781

Restricting from editing props, which are class instances, because their
internals should be opaque.

Proposed changes:
1. Adding new data type `class_instance`: based on prototype chain of an
object we will check if its plain or not. If not, then will be marked as
`class_instance`. This should not affect `arrays`, ..., because we do
this in the end of an `object` case in `getDataType` function.

Important detail: this approach won't work for objects created with
`Object.create`, because of the custom prototype. This can also be
bypassed by manually deleting a prototype ¯\\\_(ツ)_/¯
I am not sure if there might be a better solution (which will cover all
cases) to detect if object is a class instance. Initially I was trying
to use `Object.getPrototypeOf(object) === Object.prototype`, but this
won't work for cases when we are dealing with `iframe`.


2. Objects with a type `class_instance` will be marked as unserializable
and read-only.

## Demo
`person` is a class instance, `object` is a plain object


https://user-images.githubusercontent.com/28902667/228914791-ebdc8ab0-eb5c-426d-8163-66d56b5e8790.mov
kassens pushed a commit to kassens/react that referenced this issue Apr 17, 2023
…#26522)

## Summary
Fixes facebook#24781

Restricting from editing props, which are class instances, because their
internals should be opaque.

Proposed changes:
1. Adding new data type `class_instance`: based on prototype chain of an
object we will check if its plain or not. If not, then will be marked as
`class_instance`. This should not affect `arrays`, ..., because we do
this in the end of an `object` case in `getDataType` function.

Important detail: this approach won't work for objects created with
`Object.create`, because of the custom prototype. This can also be
bypassed by manually deleting a prototype ¯\\\_(ツ)_/¯
I am not sure if there might be a better solution (which will cover all
cases) to detect if object is a class instance. Initially I was trying
to use `Object.getPrototypeOf(object) === Object.prototype`, but this
won't work for cases when we are dealing with `iframe`.


2. Objects with a type `class_instance` will be marked as unserializable
and read-only.

## Demo
`person` is a class instance, `object` is a plain object


https://user-images.githubusercontent.com/28902667/228914791-ebdc8ab0-eb5c-426d-8163-66d56b5e8790.mov
EdisonVan pushed a commit to EdisonVan/react that referenced this issue Apr 15, 2024
…#26522)

## Summary
Fixes facebook#24781

Restricting from editing props, which are class instances, because their
internals should be opaque.

Proposed changes:
1. Adding new data type `class_instance`: based on prototype chain of an
object we will check if its plain or not. If not, then will be marked as
`class_instance`. This should not affect `arrays`, ..., because we do
this in the end of an `object` case in `getDataType` function.

Important detail: this approach won't work for objects created with
`Object.create`, because of the custom prototype. This can also be
bypassed by manually deleting a prototype ¯\\\_(ツ)_/¯
I am not sure if there might be a better solution (which will cover all
cases) to detect if object is a class instance. Initially I was trying
to use `Object.getPrototypeOf(object) === Object.prototype`, but this
won't work for cases when we are dealing with `iframe`.


2. Objects with a type `class_instance` will be marked as unserializable
and read-only.

## Demo
`person` is a class instance, `object` is a plain object


https://user-images.githubusercontent.com/28902667/228914791-ebdc8ab0-eb5c-426d-8163-66d56b5e8790.mov
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants