[React@18] Upgrade `@testing-library/react` #198918

Dosant · 2024-11-05T11:37:10Z

Summary

Part of https://github.com/elastic/kibana-team/issues/1016#issuecomment-2454845292

As part of upgrade to React@18 we need to upgrade testing-library. The offical upgrade path is that for React@17 testing-library < 13 should be used. And for React@18, testing-library > 13

React Testing Library versions 13+ require React v18. If your project uses an older version of React, be sure to install version 12:

This path turns out to be very problematic for us because it requires to upgrade React together with the testing-library, but we want the React runtime upgrade PR to be very small to be able to revert easily. We were ready to consider approaches were we'd bring in React@18 for unit tests, but keep runtime running React@17 https://github.com/elastic/kibana-team/issues/1016#issuecomment-2454845292

Fortunatly, we found a way to run the latest testing-library version with React@17 using legacyRoot option on render

By default we'll render with support for concurrent features (i.e. ReactDOMClient.createRoot). However, if you're dealing with a legacy app that requires rendering like in React 17 (i.e. ReactDOM.render) then you should enable this option by setting legacyRoot: true.

In this PR we're running the latest testing-library but defaulting to legacy rendering. This is done by mocking @testing-library/react as this is not global configuration options and we don't want to update every test file. Moreover, when we upgrade our packages to React@18 (phase 1 of the upgrade) this legacyRoot:true in unit tests would logically fit nicely with our plans, as Kibana will be running in legacy mode and the tests will also be rendering in legacy mode. As we will work on phase 2 and transitioning on the new concurrent mode, we will come up with parallel transition plan for the unit tests.

A pleasant surprise is that amount of places that have broken with this change is very small - a handfull of types and a couple of unit tests.

After this PR is merged, we can continue with properly addressing the largest breaking that is caused by testing-library, which for us are changes in renderHook API #195087 . With the upgraded testing-library we can make PRs to swap old renderHook with new renderHook independently from React version upgrade.

I've created an issue to track cleaning this up post upgrade #199100

…sting-library # Conflicts: # packages/kbn-test/src/jest/resolver.js

Dosant · 2024-11-05T18:59:32Z

packages/kbn-test/src/jest/resolver.js

@@ -58,6 +58,10 @@ module.exports = (request, options) => {
    });
  }

