refactor: migrate to Preact X

[francoischalifour]

This updates InstantSearch.js to Preact X. This version was released as stable and will generate a lighter bundle and a more stable development environment. The challenging part of this migration was the way we tested some components.

This history is quite messy because I started this work a few months ago and we rebased the base branch in the meantime. Rebasing is now quite hard. I can spend some time rebasing later but not sure it's worth the trouble for now.

Why

We want to migrate to Preact X for the following reasons:

• Recent and future-proof React public API
  • Fragment support: some component could not follow the InstantSearch.css spec because Fragments were not supported: additional divs were added.
• Smaller bundle size
• Less confusion in our setup (we used to use Preact components but test them with React)

Changes

unmountComponentAtNode is not exported by Preact anymore

We now rely on rendering null (which is the internal implementation in Preact) because unmountComponentAtNode is part of preact/compat which we don't need anymore.

onChange becomes onInput on input elements

This makes it hard to test with Enzyme. Besides Enzyme's mount doesn't mount children (it shallow-render them). Therefore I introduced preact-testing-library which correctly trigger DOM events that can be caught up in tests.

React is fully removed from the codebase

We used to depend on Preact for our components but React in the testing environment, which led to inconsistent results. We now use Preact everywhere and use the h pragma. This should make the setup easier and more consistent.

Preact Rheostat dependency is gone

Having Preact Rheostat as a dependency caused some troubles: hard to remove its proptypes in production, depends on other versions of Preact, unmaintained source. Besides, the version we had wasn't compatible with Preact X (it used string refs). The Rheostat component is now part of InstantSearch.js code and will be able to evolve with time. With this change, we're also in control of what goes into our library (I removed some unused part).

Snapshot format has changed

The mount function of Enzyme with the enzyme-adapter-preact-pure adapter now always returns arrays. Component snapshots have all been updated. Some were migrated using the snapshot version of preact-testing-library (we might want to use this version for new component tests).

Snapshot to Preact's render function have changed

We used to snapshot the whole call to the Preact render function but now only snapshot the props object. It'd be nice at some point to get rid of these snapshots.

Less usage of shouldComponentUpdate

shouldComponentUpdate acts differently in Preact X and cause issues where used like this:

shouldComponentUpdate(nextProps) {
  return !isEqual(this.props, nextProps);
}

I removed cases where it was used this way. The performance gains were minimal if not inexistent.

Remove usage of componentWillMount

componentWillMount is a deprecated lifecycle hook that was trivial to remove.

Browser support

Preact X supports IE11, like InstantSearch.js.

Skipped tests

Some tests don't work with the current setup and are skipped. I suspect this is because of underlying libraries not being updated yet. If we don't plan to touch the SearchBox component, we can wait for libraries to be updated and keep these skipped tests for now.

SearchBox autofocus

The test environment doesn't reflect the autofocus prop like it does in production.

SearchBox reset

The reset event doesn't seem to be caught by the test setup.

How to review

A lot of snapshots were updated so there are a lot of files to check. Reviewing components' behavior on the Storybook and on the examples will help make sure that everything behaves as expected.

Next steps

• Migrate away from deprecated React lifecycles

Preview

• Storybook
• E-commerce demo

Related

• Closes Upgrade preact-compat #3828

References

• https://preactjs.com/guide/v10/whats-new/
• https://preactjs.com/guide/v10/upgrade-guide/#notes-for-library-authors

[Haroenv]

You can update the dev bundle size, it's 10kb smaller than the limit

[Haroenv]

Looks like this is a great change! Thanks 🙏

questions I couldn't tag:

• why is src/types/utils.ts listed when it has no changes?

[francoischalifour]

@Haroenv Good call about the utils.ts file, it an unused file that comes from a merge issue.

[yannickcr]

LGTM + @Haroenv comments.

This is a great migration 🚀
It is very nice to remove React from the tests 🙂

But it's a bit confusing to have some tests with enzyme and some tests with testing-library. What would be the cost of a future migration to only one solution for DOM testing?

[francoischalifour]

@yannickcr I didn't want to migrate the files that didn't need to be updated but we should indeed at some point get rid of one solution (likely Enzyme). From my experience it's way easier to write tests with the Testing Library, except for the few problems that I mentioned and that I'm gonna write issues about on their respective projects.

@Haroenv @yannickcr Are you guys confident enough to include this in the first v4 release? (i.e. merging now)

[yannickcr]

@francoischalifour Yes, my intent was not to migrate in the other files right now (this PR is already big 😋) but more about thinking for the future. I agree for Testing Library vs Enzyme 😇.

We are currently at 4.0.0-beta.1 so I suppose this can still be added to the release. I'm relatively confident but I would be more at ease with at least a beta and an RC before the final release.

[Haroenv]

I'm confident enough too once all comments have been solved

[Haroenv]

Let's open an issue for the onReset test