-
-
Notifications
You must be signed in to change notification settings - Fork 2k
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
New memo
feature from React 16.6 is not supported
#1875
Comments
Yep, it’s only been 2 days, so we don’t quite support 16.6 just yet :-) |
Meanwhile, a named export of the component for the tests did the trick for me: export { MyComponent }
export default React.memo(MyComponent) But the snapshots of the components using - Snapshot
+ Received
- <MyComponent someProp="hello there" />
+ <[object Object] someProp="hello there" /> |
Another alternative is a wrapper around the enzyme methods. We already use this, so I just added this, and it seems to work: const unwrapMemo = (shallow: typeof Enzyme.shallow): typeof Enzyme.shallow => (
node: React.ReactElement<any>,
options?: Enzyme.ShallowRendererProps,
) => {
if (typeof node.type === 'object' && (node.type as any).$$typeof === Symbol.for('react.memo')) {
node = Object.create(node, {
type: {
configurable: true,
enumerable: true,
value: (node.type as any).type,
},
});
}
return shallow(node, options);
}; |
I'd strongly suggest not doing anything that requires exporting an unwrapped component. @Alxandr that looks suspiciously close to the actual code we'd need to support it in enzyme. Instead of relying on a wrapper, would you be willing to make a PR that directly adds the support to enzyme? 😄 🙏 |
@ljharb I might look at it, though tbh we're considering moving away from enzyme at work so I doubt I'll get any allocated time for it there 😛. Also, the code didn't work for our cases, cause it works when you render simple components, it doesn't deal with the fact that |
I would be happy to take it if @Alxandr can't, I might need some guidance though as I'm not really familiar with the enzyme codebase besides just using it. I'm working on improving performance for an app and memoizing some components will help. |
@pnavarrc that would be great. I already have a TODO list of OSS stuff I need to fix and I'd like to clear out some items before I add more 🙃 |
I will start by forking the repo and reading the Contributing Guide and forking the repo to get familiar with the environment. |
@pnavarrc is any progress with PR? |
Hi, sorry for the delay! I have been getting more familiar with the enzyme codebase, but I think I will need some guidance on what's needed to implement this (tests that will need to be added, maybe an overview of how adapters work, etc), maybe a more experienced contributor can get in touch with me? (my email is on my GitHub profile). I can still give it a shot, but it would be ideal to get some help to have this ready sooner. If someone else has time and more experience and can get this done faster, I will be happy to help testing or updating the docs. |
@pnavarrc I'm traveling through Thanksgiving (so i might be slow to respond), but feel free to ping me anytime in Slack, freenode IRC, or gitter, and I'll help walk you through it. |
I created a PR #1914 based out of the above discussion. Please let me know if this is something we can go ahead with. |
I get the error when shallow is called with the memoized component, but if I wrap it in a div and then call shallow on it, it seems to work. |
Temporal workaround with jest... :/ jest.mock('react', () => {
const r = jest.requireActual('react');
return { ...r, memo: (x) => x };
}); cc. @asdelday |
@drpicox Thanks, this works for now. |
Just to reiterate over @drpicox 's life saving answer. One could also have const react = require("react");
module.exports = { ...react, memo: x => x }; |
This comment has been minimized.
This comment has been minimized.
It's also not supported in @wintercounter Could you include in the original issue description that the |
Done. |
Another temporary workaround: implement memo yourself. const memo = Component => class extends React.PureComponent {
render() {
return <Component {...this.props} />
}
} It lacks propsAreEqual but it still works. |
One step further, to merge @drpicox, @icyJoseph and @dkamyshov workarounds: In
Note support for the second param of |
Can confirm that the |
@ljharb i am sorry to bother you, but memo is still not supported by enzyme@^3.11.0. i still get
when doing Any idea, what's wrong? |
@macrozone make sure you're using the exact same minor version of react, react-dom, and react-test-renderer, as well as the latest enzyme and react adapter. If that doesn't work, please file a new issue. |
Current behavior
Shallow render will fail with error:
Expected behavior
Render without error.
Your environment
No need, happenes everywhere. I new adaper version will be needed I guess.
API
Version
Adapter
The text was updated successfully, but these errors were encountered: