support non plain objects

[barakyosi]

Added non plain object detection:

1. Verify prototypes are equal.
2. Verify all keys are the same.

Wasn't sure why there's a copy of the object before sending it to trackDiff (introduced here).
This broke for non plain objects, so I passed the object without clone in case it's not a plain one. Open for other suggestions here :)

This PR fixes #74

[barakyosi]

I tried to add an integration test with Error prop, but it kept failing because Error.stack is different in different renders.
Any suggestions how to solve it?

test('Non simple objects', () => {
  const Foo = React.memo(function Foo({ error }) {
    return (
      <div>
        <h1>{error.message}</h1>
        <p>{error.stack}</p>
      </div>
    );
  });

  const App = React.memo(function App() {
    const [text, setText] = React.useState('Click me');

    return (
      <div className="App">
        <button
          onClick={() => setText(state => state + '.')}
          data-testid="button"
        >
          {text}
        </button>
        <hr/>
        <Foo error={new Error('message')}/>
      </div>
    );
  });

  const { getByTestId } = rtl.render(
    <App/>
  );

  const button = getByTestId('button');
  rtl.fireEvent.click(button);

  expect(updateInfos[1].reason.propsDifferences[0]).toEqual({
    diffType: 'deepEquals', // <-- Getting `different` here due to error.stack
  });
});

Edit:
I see this check in assert wondering if we want to do the same here. WDYT?

[vzaidman]

Edit:
I see this check in assert wondering if we want to do the same here. WDYT?

Yes!
please add the error comparison to run before the comparison of objects.

[barakyosi]

@vzaidman Updated the PR.

Some thoughts:

1. I duplicated object keys check between isError and the old object comparison. WDYT about extracting it to a new method compareObjectKeys? That might be a complex one as there are multiple arguments needed there.
2. Any idea how to verify whether there are performance implications here? I don't see a direct reason for it, but since it changes the way we compare objects I thought if we need to at least consider it.
3. I couldn't use the cloning mechanism introduced here. Can you share why it was needed, and whether we need it for Errors and non plain objects?

[vzaidman]

let's move the relevantKeys calculation in to the

if (typeof a === 'object' && typeof b === 'object' && Object.getPrototypeOf(a) === Object.getPrototypeOf(b)) {

scope

so we won't repeat the rest of the code.

[vzaidman]

@vzaidman Updated the PR.

Some thoughts:

1. I duplicated object keys check between isError and the old object comparison. WDYT about extracting it to a new method compareObjectKeys? That might be a complex one as there are multiple arguments needed there.
2. Any idea how to verify whether there are performance implications here? I don't see a direct reason for it, but since it changes the way we compare objects I thought if we need to at least consider it.
3. I couldn't use the cloning mechanism introduced here. Can you share why it was needed, and whether we need it for Errors and non plain objects?

1. See my comment on the code. I think we can extract the calculation of the relevant keys into the comparison of objects "if" where it would only differ for Error objects excluding the stack
2. The library is for development anyway. I don't think it affects performance on a significant level.
3. It's only needed for correct reporting in the console. I've seen cases where you report a prop and then the object gets modified. So when inspecting the object in the console, it might be different from what was actually passed to a component.

[vzaidman]

Sorry it took so long to merge

[Hypnosphi]

Now I get following warnings, apparently because WDYR tries to access a prop that shouldn't be accessed:

Warning: span: `key` is not a prop. Trying to access it will result in `undefined` being returned. If you need to access the same value within the child component, you should pass it as a different prop. (https://reactjs.org/link/special-props)

Let's maybe ignore the properties with getters?

[vzaidman]

Now I get following warnings, apparently because WDYR tries to access a prop that shouldn't be accessed:

Warning: span: `key` is not a prop. Trying to access it will result in `undefined` being returned. If you need to access the same value within the child component, you should pass it as a different prop. (https://reactjs.org/link/special-props)

Let's maybe ignore the properties with getters?

In what cases do you see this?
For what types of objects?

[vzaidman]

@Hypnosphi hey! how can i reproduce it?

[Hypnosphi]

Sorry I missed your previous comment. I'll create a reproduction case

[Hypnosphi]

Here's the minimal reproduction: <TrackedComponent><Child key="key"/></TrackedComponent>. Replacing Object. getOwnPropertyNames with Object.keys fixes it

[vzaidman]

fixed in v6.1.1