Repository: facebook/react
Pull request: [devtools] Fix can't expand prop value in some scenario

<!--
  Thanks for submitting a pull request!
  We appreciate you spending the time to work on these changes. Please provide enough information so that others can review your pull request. The three fields below are mandatory.

  Before submitting a pull request, please make sure the following is done:

  1. Fork [the repository](https://github.com/facebook/react) and create your branch from `master`.
  2. Run `yarn` in the repository root.
  3. If you've fixed a bug or added code that should be tested, add tests!
  4. Ensure the test suite passes (`yarn test`). Tip: `yarn test --watch TestName` is helpful in development.
  5. Run `yarn test --prod` to test in the production environment. It supports the same options as `yarn test`.
  6. If you need a debugger, run `yarn debug-test --watch TestName`, open `chrome://inspect`, and press "Inspect".
  7. Format your code with [prettier](https://github.com/prettier/prettier) (`yarn prettier`).
  8. Make sure your code lints (`yarn lint`). Tip: `yarn linc` to only check changed files.
  9. Run the [Flow](https://flowtype.org/) type checks (`yarn flow`).
  10. If you haven't already, complete the CLA.

  Learn more about contributing: https://reactjs.org/docs/how-to-contribute.html
-->

## Summary

**TL;DR**
1. A bug incoming with this PR #19786
2. Devtools extension user can't expand prop value in some scenario
3. Versions affected 4.9.0 / 4.10.0 / 4.10.1

## Bug Record Screenshot
As you can see, the mobxStores value not show correctly!
<div style="display:flex;justify-content:center;width=650px;">
		<p><img src="https://user-images.githubusercontent.com/14012511/103462331-6b28cd00-4d5f-11eb-8fe0-f5297f6cf147.png" width="650px" />
	    </p>
</div>
<div style="display:flex;justify-content:center;width=650px;">
		<p><img src="https://user-images.githubusercontent.com/14012511/103462489-77f9f080-4d60-11eb-872c-387673e72d8d.png" width="650px" />
	    </p>
</div>
<div style="display:flex;justify-content:center;width=650px;">
		<p><img src="https://user-images.githubusercontent.com/14012511/103462503-8fd17480-4d60-11eb-9756-c688274234c7.png" width="650px" />
	    </p>
</div>

>  Don't focus the **legacy context** block, cause the same bug can reappear with context or hooks(useContext)

## Code Analysis
The code from pr #19786 by omarsy, [#L71-L97](https://github.com/facebook/react/blob/master/packages/react-devtools-shared/src/utils.js#L71-L97)
```js
export function getAllEnumerableKeys(
  obj: Object,
): Array<string | number | Symbol> {
    /*** omit ***/
    current = Object.getPrototypeOf(current);
   /*** omit ***/
}
```
[Mobx](https://github.com/mobxjs/mobx)(with React) user offen define data store this way:
```js
class ReactDataStore {
    @observable appStatus;

    @action.bonud
    changeAppStatus (status) {
        this.appStatus = status;
    }
}
```
Will transpile to: 
```js
/**** omit polyfill code ****/
class ReactDataStore {
    changeAppStatus(init) {
        this.appInit = init;
    }
}
__decorate([
    observable,
    __metadata("design:type", Object)
], ReactDataStore.prototype, "appInit", void 0);

__decorate([
    action.bonud,
    __metadata("design:type", Function),
    __metadata("design:paramtypes", [Object]),
    __metadata("design:returntype", void 0)
], ReactDataStore.prototype, "changeAppStatus", null);

```
So when the react devtools start [dehydrate](https://github.com/facebook/react/blob/master/packages/react-devtools-shared/src/hydration.js#L102-L332) the react app instance data, the **cleaned** will create duplicate path cause the **getAllEnumerableKeys** function.
<div style="display:flex;justify-content:center;width=650px;">
		<p><img src="https://user-images.githubusercontent.com/14012511/103463234-f6a55c80-4d65-11eb-8677-254a233c9367.png" width="650px" />
	    </p>
</div>

I set a *debugger* for react devtools panel page to find the reason of the bug.
### So what will happen when we create duplicate **cleaned** path? (💀  if the prop value is **undefined** )
```js
export function hydrate(
  object: any,
  cleaned: Array<Array<string | number>>,
  unserializable: Array<Array<string | number>>,
): Object {
  cleaned.forEach((path: Array<string | number>) => {
    const length = path.length;
    const last = path[length - 1];
    const parent = getInObject(object, path.slice(0, length - 1));
    if (!parent || !parent.hasOwnProperty(last)) {
      return;
    }
    const value = parent[last];

   if (value.type === 'infinity') {
      parent[last] = Infinity;
    } else if (value.type === 'nan') {
      parent[last] = NaN;
    } else if (value.type === 'undefined') {
      parent[last] = undefined;
    } else {
/*** omit  ***/
```
When we meet duplicate path firstly , the **parent[last]** is **{ type: undefined }**([data process logic](https://github.com/facebook/react/blob/master/packages/react-devtools-shared/src/backend/utils.js#L15-L39)), then **parent[last]** will be reassign as **undefined**。
```js
    } else if (value.type === 'undefined') {
      parent[last] = undefined;
    } else {
```
Everything is ok until we meet duplicate path secondly, this time our **parent[last]** is **undefined** not **{ type: undefined }**, cause the path is same. so error occured !
```js
Error in event handler: TypeError: Cannot read property 'type' of undefined
```
## Expected Expand Result
> After I fix the TypeError
<div style="display:flex;justify-content:center;width=650px;">
		<p><img src="https://user-images.githubusercontent.com/14012511/103464002-987b7800-4d6b-11eb-9ec9-9f8888822273.png" width="650px" />
	    </p>
</div>

<!-- Demonstrate the code is solid. Example: The exact commands you ran and their output, screenshots / videos if the pull request changes the user interface. -->

## Code Review 

<del>The PR #19786 is for *React DevTools: Show symbols used as keys in state*, but I change some code logic, so I hope @omarsy can review my PR,</del> of course @bvaughn is also important reviewer.
💔  Change the logic will broken devtools CI [detail](https://app.circleci.com/pipelines/github/facebook/react/8611/workflows/ac137a52-cab8-48a3-96e1-ded0ca7b7bf4/jobs/249856), so just skip **parent[last]** when the value is not a valid object.
```js
// previous commit to avoid duplicate cleaned path
// packages/react-devtools-shared/src/utils.js  -- getAllEnumerableKeys
- return keys;
+ return [...new Set(keys)];
```

Anyway, Thanks for bvaughn's great work for React Devtools. 👍 

## Test Plan

Necessary unit test will add in the this PR later,  if my pr is valuable.


Changed files:
filename: packages/react-devtools-shared/src/__tests__/inspectedElement-test.js
patch: @@ -1168,6 +1168,83 @@ describe('InspectedElement', () => {
     done();
   });
 
+  it('should allow component prop value and value`s prototype has same name params.', async done => {
+    const testData = Object.create(
+      {
+        a: undefined,
+        b: Infinity,
+        c: NaN,
+        d: 'normal',
+      },
+      {
+        a: {
+          value: undefined,
+          writable: true,
+          enumerable: true,
+          configurable: true,
+        },
+        b: {
+          value: Infinity,
+          writable: true,
+          enumerable: true,
+          configurable: true,
+        },
+        c: {
+          value: NaN,
+          writable: true,
+          enumerable: true,
+          configurable: true,
+        },
+        d: {
+          value: 'normal',
+          writable: true,
+          enumerable: true,
+          configurable: true,
+        },
+      },
+    );
+    const Example = ({data}) => null;
+    const container = document.createElement('div');
+    await utils.actAsync(() =>
+      ReactDOM.render(<Example data={testData} />, container),
+    );
+
+    const id = ((store.getElementIDAtIndex(0): any): number);
+
+    let inspectedElement = null;
+
+    function Suspender({target}) {
+      inspectedElement = useInspectedElement(target);
+      return null;
+    }
+
+    await utils.actAsync(
+      () =>
+        TestRenderer.create(
+          <Contexts
+            defaultSelectedElementID={id}
+            defaultSelectedElementIndex={0}>
+            <React.Suspense fallback={null}>
+              <Suspender target={id} />
+            </React.Suspense>
+          </Contexts>,
+        ),
+      false,
+    );
+    expect(inspectedElement.props).toMatchInlineSnapshot(`
+      Object {
+        "data": Object {
+          "a": undefined,
+          "b": Infinity,
+          "c": NaN,
+          "d": "normal",
+        },
+      }
+    `);
+
+    done();
+  });
+
   it('should not dehydrate nested values until explicitly requested', async done => {
     const Example = () => {
       const [state] = React.useState({

filename: packages/react-devtools-shared/src/__tests__/legacy/inspectElement-test.js
patch: @@ -588,6 +588,66 @@ describe('InspectedElementContext', () => {
     done();
   });
 
+  it('should allow component prop value and value`s prototype has same name params.', async done => {
+    const testData = Object.create(
+      {
+        a: undefined,
+        b: Infinity,
+        c: NaN,
+        d: 'normal',
+      },
+      {
+        a: {
+          value: undefined,
+          writable: true,
+          enumerable: true,
+          configurable: true,
+        },
+        b: {
+          value: Infinity,
+          writable: true,
+          enumerable: true,
+          configurable: true,
+        },
+        c: {
+          value: NaN,
+          writable: true,
+          enumerable: true,
+          configurable: true,
+        },
+        d: {
+          value: 'normal',
+          writable: true,
+          enumerable: true,
+          configurable: true,
+        },
+      },
+    );
+
+    const Example = ({data}) => null;
+    act(() =>
+      ReactDOM.render(
+        <Example data={testData} />,
+        document.createElement('div'),
+      ),
+    );
+
+    const id = ((store.getElementIDAtIndex(0): any): number);
+    const inspectedElement = await read(id);
+
+    expect(inspectedElement.props).toMatchInlineSnapshot(`
+      Object {
+        "data": Object {
+          "a": undefined,
+          "b": Infinity,
+          "c": NaN,
+          "d": "normal",
+        },
+      }
+    `);
+    done();
+  });
+
   it('should not dehydrate nested values until explicitly requested', async done => {
     const Example = () => null;
 

filename: packages/react-devtools-shared/src/hydration.js
patch: @@ -383,7 +383,9 @@ export function hydrate(
 
     const value = parent[last];
 
-    if (value.type === 'infinity') {
+    if (!value) {
+      return;
+    } else if (value.type === 'infinity') {
       parent[last] = Infinity;
     } else if (value.type === 'nan') {
       parent[last] = NaN;

filename: packages/react-devtools-shared/src/utils.js
patch: @@ -70,8 +70,8 @@ export function alphaSortKeys(
 
 export function getAllEnumerableKeys(
   obj: Object,
-): Array<string | number | Symbol> {
-  const keys = [];
+): Set<string | number | Symbol> {
+  const keys = new Set();
   let current = obj;
   while (current != null) {
     const currentKeys = [
@@ -82,7 +82,7 @@ export function getAllEnumerableKeys(
     currentKeys.forEach(key => {
       // $FlowFixMe: key can be a Symbol https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/getOwnPropertyDescriptor
       if (descriptors[key].enumerable) {
-        keys.push(key);
+        keys.add(key);
       }
     });
     current = Object.getPrototypeOf(current);
@@ -767,7 +767,7 @@ export function formatDataForPreview(
       return data.toString();
     case 'object':
       if (showFormattedValue) {
-        const keys = getAllEnumerableKeys(data).sort(alphaSortKeys);
+        const keys = Array.from(getAllEnumerableKeys(data)).sort(alphaSortKeys);
 
         let formatted = '';
         for (let i = 0; i < keys.length; i++) {