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

react-is: 16.12.0 isMemo is returning wrong value #17457

Closed
changran52m opened this issue Nov 26, 2019 · 4 comments
Closed

react-is: 16.12.0 isMemo is returning wrong value #17457

changran52m opened this issue Nov 26, 2019 · 4 comments

Comments

@changran52m
Copy link

changran52m commented Nov 26, 2019

Bug in react-is: 16.12.0
https://codesandbox.io/s/determined-lehmann-hl43r
Screen Shot 2019-11-25 at 10 34 22 PM

This issue does not exist in ### 16.11.0
https://codesandbox.io/s/nifty-paper-8cqil

@milesj
Copy link
Contributor

milesj commented Nov 26, 2019

This changed in v16.12: #17278

@changran52m
Copy link
Author

changran52m commented Nov 26, 2019

This changed in v16.12: #17278

@milesj Thanks for your quick reply.
However, I am still able to reproduce it here. Please correct me if I am wrong.
Screen Shot 2019-11-25 at 10 52 47 PM

https://codesandbox.io/s/determined-lehmann-hl43r
Screen Shot 2019-11-25 at 10 34 22 PM

import React, { memo } from "react";

import { isMemo } from "react-is";

const Test = memo(() => {
  return <div>test</div>;
});

console.log("Test", isMemo(Test)); // false

@changran52m
Copy link
Author

changran52m commented Nov 26, 2019

@milesj
I think I get it now. It was a bug and got fixed in 16.12. But seems there are so many packages depending on the buggy function.

I will try some work around in our side and open issue in material UI and enzyme repo then.

Thanks again for your quick reply.

@bvaughn
Copy link
Contributor

bvaughn commented Nov 26, 2019

As the linked PR #17278 mentions:

const MemoizedComponent = React.memo(MyComponent);

// MemoizedComponent is a valid element TYPE.
expect(ReactIs.isValidElementType(MemoizedComponent)).toBe(true);

// <MemoizedComponent /> is a valid element.
expect(ReactIs.typeOf(<MemoizedComponent />)).toBe(ReactIs.Memo);
expect(ReactIs.isMemo(<MemoizedComponent />)).toBe(true);

// MemoizedComponent is NOT an element!
// (This used to return a false positive.)
expect(ReactIs.isMemo(MemoizedComponent)).toBe(false);

The previous behavior was broken. It's unfortunate that the bug fix caused some existing use cases to "break" but I think it was still the right change to make.

Sorry for the inconvenience!

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

No branches or pull requests

3 participants