-
-
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
[Fix] shallow
: .parents
: ensure that one .find
call does not affect another
#1781
Conversation
…fect another. Fixes #1780.
6844716
to
c294dc0
Compare
expect(bChildParents).to.have.lengthOf(1); | ||
*/ | ||
|
||
const aChildParents = aChild.parents('.b'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ljharb I believe this should this be const aChildParents = aChild.parents('.a');
? (which fixes this test)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks, i'll fix that
c294dc0
to
aa74929
Compare
It looks like this issue is coming from the changes in #1499, the state is changing when The test can be reduced to it.only('works when calling parents after getNodesInternal', () => {
class Test extends React.Component {
render() {
return (
<div>
<div className="a">
<div>A child</div>
</div>
</div>
);
}
}
const wrapper = shallow(<Test />);
const aChild = wrapper.find({ children: 'A child' });
expect(aChild.debug()).to.equal(`<div>
A child
</div>`);
expect(aChild).to.have.lengthOf(1);
// Fails when uncommented
// wrapper.update();
const aChildParents = aChild.parents('.a');
expect(aChildParents.debug(`<div className="a">
<div>A child</div>
</div>`));
expect(aChildParents).to.have.lengthOf(1);
}); |
@ljharb when To illustrate class Test extends React.Component {
render() {
return (
<div>
<div className="a">
<div>A child</div>
</div>
</div>
);
}
}
const wrapper = shallow(<Test />);
const aChild = wrapper.find({ children: 'A child' });
const oldNode = aChild[NODES];
const oldRoot = aChild.root()[NODES];
wrapper.update();
const newNode = aChild[NODES];
const newRoot = aChild.root()[NODES];
console.log(oldNode === newNode); // true
console.log(oldRoot === newRoot); // false Im not really sure what the correct fix is here. Im also not sure why we are updating on all Im also not sure whether this affects anything else besides the |
aa74929
to
d31b829
Compare
I think you're exactly right, and it's the reason I had to do the weird hierarchy stuff for simulateError's componentStack. I think that |
@ljharb 👍 I will try to make that change |
d31b829
to
e792c97
Compare
e792c97
to
4dd2780
Compare
[Fix] Freeze ROOT_NODES for child wrappers
- [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`
Fixes #1780.
These tests are failing; anyone who can help by posting a link to their branch would be very appreciated.