Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DEPR]: Enzyme #195

Open
32 of 38 tasks
adamstankiewicz opened this issue May 31, 2023 · 10 comments
Open
32 of 38 tasks

[DEPR]: Enzyme #195

adamstankiewicz opened this issue May 31, 2023 · 10 comments
Assignees
Labels
depr Proposal for deprecation & removal per OEP-21 epic Large unit of work, consisting of multiple tasks

Comments

@adamstankiewicz
Copy link
Member

adamstankiewicz commented May 31, 2023

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

Outstanding Issues

  1. maintenance
    GlugovGrGlib
  2. maintenance needs maintainer attention
    jciasenza

Issues

  1. epic maintenance
  2. 1 of 1
  3. 1 of 1
  4. 0 of 1
    engineering epic maintenance raccoon-gang
    monteri

PRs

  1. arbrandes
  2. BilalQamar95
  3. Syed-Ali-Abbas-Zaidi
  4. Syed-Ali-Abbas-Zaidi
  5. BilalQamar95
  6. Syed-Ali-Abbas-Zaidi
  7. zubair-ce07
  8. Syed-Ali-Abbas-Zaidi
  9. Syed-Ali-Abbas-Zaidi
  10. Syed-Ali-Abbas-Zaidi
  11. Syed-Ali-Abbas-Zaidi
  12. released released on @alpha
  13. blended released released on @alpha
@adamstankiewicz adamstankiewicz added the depr Proposal for deprecation & removal per OEP-21 label May 31, 2023
@adamstankiewicz adamstankiewicz moved this from Proposed to Communicated in DEPR: Deprecation & Removal May 31, 2023
@jesperhodge
Copy link
Member

jesperhodge commented Jun 14, 2023

@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
Copy link

@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?

@arbrandes arbrandes moved this from Backlog to In Progress in Axim Engineering Tasks Mar 6, 2024
@Syed-Ali-Abbas-Zaidi
Copy link

@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
Copy link

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

@dianakhuang
Copy link

dianakhuang commented Jul 25, 2024

Since it seems like this is not active and the owners are not being responsive, we're going to move this back into 'Proposed'

edit: Rescinding previous comment since @feanil took ownership of it.

@feanil feanil self-assigned this Jul 25, 2024
@feanil feanil moved this to In Progress in Maintenance Jul 25, 2024
@MaxFrank13
Copy link
Member

Thanks for bringing this back to life. Not being on React 18 is forcing us to stay on version 12 of React Testing Library. We even had a couple PRs with dependency conflicts get merged by renovate:

@feanil
Copy link

feanil commented Jul 25, 2024

@MaxFrank13 can you provide some more info, it sounds like your repos are already on React Testing Library and so your issues are unrelated to enzyme? Maybe I missed something?

@MaxFrank13
Copy link
Member

Yes you're right. It's not directly related. It's just that deprecating Enzyme helps clear the way for React 18, and until then, we have to stay on React Testing Library v12 (the latest is v16). Other than Renovate merging those PRs with conflicts, this isn't really causing problems other than preventing us from staying up to date.

The only issue I've been able to find that is referencing React 18 upgrades is one for the React 17 upgrade in platform-roadmap, and this Enzyme deprecation issue. Perhaps I'm not looking in the right places or maybe the work just hasn't been prioritized yet.

Worth noting that Enzyme isn't the only blocker for React 18. We'd still need to upgrade other FE dependencies/peerDependencies in places like frontend-platform and frontend-build to support React 18. And perhaps that's why work on this issue was paused?

@feanil
Copy link

feanil commented Jul 29, 2024

It sounds like what you actually care about is tracking openedx/platform-roadmap#277 which you're right is not currently being lead/in-progress anymore.

My recommendation is that if there are specifics preventing upgrades in repos you care about to create issues in those repos about the specific upgrades that need to happen and to work with the maintainers to plan/take-on that work. If you need help with that please let me know but it sounds like your issue is more that "We're not on react 18 yet in learner record and profile" but the root cause of that is not the fact that those repos are still running "Enzyme" but some other blocker or just scheduling the work to do the upgrade.

@MaxFrank13
Copy link
Member

Ahh thanks for the suggestion. I think I incorrectly assumed that upgrades to frontend-platform and frontend-build were also waiting on the deprecation of Enzyme. I see now that technically that shouldn't be the case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
depr Proposal for deprecation & removal per OEP-21 epic Large unit of work, consisting of multiple tasks
Projects
Status: In Progress
Status: Accepted
Status: In progress
Status: In Progress
Status: Being Developed
Development

No branches or pull requests

7 participants