Adds Mount tracking #1730
Adds Mount tracking #1730
Conversation
|
looks like one of the jobs in my build timed out before starting the tests: https://travis-ci.org/airbnb/enzyme/jobs/411307270 cc @ljharb |
packages/enzyme/src/mountTracking.js
Outdated
| * @param {ReactWrapper|ShallowWrapper} wrapper | ||
| */ | ||
| export function trackMountedWrapper(wrapper) { | ||
| const { enableMountTracking } = get(); |
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
packages/enzyme/src/mountTracking.js
Outdated
| export function trackMountedWrapper(wrapper) { | ||
| const { enableMountTracking } = get(); | ||
| if (enableMountTracking) { | ||
| mountedWrappers.push(wrapper); |
There was a problem hiding this comment.
what if it's already in there? this should probably use a Set
docs/installation/mount-tracking.md
Outdated
|
|
||
| configure({ | ||
| adapter: new Adapter(), | ||
| enableMountTracking: true, |
There was a problem hiding this comment.
it seems kind of weird to call this option "mount tracking" when it tracks both shallow and mount. maybe "enableSandbox", since sinon-sandbox is actually the behavior it's emulating?
packages/enzyme/src/index.js
Outdated
| @@ -15,4 +16,6 @@ module.exports = { | |||
| ReactWrapper, | |||
| configure, | |||
| EnzymeAdapter, | |||
| trackMountedWrapper, | |||
There was a problem hiding this comment.
why does this need to be re-exported?
There was a problem hiding this comment.
for custom implementation of mount that extend the enzyme wrapper. We use this pattern often, for example, to encapsulate logic around application context.
There was a problem hiding this comment.
I don't think it needs to be in the root export tho - consumers can deep-import whatever they need from inside build
upstream changes
|
@ljharb can you have another look when you get a chance? |
|
|
||
| it('should call trackWrapper', () => { | ||
| const spy = sinon.spy(); | ||
| const originalTrackWrapper = wrapperSandbox.trackWrapper; |
There was a problem hiding this comment.
eesh, this is a really brittle test that will definitely break in babel 7 and when node ships native ESM support. is this necessary? can we instead test the actual behavior?
|
|
||
| it('should call trackWrapper', () => { | ||
| const spy = sinon.spy(); | ||
| const originalTrackWrapper = wrapperSandbox.trackWrapper; |
| unmountAllWrappers, | ||
| } from 'enzyme/build/wrapperSandbox'; | ||
|
|
||
| const originalConfig = get(); |
There was a problem hiding this comment.
this should probably be populated before or beforeEach rather than at require time
| it('does what i expect', () => { | ||
| const wrapper = new ReactWrapper(<p>foo</p>); | ||
| const spy = sinon.spy(); | ||
| wrapper.unmount = spy; |
There was a problem hiding this comment.
const spy = sinon.spy(wrapper, 'unmount');
| * until after you have restored those globals will lead to their stored | ||
| * identifiers being invalid. | ||
| */ | ||
| export function unmountAllWrappers() { |
There was a problem hiding this comment.
i kind of feel like if enableSandbox is false, this function should throw
ljharb
force-pushed
the
master
branch
3 times, most recently
from
40cc703
to
0a17404
Compare
ljharb
force-pushed
the
master
branch
4 times, most recently
from
2227326
to
0d5ead7
Compare
ljharb
force-pushed
the
master
branch
3 times, most recently
from
43eb75e
to
39e6b1f
Compare
Closes #1689. Closes #437.