+  if (request === 'react-dom/client') {
+    return Path.resolve(__dirname, 'mocks/react_dom_client_mock.ts');


We're just mocking this with an {}, this API from React@18 won't be used because we're forcing legacyRoot:true. Just need to it help to resolve the modules. This mock can be removed with an upgrade to React@18

Dosant · 2024-11-05T19:02:48Z

packages/kbn-test/src/jest/setup/react_testing_library.js

+
+  return {
+    ...actual,
+    render: (ui, options) => actual.render(ui, { ...options, legacyRoot: true }),


Forcing legacyRoot:true by default makes testing library to use legacy API ReactDOM.render api which is how we make it work with React@17.
The option is documented here https://testing-library.com/docs/react-testing-library/api/#legacyroot .

There is no global configuration for this option, so we have to do this mocking workaround to adjust the default.

Event after upgrade of the react packages to React@18 it would make sense to keep this option as it until we start upgrading to the Concurrent Mode. We will likely come of a gradual transition plan in parallel with the runtime.

Yeah same thing here as above, I feel at minimum we need a comment to track this.

Dosant · 2024-11-05T19:03:57Z

...hboard/impl/data_quality_panel/data_quality_summary/summary_actions/check_all/index.test.tsx

@@ -403,10 +403,10 @@ describe('CheckAll', () => {

      // simulate the wall clock advancing
      for (let i = 0; i < totalIndexNames + 1; i++) {
+        await waitFor(() => {});


This test seem to rely on some very specific internal timings that apparently have changed. I found that adding another waitFor fixes the tests. Feel free to take a look and contribute to make it more robust

x-pack/plugins/cases/public/components/add_comment/index.test.tsx

Ikuni17

This approach seems good to me, nice find. My only request would be to track the removal of the mock so that bit of work isn't forgotten about. Likely in the overall React@18 issue.

elasticmachine · 2024-11-06T10:10:42Z

Pinging @elastic/appex-sharedux (Team:SharedUX)

wayneseymour

CR only, looks fine, but 2 asks and I'll approve.

wayneseymour · 2024-11-06T10:12:55Z

packages/kbn-test/src/jest/setup/react_testing_library.js

+
+  return {
+    ...actual,
+    render: (ui, options) => actual.render(ui, { ...options, legacyRoot: true }),


Yeah same thing here as above, I feel at minimum we need a comment to track this.

adcoelho

response-ops changes OK

maxcold

kubernetes_security and session_view changes look good to me

gergoabraham

@elastic/security-defend-workflows changes (that one line) look good! thank you for the upgrade! 🙌

Dosant · 2024-11-07T15:01:03Z

@kapral18, thank you for pointing that out. I think I figured this one out and should be good now. Please take another look

Dosant · 2024-11-08T09:37:24Z

@elasticmachine merge upstream

kapral18

LGTM for threat-hunting-explore

Dosant · 2024-11-11T06:42:47Z

@elasticmachine merge upstream

elasticmachine · 2024-11-11T07:44:38Z

⏳ Build in-progress

Buildkite Build
Commit: 2229964
Kibana Serverless Image: docker.elastic.co/kibana-ci/kibana-serverless:pr-198918-22299648f2a0

History

💛 Build #249478 was flaky 3c08a79
💔 Build #249273 failed 886fa4e
💔 Build #249107 failed 989fb10
💔 Build #248749 failed 09573a7
💚 Build #248613 succeeded f168f32

cc @Dosant

kibanamachine · 2024-11-11T08:31:39Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11774884870

(cherry picked from commit c8227a2)

kibanamachine · 2024-11-11T08:37:38Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

…#199592) # Backport This will backport the following commits from `main` to `8.x`: - [[React@18] Upgrade `@testing-library/react` (#198918)](#198918)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: Anton Dosov <anton.dosov@elastic.co>

Dosant added 7 commits November 5, 2024 12:36

bump testing library

b5fdc8b

Merge branch 'main' of github.com:elastic/kibana into react18/bump-te…

d509329

…sting-library # Conflicts: # packages/kbn-test/src/jest/resolver.js

focus on unit

2bdd98d

fix renderHook

bf93455

fix types

fb7b8e1

fix failed tests

1320f6d

revert scoped tests

f168f32

Dosant commented Nov 5, 2024

View reviewed changes

Ikuni17 reviewed Nov 5, 2024

View reviewed changes

Dosant changed the title ~~bump testing library~~ [React@18] Upgrade @testing-library/react Nov 6, 2024

Dosant added release_note:skip Skip the PR/issue when compiling release notes Team:SharedUX Team label for AppEx-SharedUX (formerly Global Experience) backport:prev-minor Backport to (8.x) the previous minor version (i.e. one version back from main) labels Nov 6, 2024

Dosant marked this pull request as ready for review November 6, 2024 10:10

Dosant requested review from a team as code owners November 6, 2024 10:10

Dosant requested review from joeypoon, gergoabraham and dhurley14 November 6, 2024 10:10

Dosant self-assigned this Nov 6, 2024

Dosant requested a review from Ikuni17 November 6, 2024 10:10

wayneseymour requested changes Nov 6, 2024

View reviewed changes

botelastic bot added the ci:project-deploy-observability Create an Observability project label Nov 6, 2024

adcoelho requested a review from a team as a code owner November 7, 2024 08:58

adcoelho approved these changes Nov 7, 2024

View reviewed changes

wayneseymour approved these changes Nov 7, 2024

View reviewed changes

maxcold approved these changes Nov 7, 2024

View reviewed changes

gergoabraham approved these changes Nov 7, 2024

View reviewed changes

Dosant added 2 commits November 7, 2024 15:58

better fix for ecs

0a9c257

respect testing-library’s intention to mute act warnings in some cases

ed2d951

add comment

886fa4e

Merge branch 'main' into react18/bump-testing-library

3c08a79

joeypoon approved these changes Nov 8, 2024

View reviewed changes

dhurley14 approved these changes Nov 8, 2024

View reviewed changes

Dosant mentioned this pull request Nov 8, 2024

[React@18] Env variable to use React@18 #193113

Merged

kapral18 approved these changes Nov 8, 2024

View reviewed changes

TattdCodeMonkey approved these changes Nov 8, 2024

View reviewed changes

Merge branch 'main' into react18/bump-testing-library

2229964

Dosant enabled auto-merge (squash) November 11, 2024 08:12

Dosant merged commit c8227a2 into elastic:main Nov 11, 2024
50 checks passed

kibanamachine added the v9.0.0 label Nov 11, 2024

kibanamachine pushed a commit to kibanamachine/kibana that referenced this pull request Nov 11, 2024

[React@18] Upgrade @testing-library/react (elastic#198918)

5380254

(cherry picked from commit c8227a2)

kibanamachine mentioned this pull request Nov 11, 2024

[8.x] [React@18] Upgrade `@testing-library/react` (#198918) #199592

Merged

kibanamachine added the v8.17.0 label Nov 11, 2024

CAWilson94 pushed a commit to CAWilson94/kibana that referenced this pull request Nov 11, 2024

[React@18] Upgrade @testing-library/react (elastic#198918)

fde3bdb

tkajtoch pushed a commit to tkajtoch/kibana that referenced this pull request Nov 12, 2024

[React@18] Upgrade @testing-library/react (elastic#198918)

3593b5a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[React@18] Upgrade `@testing-library/react` #198918

[React@18] Upgrade `@testing-library/react` #198918

Dosant commented Nov 5, 2024 •

edited by kibanamachine

Loading

Dosant Nov 5, 2024

Dosant Nov 5, 2024

wayneseymour Nov 6, 2024

Dosant Nov 5, 2024 •

edited

Loading

Ikuni17 left a comment

elasticmachine commented Nov 6, 2024

wayneseymour left a comment

wayneseymour Nov 6, 2024

adcoelho left a comment

maxcold left a comment •

edited

Loading

gergoabraham left a comment

Dosant commented Nov 7, 2024

Dosant commented Nov 8, 2024

kapral18 left a comment

Dosant commented Nov 11, 2024

elasticmachine commented Nov 11, 2024

kibanamachine commented Nov 11, 2024

kibanamachine commented Nov 11, 2024

[React@18] Upgrade @testing-library/react #198918

[React@18] Upgrade @testing-library/react #198918

Conversation

Dosant commented Nov 5, 2024 • edited by kibanamachine Loading

Summary

Dosant Nov 5, 2024

Choose a reason for hiding this comment

Dosant Nov 5, 2024

Choose a reason for hiding this comment

wayneseymour Nov 6, 2024

Choose a reason for hiding this comment

Dosant Nov 5, 2024 • edited Loading

Choose a reason for hiding this comment

Ikuni17 left a comment

Choose a reason for hiding this comment

elasticmachine commented Nov 6, 2024

wayneseymour left a comment

Choose a reason for hiding this comment

wayneseymour Nov 6, 2024

Choose a reason for hiding this comment

adcoelho left a comment

Choose a reason for hiding this comment

maxcold left a comment • edited Loading

Choose a reason for hiding this comment

gergoabraham left a comment

Choose a reason for hiding this comment

Dosant commented Nov 7, 2024

Dosant commented Nov 8, 2024

kapral18 left a comment

Choose a reason for hiding this comment

Dosant commented Nov 11, 2024

elasticmachine commented Nov 11, 2024

⏳ Build in-progress

History

kibanamachine commented Nov 11, 2024

kibanamachine commented Nov 11, 2024

💚 All backports created successfully

Questions ?

[React@18] Upgrade `@testing-library/react` #198918

[React@18] Upgrade `@testing-library/react` #198918

Dosant commented Nov 5, 2024 •

edited by kibanamachine

Loading

Dosant Nov 5, 2024 •

edited

Loading

maxcold left a comment •

edited

Loading