Update to Jest 22 #11956
Update to Jest 22 #11956
Conversation
This relies on our existing special field that we use to mute errors. Perhaps, it would be better to instead rely on preventDefault() directly. I outlined a possible strategy here: facebook#11098 (comment)
| "jest": "^21.3.0-beta.4", | ||
| "jest-config": "^21.3.0-beta.4", | ||
| "jest-jasmine2": "^21.3.0-beta.4", | ||
| "jest-runtime": "^21.3.0-beta.4", |
There was a problem hiding this comment.
We weren't using these anymore
There was a problem hiding this comment.
expect is still in here in beta, is that used?
|
that SVG element support tho 👌 |
| window.addEventListener('error', event => { | ||
| if (event.error != null && event.error.suppressReactErrorLogging) { | ||
| event.preventDefault(); | ||
| } |
There was a problem hiding this comment.
might be worth ensuring these are logged somewhere. I noticed on RDL that errors thrown in event handlers where being swallowed by jsdom under mysterious circumstances...I guess there is already an issue for it tho: #8260
There was a problem hiding this comment.
Yeah, thanks for highlighting that. They are logged now (by jsdom) but we should probably fail the build on them (?)
This issue (and related PR) is actually why I got curious about updating jsdom :P
The test passed in jsdom due to its implementation details. The original intention was to test the mutation method, but it was removed a while ago. Following @nhunzaker's suggestion, I moved the tests to ReactDOMInput and adjusted them to not rely on implementation details.
|
The remaining failing tests are in the DOM server integration suite and have to do with the fact that now we have a few cases where some warnings only fire on the client side (previously we didn't "see" them because of jsdom not faithfully emulating SVG or custom elements). In some cases we can't really fix the discrepancy. For example "render HTML only and then use innerHTML" tests will never emit React warnings about unknown DOM element type because those only happen in the client-side code. So maybe we should explicitly check expected warnings somehow, or pass different numbers for different cases. |
This is a bit ugly but it's just two places. I think we can live with this.
We used to warn both about bad casing and about a mismatch. The mismatch warning was a bit confusing. We didn't know we warned twice because jsdom didn't faithfully emulate SVG. This changes the behavior to only leave the warning about bad casing if that's what caused the mismatch. It also adjusts the test to have an expectation that matches the real world behavior.
|
Failing the build on uncaught errors in event handlers (#8260) is more challenging and I won't do it here. (We didn't do it before either.) We know how to do it now though (now that we own the error handler), and I think that should be fixed as part of #11098 (comment). |
| const containerTag = tags.shift(); | ||
| const container = | ||
| containerTag === 'svg' | ||
| ? document.createElementNS('http://www.w3.org/2000/svg', containerTag) |
| // so that it gets deduplicated later, and doesn't fail the test. | ||
| expect(() => { | ||
| ReactDOM.render(<nonstandard />, document.createElement('div')); | ||
| }).toWarnDev('The tag <nonstandard> is unrecognized in this browser.'); |
| // So we'll skip the misleading extra mismatch warning in this case. | ||
| isMismatchDueToBadCasing = true; | ||
| // $FlowFixMe - Should be inferred as not undefined. | ||
| extraAttributeNames.delete(standardName); |
|
Yeah, forgot to update that part. Individual commits show how I fixed them. |
* Bump deps to Jest 22 * Prevent jsdom from logging intentionally thrown errors This relies on our existing special field that we use to mute errors. Perhaps, it would be better to instead rely on preventDefault() directly. I outlined a possible strategy here: facebook#11098 (comment) * Update snapshots * Mock out a method called by ReactART that now throws * Calling .click() no longer works, dispatch event instead * Fix incorrect SVG element creation in test * Render SVG elements inside <svg> to avoid extra warnings * Fix range input test to use numeric value * Fix creating SVG element in test * Replace brittle test that relied on jsdom behavior The test passed in jsdom due to its implementation details. The original intention was to test the mutation method, but it was removed a while ago. Following @nhunzaker's suggestion, I moved the tests to ReactDOMInput and adjusted them to not rely on implementation details. * Add a workaround for the expected extra client-side warning This is a bit ugly but it's just two places. I think we can live with this. * Only warn once for mismatches caused by bad attribute casing We used to warn both about bad casing and about a mismatch. The mismatch warning was a bit confusing. We didn't know we warned twice because jsdom didn't faithfully emulate SVG. This changes the behavior to only leave the warning about bad casing if that's what caused the mismatch. It also adjusts the test to have an expectation that matches the real world behavior. * Add an expected warning per comment in the same test
jsdom update made this one challenging.
It uncovered some issues in tests, but I believe some of the remaining 9 tests I haven't fixed yet actually point out legit problems (where we emit a different number of warnings on the server and on the client).I made one change to the behavior. Previously we were emitting two different warnings (about bad prop casing and about client/server mismatch) when hydrating a badly cased SVG attribute like
<text textlength="10" />. In ed93325, I changed it to only emit one warning (about the prop casing) since that's the actionable one. This fixed the tests (which assume the number of warnings is consistent between server and client).