add enzyme-adapter-react-17 (no TODO tags)#2534
add enzyme-adapter-react-17 (no TODO tags)#2534createthis wants to merge 18 commits intoenzymejs:masterfrom
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2534 +/- ##
==========================================
- Coverage 96.31% 95.83% -0.49%
==========================================
Files 49 53 +4
Lines 4207 4749 +542
Branches 1130 1304 +174
==========================================
+ Hits 4052 4551 +499
- Misses 155 198 +43
Continue to review full report at Codecov.
|
| expect(info).to.deep.equal(expectedInfo); | ||
| if (is('>= 17')) { | ||
| expect(info).to.have.property('componentStack'); | ||
| expect(info.componentStack).to.match(/at Thrower (.+)\n/); |
There was a problem hiding this comment.
This change is necessary because the backtrace now includes local file paths in 17.x.
UNSAFE_componentWillReceiveProps() on setContext()
lifecycle methods disabled according to test.
| instance.componentWillReceiveProps(props); | ||
| } | ||
| if (typeof instance.UNSAFE_componentWillReceiveProps === 'function') { // eslint-disable-line new-cap | ||
| instance.UNSAFE_componentWillReceiveProps(props); // eslint-disable-line new-cap |
There was a problem hiding this comment.
This code corrects shallow()'s calling of componentWillReceiveProps() and UNSAFE_componentWillReceiveProps() under react 17.
React.forwardRef under 17.
didn't come up with a workaround.
original PR, plus add an act() wrapper on shallow's simulate(), just to show it doesn't help.
| it('works without memoizing', () => { | ||
| const wrapper = shallow(<RendersApp />); | ||
| expect(wrapper.debug()).to.equal('<App />'); | ||
| expect(wrapper.debug()).to.equal(is('>= 17') ? '<AppMemoized />' : '<App />'); |
There was a problem hiding this comment.
This appears to be due to a change in how React.memo() operates in React 17. Prior to 17 it seemed to return a copy of the component. In 17 it seems to return a reference.
| const MemoForwardFoo = React.memo(React.forwardRef(Foo)); | ||
| expect(adapter.displayNameOfNode(<MemoForwardFoo />)).to.equal('Memo(ForwardRef(CustomWrapper))'); | ||
| if (is('>= 17')) { | ||
| expect(adapter.displayNameOfNode(<MemoForwardFoo />)).to.equal('Memo([object Object])'); |
| return isMemo(type) ? type.type : type; | ||
| } | ||
|
|
||
| function transformSuspense(renderedEl, prerenderEl, { suspenseFallback }) { |
There was a problem hiding this comment.
If I remove this method and all associated code, it goes from 5 suspense related test failures to 16 suspense related test failures. This code is clearly doing something useful.
| expect(wrapper.debug()).to.equal(`<SuspenseComponent> | ||
| <Suspense fallback={{...}}> | ||
| <div /> | ||
| ${is('>= 17') ? '<Suspense mode="visible" />' : `<div /> |
There was a problem hiding this comment.
Regrettably, I don't know why <Suspense mode="visible" /> is appearing instead of the expected html. The <Suspense /> subsystem in the 17 adapter is a bit beyond the limits of my domain knowledge so I am a little lost. I did my best to restore these suspense tests as completely as I could given my limitations. The <Suspense /> subsystem is listed as experimental by React, so I'm not sure how important it is: https://reactjs.org/docs/concurrent-mode-suspense.html
|
|
||
| In another terminal tab execute a specific test file for faster TDD test execution: | ||
| ```bash | ||
| node_modules/.bin/mocha packages/enzyme-test-suite/build/ReactWrapper-spec.js |
There was a problem hiding this comment.
Incorporated tips provided by https://github.com/ljharb back into documentation.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Hi guys, is this the correct place to ask what support my team and I can give to get a React 17 adaptor over the line? We've considered React-Testing-Library but the overhead of converting tests is so much that it might make better sense for us to provide support here. |
|
@shiraze yes, if you can provide people and time, i'd be happy to set up a call to get everyone on the same page so we can get things landed. |
|
@ljharb excellent. Let's set up a call |
|
@shiraze please reach out on twitter DMs or at gmail |
|
Any update on this? |
|
Unfortunately I wasn't able to provide people and time |
43eb75e to
39e6b1f
Compare

Same as #2430 with TODO_17 tags removed.
How working on this PR made me feel:
