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

ShallowWrapper::dive fails when diving into a memoized component inside a context provider #2290

Closed
2 of 5 tasks
moward opened this issue Nov 22, 2019 · 8 comments · Fixed by #2296
Closed
2 of 5 tasks

Comments

@moward
Copy link

moward commented Nov 22, 2019

Current behavior

Calling

const SampleContextA = React.createContext<T>(true);

const MyComponent = () => <div />;
const MyComponentMemoized = React.memo(MyComponent);

enzyme
  .shallow(
    <SampleContextA.Provider value={false}>
      <MyComponentMemoized />
    </SampleContextA.Provider>
  )
  .dive();

throws with TypeError: ShallowWrapper::dive() can only be called on components. If you replace the provider wrapper with just a plain wrapper, e.g.

const Wrapper = (props) => React.Children.only(props.children);

it renders fine. It also works fine in you use MyComponent unmemoized.

Expected behavior

This code should return a ShallowWrapper to the contents of <MyComponent /> instead of throwing.

Your environment

API

  • shallow
  • mount
  • render

Version

library version
enzyme 3.10.0
react 16.12.0
react-dom 16.12.0
react-test-renderer 16.12.0
adapter (below) 1.15.1

Adapter

  • enzyme-adapter-react-16
  • others ( )

Notes

I believe this is similar to but distinct from #2103. That issue addressed memoized components being wrapped by regular custom components.

@ljharb
Copy link
Member

ljharb commented Nov 22, 2019

This is because a shallow wrapper "is" what the component you pass to it renders - in other words, check out:

const wrapper = shallow(
    <SampleContextA.Provider value={false}>
      <MyComponentMemoized />
    </SampleContextA.Provider>
  );
console.log(wrapper.debug());

and you'll see that the wrapper is already a MyComponentMemoized. Calling .dive() on it is trying to wrap the div.

@moward
Copy link
Author

moward commented Nov 22, 2019

So why don't I get back a wrapper on the <div/> then? If I run:

const wrapper = enzyme.shallow(
  <SampleContextA.Provider value={false}>
    <MyComponentMemoized />
  </SampleContextA.Provider>
);
console.log(wrapper.debug());
try {
  console.log(wrapper.dive().debug());
} catch (err) {
  console.error(err.message);
}

const wrapper2 = enzyme.shallow(
  <SampleContextA.Provider value={false}>
    <MyComponent />
  </SampleContextA.Provider>
);
console.log(wrapper2.debug());
try {
  console.log(wrapper2.dive().debug());
} catch (err) {
  console.error(err.message);
}

I get back:

<Memo(MyComponent) />
ShallowWrapper::dive() can only be called on components
<MyComponent />
<div />

So why does calling wrapper1.dive() give me an error instead of a <div/>? Should I be calling something besides dive` if a component is memoized?

@ljharb
Copy link
Member

ljharb commented Nov 22, 2019

Specifically because dive is conceptually only for custom components. You can .childAt(0).shallow() if you know there's a single child div and want a wrapper around it, but i'm not sure why you'd need one :-)

@moward
Copy link
Author

moward commented Nov 25, 2019

Ah, I see. And thanks for bearing with me here. I think I can render wrapped component with wrapper.shallow() instead of wrapper.dive(), but it looks like the context is lost when I do this. E.g. dive has special handling to set PROVIDER_VALUES on childOptions but calling shallow clears the child context. Is there a way around that?

For context, I'm trying to rewrite some test rendering code so I can set the context (like the Theme) using Provider components and then shallowly render the component under test for snapshots. I have it working generally except for this snag when the component under test is memoized.

@ljharb
Copy link
Member

ljharb commented Nov 25, 2019

ah, i'd say that's a bug in shallow that should be fixed :-) i think in the meantime you can wrapper.shallow({ context: wrapper.context() }) tho?

(Separately, I'd strongly discourage any kind of snapshot testing; it's very brittle and is only useful when making no-semantic-change refactors, which happens rarely)

@moward
Copy link
Author

moward commented Nov 25, 2019

I'm getting ShallowWrapper::context() can only be called on wrapped nodes that have a non-null instance when I try that which makes sense because the wrapper is on Memo which is a functional component. And even when I try to inject the correct context, it's not received from the consumer. If I understand the logic inside ReactSixteenAdapter::wrapFunctionalComponent, it looks like legacy context (which can be provided via argument) and providerValues are tracked separately.

And thanks for the guidance. I'm already skeptical of the value of snapshot tests, but we have over 600 in our repo so we can't do without them, at least for the time being.

@ljharb
Copy link
Member

ljharb commented Nov 25, 2019

The workaround would be to call .shallow but replicate the logic used in .dive here: https://github.com/airbnb/enzyme/blob/master/packages/enzyme/src/ShallowWrapper.js#L1686 but that might be prohibitive from outside.

I'd be happy to review a PR that adds the same affordances to .shallow().

@moward
Copy link
Author

moward commented Nov 26, 2019

The symbol-keyed properties make that pretty convoluted 😂. I’ll work on a fix. Thanks again!

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

Successfully merging a pull request may close this issue.

2 participants