React 16 (Step 2): Enzyme Strikes Back

[tlrobinson]

Next PR: #6758
Parent PR: #6756

NOTE: react-16-step1 should be merged before this (then remember to update the base branch to master). Regularly merge react-16-step1 into this branch.

Upgrades Enzyme to version 3, which is required for React 16. Unfortunately it breaks some unit tests and most integration tests so we should fix those before upgrading to React 16, which may also break tests.

• upgrade Enzyme
• fix unit tests
• fix integration tests

Here are my notes on some issues:

• needs app.update() in some places, in particular after store.waitForActions() in integration tests
• element.find() doesn't include the element itself
  • for checking classes it's better to do element.hasClass("...") etc
  • possibly related: select.find(Popover) (or children of Popover) no longer works, but app.find(Popover) may

[attekei]

TLDR: Updating to Enzyme v3 requires updating around 90% of our integration tests. It's pretty mechanical work but will take a lot of time. Enzyme v3 also introduced a bunch of new bugs so there might be new surprises around the corner which I don't know about yet.

These breaking changes in Enzyme v3 impact us directly:

1. The React DOM tree contained by a mount()ed Enzyme wrapper is immutable.

Only the Enzyme wrapper contains the latest information about the state of DOM tree. If you are used to do stuff like this:

const app = mount(store.getAppContainer())
const toggle = app.find(Toggle)
expect(toggle.prop("isEnabled")).toBe(false)
click(toggle)
expect(toggle.prop("isEnabled")).toBe(true)

...that doesn't work anymore in Enzyme v3 because toggle is part of immutable DOM tree and clicking it doesn't change the existing DOM element or its children. You have to do something like this to ensure that you always fetch the Toggle component in its latest state from app:

const app = mount(store.getAppContainer())
const getToggle = () => app.find(Toggle)
expect(getToggle().prop("isEnabled")).toBe(false)
click(getToggle())
expect(getToggle().prop("isEnabled")).toBe(true)

From our perspective, that's a pretty annoying change, but something we maybe can't avoid if we want to keep using standard Enzyme.

2. In earlier Enzyme versions we were interacting with the real React DOM directly. The new immutable DOM is actually just an intermediate representation of React tree and it knows nothing about what's happening in React components inside it.

This means that we have to occasionally call app.update() to update that intermediate representation to match the actual state of React tree. Some Enzyme methods (like wrapper.simulate(event) will call update method automatically, but some of our own methods (like waitForActions(...) of test store) need to be changed to do the update. As a consequence the test store needs to be aware about Enzyme wrappers and I had to change its API a bit.

3. .parent() returns an incomplete React component (?!) and we need to use .parents().at(0) instead.

I think that this is a regression rather than an intentional breaking change and there are probably other similar bugs too.

---

More about (1) and (2) in https://github.com/airbnb/enzyme/blob/master/docs/guides/migration-from-2-to-3.md#element-referential-identity-is-no-longer-preserved.

As a sidenote, many Enzyme have have recently created GitHub issues because they've confused by (1) ; see for instance enzymejs/enzyme#1400 and enzymejs/enzyme#1153. (3) is reported in enzymejs/enzyme#1373.

[attekei]

FieldApp.integ.spec.js is the only test file that has been migrated so far

[attekei]

I was actually able to fix test failures caused by (1) by monkey patching the find() method of Enzyme wrapper so that when doing consecutive method calls / property lookups (like .prop("enabled") inapp.find(Toggle).prop("enabled")) it actually always runs app.find(Toggle) again, making sure that we always have the latest DOM.

I might later actually rename the method to something like findLazy or findLatest and that way avoid modifying the standard Enzyme methods.

I'll put this branch on hold for a while to get other stuff done, but now it looks like the workload of fixing failing tests isn't very big anymore.

[camsaul]

Closing this and other old POC or WIP PRs for now to help clear the PR backlog. I'll leave the branch open in case we want to revisit this