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

New memo feature from React 16.6 is not supported #1875

Closed
3 of 13 tasks
wintercounter opened this issue Oct 25, 2018 · 24 comments · Fixed by #1914
Closed
3 of 13 tasks

New memo feature from React 16.6 is not supported #1875

wintercounter opened this issue Oct 25, 2018 · 24 comments · Fixed by #1914
Projects

Comments

@wintercounter
Copy link

wintercounter commented Oct 25, 2018

Current behavior

Shallow render will fail with error:

Invariant Violation: ReactShallowRenderer render(): Shallow rendering works only with custom components, but the provided element type was 'object'

Expected behavior

Render without error.

Your environment

No need, happenes everywhere. I new adaper version will be needed I guess.

API

  • shallow
  • mount
  • render

Version

library version
enzyme 3.7.0
react 16.6.0

Adapter

  • enzyme-adapter-react-16
  • enzyme-adapter-react-16.3
  • enzyme-adapter-react-16.2
  • enzyme-adapter-react-16.1
  • enzyme-adapter-react-15
  • enzyme-adapter-react-15.4
  • enzyme-adapter-react-14
  • enzyme-adapter-react-13
  • enzyme-adapter-react-helper
  • others ( )
@ljharb
Copy link
Member

ljharb commented Oct 25, 2018

Yep, it’s only been 2 days, so we don’t quite support 16.6 just yet :-)

@simonrelet
Copy link

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 MyComponent need to be updated:

- Snapshot
+ Received

- <MyComponent someProp="hello there" />
+ <[object Object] someProp="hello there" />

@Alxandr
Copy link

Alxandr commented Oct 31, 2018

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);
};

@ljharb
Copy link
Member

ljharb commented Nov 2, 2018

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? 😄 🙏

@Alxandr
Copy link

Alxandr commented Nov 2, 2018

@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 children can also contain memos (which shouldn't be too hard to handle though).

@pnavarrc
Copy link

pnavarrc commented Nov 2, 2018

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.

@Alxandr
Copy link

Alxandr commented Nov 2, 2018

@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 🙃

@pnavarrc
Copy link

pnavarrc commented Nov 2, 2018

I will start by forking the repo and reading the Contributing Guide and forking the repo to get familiar with the environment.

@aurimas4
Copy link

@pnavarrc is any progress with PR?

@pnavarrc
Copy link

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.

@ljharb
Copy link
Member

ljharb commented Nov 21, 2018

@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.

@jintoppy
Copy link
Contributor

I created a PR #1914 based out of the above discussion. Please let me know if this is something we can go ahead with.

@jeffwillette
Copy link

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.

@drpicox
Copy link

drpicox commented Jan 3, 2019

Temporal workaround with jest... :/

jest.mock('react', () => {
  const r = jest.requireActual('react');

  return { ...r, memo: (x) => x };
});

cc. @asdelday

@pataar
Copy link

pataar commented Jan 8, 2019

@drpicox Thanks, this works for now.

@icyJoseph
Copy link

Just to reiterate over @drpicox 's life saving answer. One could also have src/__mocks__/react.js, where you can centralize this hack, to easily remove it later on.

const react = require("react");
module.exports = { ...react, memo: x => x };

@ljharb ljharb moved this from Needs Triage to v16.6: memo, lazy, suspense in React 16 Jan 23, 2019
@cassels

This comment has been minimized.

@eps1lon
Copy link
Contributor

eps1lon commented Feb 10, 2019

It's also not supported in mount causing Error: Enzyme Internal Error: unknown node with tag 15.

@wintercounter Could you include in the original issue description that the mount API is affected too.

@wintercounter
Copy link
Author

Done.

@dkamyshov
Copy link

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.

@ljharb ljharb closed this as completed Feb 14, 2019
@ljharb ljharb reopened this Feb 14, 2019
@mrjackdavis
Copy link

One step further, to merge @drpicox, @icyJoseph and @dkamyshov workarounds:

In src/__mocks__/react.js ...

import * as React from "react";

// Shim memo
const memo = (Component: React.ReactType) =>
  class extends React.PureComponent {
    render() {
      return <Component {...this.props} />;
    }
  };

module.exports = { ...React, memo };

Note support for the second param of React.memo is not implemented here

@carloscuesta
Copy link

Can confirm that the mount API it's affected too

@macrozone
Copy link

@ljharb i am sorry to bother you, but memo is still not supported by enzyme@^3.11.0. i still get

  Enzyme Internal Error: unknown node with tag 15

when doing mount(<Component />) where Component is a React.memo() wrapped component.

Any idea, what's wrong?

@ljharb
Copy link
Member

ljharb commented Jan 27, 2020

@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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
React 16
  
v16.6+: memo, lazy, suspense
Development

Successfully merging a pull request may close this issue.