openedx / public-engineering

General public issue repository for the Open edX engineering community
3 stars 1 forks source link

[DEPR]: Enzyme #195

Open adamstankiewicz opened 1 year ago

adamstankiewicz commented 1 year ago

Proposal Date

2023-05-31

Target Ticket Acceptance Date

2023-06-16

Earliest Open edX Named Release Without This Functionality

Quince - 2023-10

Rationale

Enzyme was largely the tool of choice for most JS tests throughout the Open edX platform until React testing-library came out, which afforded developers a simpler way to write tests that more closely resembled how the web pages are used in practice by largely not relying on internal implementation details of the component. See https://testing-library.com/docs/guiding-principles/ for more details about testing-library's philosophy.

However, while most new tests being written throughout Open edX are using React testing-library, there stills exists a large number of Enzyme tests. Having these tests around would usually not be an issue, but it has recently come to our attention that Enzyme is no longer maintained; its maintainers and the community have publicly mentioned there are no plans to create a React 18 adapter for Enzyme. See more details here: https://dev.to/wojtekmaj/enzyme-is-dead-now-what-ekl

While it does not have any immediate impact on Open edX given it is largely running either React 16 and/or 17, Enzyme's lack of support for React 18 will impact and block an imminent upgrade to React 18 in the future.

Removal

Any repository which intends to upgrade to React 18 will need to remove its usage of Enzyme throughout its test suites. These Enzyme tests may be refactored using the suggestions made below.

Replacement

We recommend the use of React testing library and/or react-test-renderer.

Existing tests that rely on Enzyme will need to be refactored to use an alternative like React testing-library prior to any attempts to upgrade to React 18.

Deprecation

Stub: we may be able to include build-time deprecation warnings within @edxfrontend-build to detect when a consuming MFE is using Enzyme based on whether it's installed in the MFE's package.json file.

Migration

Incremental migration is acceptable. Open edX is currently running React 16 in most places, but React 17 in a handful of others. Once React 18 becomes more imminent, refactoring away from Enzyme will become more important to not become a blocker for a future React 18 upgrade.

Additional Info

#### Issues
- [ ] https://github.com/openedx/wg-frontend/issues/166
- [ ] https://github.com/openedx/frontend-app-learner-dashboard/issues/263
- [ ] https://github.com/openedx/frontend-app-library-authoring/issues/166
- [ ] https://github.com/openedx/paragon/issues/2325
#### PRs
- [ ] https://github.com/openedx/open-edx-proposals/pull/487
- [ ] https://github.com/openedx/open-edx-proposals/pull/501
- [ ] https://github.com/openedx/frontend-app-authn/pull/1128
- [ ] https://github.com/openedx/frontend-app-authn/pull/1117
- [ ] https://github.com/openedx/frontend-app-authn/pull/1115
- [ ] https://github.com/openedx/frontend-app-authn/pull/1122
- [ ] https://github.com/openedx/frontend-app-authn/pull/1116
- [ ] https://github.com/openedx/frontend-app-authn/pull/1123
- [ ] https://github.com/openedx/frontend-app-learner-portal-programs/pull/297
- [ ] https://github.com/openedx/frontend-app-program-console/pull/160
- [ ] https://github.com/openedx/frontend-app-learner-dashboard/pull/276
- [ ] https://github.com/openedx/frontend-app-ora-grading/pull/295
- [ ] https://github.com/openedx/frontend-app-gradebook/pull/378
- [ ] https://github.com/openedx/frontend-app-library-authoring/pull/397
- [ ] https://github.com/openedx/frontend-app-profile/pull/941
- [ ] https://github.com/openedx/frontend-app-payment/pull/844
- [ ] https://github.com/openedx/frontend-app-learner-portal-enterprise/pull/915
- [ ] https://github.com/openedx/frontend-app-admin-portal/pull/1164
- [ ] https://github.com/openedx/frontend-app-ecommerce/pull/359
- [ ] https://github.com/openedx/openedx-tech-radar/pull/19
- [ ] https://github.com/openedx/frontend-lib-content-components/pull/450
- [ ] https://github.com/openedx/frontend-app-course-authoring/pull/770
- [ ] https://github.com/openedx/frontend-platform/pull/615
- [ ] https://github.com/openedx/frontend-component-header/pull/439
- [ ] https://github.com/openedx/frontend-app-account/pull/945
- [ ] https://github.com/openedx/frontend-component-footer/pull/372
- [ ] https://github.com/openedx/paragon/pull/2611
- [ ] https://github.com/openedx/paragon/pull/2581
- [ ] frontend-app-publisher
- [ ] frontend-app-support-tools
- [ ] frontend-app-admin-portal
- [ ] frontend-app-cookie-policy-banner
jesperhodge commented 1 year ago

@adamstankiewicz before we merge this, I would love to add more guidance here on how to migrate away from enzyme. I would like to amend the "Replacement" section with something like this:

"Replacing an enzyme test with react-testing-library means rewriting a lot of code. Thus, for existing tests that you don't want to completely rewrite, we recommend using @muselesscreator 's https://github.com/muselesscreator/react-unit-test-utils as a wrapper for react-test-renderer that can for the most part act as a drop-in-replacement for enzyme without too much work.

For new tests, we recommend using react-testing-library at least partially. It allows you also to test multiple components together, test behavior as it appears to users in the browser, and test stateful components, which is not supported by react-test-renderer."

arbrandes commented 4 months ago

@Syed-Ali-Abbas-Zaidi, I notice you've been doing this migration accross a few MFEs. Do you see any PRs missing from the list in the description? Are you aware of any MFEs or libraries where the migration hasn't been done, yet?

Syed-Ali-Abbas-Zaidi commented 4 months ago

@Syed-Ali-Abbas-Zaidi, I notice you've been doing this migration accross a few MFEs. Do you see any PRs missing from the list in the description? Are you aware of any MFEs or libraries where the migration hasn't been done, yet?

List of MFEs that still need to be migrated:

  1. frontend-app-publisher
  2. frontend-app-support-tools
  3. frontend-app-admin-portal
  4. frontend-app-cookie-policy-banner
dianakhuang commented 3 months ago

@arbrandes What is the current status of this? Should we move it into 'removing'?