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

Ignoring attachTo and hydrateIn options set in enzyme.configure #1836

Closed
ah-adarlow opened this issue Sep 24, 2018 · 3 comments
Closed

Ignoring attachTo and hydrateIn options set in enzyme.configure #1836

ah-adarlow opened this issue Sep 24, 2018 · 3 comments

Comments

@ah-adarlow
Copy link

Describe the bug
attachTo and hydrateIn options set in enzyme.configure are ignored.

To Reproduce
In a mocha test suite, add the following code in a before section:

  const node = document.createElement('div');
  node.className = 'my-class';

  enzyme.configure({ attachTo: node });

Then in a test, mount any node without options and test whether its parent in the DOM has the class my-class. It doesn't.

Expected behavior
If attachTo is set in configure and not set in the options when calling mount the option from configure should be used.

Additional context
The issue originated in #1707 and is due to how mountTargets gets merged with the configuration. Even if a mount target is undefined it still overrides the configuration.

I can submit a PR to fix this at some point, but I'm not sure when I'll get around to it. I'm happy for someone else to pick it up earlier if they want to.

@ljharb
Copy link
Member

ljharb commented Sep 25, 2018

Hmm - are you saying that attachTo (or hydrateIn; i'm assuming that either one works, or doesn't, the same) works in an individual mount call, but not via configure?

I see what you mean about the undefined overriding, but https://github.com/airbnb/enzyme/pull/1707/files#diff-0e789a251c4bab4ce68e24abdcd87466R35 should ensure that hydrateIn takes precedence, and if not, attachTo does. A failing test case would be most helpful.

(as for testing whether the parent has "my-class", i'm not confident that react leaves such attributes intact. what if you create a different kind of element, like main, and see if the parent in the DOM is of that type?)

@ah-adarlow
Copy link
Author

ah-adarlow commented Sep 25, 2018

Hmm - are you saying that attachTo (or hydrateIn; i'm assuming that either one works, or doesn't, the same) works in an individual mount call, but not via configure?

That's correct.

should ensure that hydrateIn takes precedence, and if not, attachTo does

The defaulting only happens among options passed directly into the mount call. Anything set in configuration is only taken into account with the spread operators later. Instead, the combination of hydrateIn and attachTo should happen after the combination of configure options and mount options.

A failing test case would be most helpful.

What's the best way to provide one beyond the description I provided? I would use codesandbox or something similar for a UI issue, but I'm not sure how to best write and share a sample test suite.

as for testing whether the parent has "my-class", i'm not confident that react leaves such attributes intact

React doesn't touch this part of the DOM because it's above the root of the rendering. We need it for testing CSS and it worked with enzyme 3.3.0 and still works when passing the attachTo option directly to mount. That's what I'm currently doing as a workaround.

@ljharb
Copy link
Member

ljharb commented Sep 25, 2018

Thanks, that clears it up. I’ll see what i can do.

@ljharb ljharb closed this as completed in e29321c Oct 4, 2018
ljharb added a commit that referenced this issue Oct 5, 2018
 - [new] `mount`: `.state()`/`.setState()`: allow calling on children (#1802)
 - [new] `configuration`: add `reset`
 - [fix] `makeOptions`: ensure that config-level `attachTo`/`hydrateIn` are inherited into wrapper options (#1836)
 - [fix] `shallow`/`Utils`: call into adapter’s `isCustomComponentElement` if present (#1832)
 - [fix] `shallow`/`mount`: throw an explicit error when state is null/undefined
 - [fix] freeze ROOT_NODES for child wrappers (#1811)
 - [fix] `shallow`: `.parents`: ensure that one `.find` call does not affect another (#1781)
 - [fix] `mount`: update after `simulateError` (#1812)
 - [refactor] `mount`/`shallow`: `getElement`: use `this.single`
 - [deps] update `babel-preset-airbnb`, `chai`, `eslint`, `mocha`, `enzyme-adapter-utils`, `react-is`, `airbnb-js-shims`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants