MM-67120 - Upgrade to React 19 and other outdated libraries#219
MM-67120 - Upgrade to React 19 and other outdated libraries#219nevyangelova merged 5 commits intomasterfrom
Conversation
|
Plugin Spinwick PR #219 🎉 Test server created! Access here: https://confluence-pr-219-ritgk.test.mattermost.cloud
The test server was created successfully, but there was an issue installing or enabling the plugin automatically:
You can manually install the plugin:
Future commits will still attempt to automatically update the plugin. Installation ID: Credentials: Posted securely in this Mattermost channel - Look for PR #219 |
avasconcelos114
left a comment
There was a problem hiding this comment.
Great stuff! it feels much nicer to have react testing library instead of enzyme for the unit tests
I just have one main thing to confirm before continuing with tests
| expect(screen.getByText('Edit Your Confluence Subscription')).toBeInTheDocument(); | ||
| expect(screen.getByText('Name')).toBeInTheDocument(); | ||
| expect(screen.getByText('Confluence Base URL')).toBeInTheDocument(); | ||
| expect(screen.getByText('Subscribe To')).toBeInTheDocument(); | ||
| expect(screen.getByText('Events')).toBeInTheDocument(); | ||
| expect(screen.getByText('Save Subscription')).toBeInTheDocument(); | ||
| expect(screen.getByText('Cancel')).toBeInTheDocument(); |
There was a problem hiding this comment.
Very nice to see a test that is more specific than a basic snapshot test
Which gets me thinking, maybe as we upgrade everything, should we move away from these snapshot tests and replace them with more specific checks like this one? (where we have explicit assertions for which elements we should see in the component)
There was a problem hiding this comment.
that's exactly what this PR does, we removed the snapshot test and replaced it with explicit element assertions.
There was a problem hiding this comment.
Yep yep, i was just commenting that I'm for making sure we replace snapshot tests in all repos (since in theory we can still keep them with react testing library), but I personally prefer what you did here
| const nameInput = screen.getByPlaceholderText('Enter a name for this subscription.'); | ||
| const urlInput = screen.getByPlaceholderText('Enter the Confluence Base URL.'); | ||
| const spaceKeyInput = screen.getByPlaceholderText('Enter the Confluence Space Key.'); |
There was a problem hiding this comment.
Nit: We could make use of test IDs for elements so that selectors aren't affected by any changes in placeholder texts or other labels (https://testing-library.com/docs/queries/bytestid/)
webapp/package.json
Outdated
| "react-redux": "7.2.0", | ||
| "react-select": "3.1.0", | ||
| "redux": "4.0.5", | ||
| "react": "19.2.3", |
There was a problem hiding this comment.
Question: I think the target version for upgrade was React 18 (to match what the Mattermost webapp is running https://github.com/mattermost/mattermost/blob/master/webapp/channels/package.json#L62), do we want to make sure plugins are not moving ahead of the webapp itself or would you consider it safe to proceed with upgrading to React 19?
There was a problem hiding this comment.
I think its safe to be on top of things with plugins, and there aren't breaking changes between the versions that would affect the webapp. @marianunez what do you think?
There was a problem hiding this comment.
Commenting here what we discussed offline: We can upgrade to React 19 but we can't include any new APIs that may fail when the plugin is running a MM Server with React 18 ( > v11) and React 17 (< v11)
|
Plugin test server update attempted, but encountered an issue: The test server is still available. You can manually download and install the updated plugin using the artifact link below. Updated with git commit
|
There was a problem hiding this comment.
Pull request overview
This PR upgrades React from 16.13.1 to 19.2.3 and migrates the test framework from Enzyme to React Testing Library to address security vulnerabilities and modernize the codebase.
Changes:
- Upgraded React and related libraries (react-dom, react-redux, redux, react-bootstrap, react-select, webpack)
- Migrated from Enzyme to React Testing Library for component testing
- Updated component tests to use RTL patterns with proper async handling and user interactions
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| webapp/tests/setup.js | Replaced Enzyme configuration with React Testing Library setup |
| webapp/src/components/subscription_modal/subscription_modal.test.jsx | Migrated all tests from Enzyme shallow rendering to RTL with proper user interaction simulations |
| webapp/src/components/subscription_modal/subscription_modal.jsx | Updated Button component props from bsStyle to variant for react-bootstrap v2 compatibility and added testId props |
| webapp/src/components/subscription_modal/snapshots/subscription_modal.test.jsx.snap | Removed snapshot file as tests now use RTL assertions instead of snapshots |
| webapp/src/components/confluence_field.jsx | Updated react-bootstrap components to v2 API and replaced prop spreading with explicit props |
| webapp/package.json | Updated dependency versions and removed Enzyme-related packages |
| webapp/.eslintrc.json | Added React version detection and test file overrides for ESLint rules |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| "js-cookie": "2.2.1", | ||
| "mattermost-redux": "10.9.0", | ||
| "react": "16.13.1", | ||
| "react-bootstrap": "1.3.0", |
There was a problem hiding this comment.
This was a major update - what version are we using in the webapp? Will this cause issues?
There was a problem hiding this comment.
webapp is mostly on "react": "18.2.0", from my test it's backwards compatible. The changes affect only bootstrap props and unit tests in this case.
There was a problem hiding this comment.
Sorry, was referring to react-bootstrap, I know the webapp loads it's own version of react-bootstrap so just want to make sure we don't have any conflicts by upgrading from 1.3 -> 2.10
There was a problem hiding this comment.
Yeah, it seems for older versions of MM we must use [email protected] because the Mattermost webapp exposes a custom fork of react-bootstrap v0.32.4. At runtime the plugin uses Mattermost's ReactBootstrap not our bundled version.But this is a bit strange for me? For unit tests and local development we the local node_modules versions but at runtime inside Mattermost the webpack externals config the bundled main.js doesn't include React and ReactBootstrap. Can we just use whatever versions we specify and decouple from being dependent on MM bundles?
| "react-bootstrap": "1.3.0", | ||
| "react-dom": "16.13.1", | ||
| "react-redux": "7.2.0", | ||
| "react-select": "3.1.0", |
There was a problem hiding this comment.
Same as above as this is also a major update, any conflicts with the webapp?
There was a problem hiding this comment.
webapp is mostly on "react": "18.2.0", for me it's backwards compatible. The changes affect only bootstrap props and unit tests in this case.
There was a problem hiding this comment.
Here I was mostly concerned about react-select going from 3.1 to 5.10 - just double checking that it doesn't conflict with webapp as it's a 2 major version bumps
avasconcelos114
left a comment
There was a problem hiding this comment.
Code-wise I think everything looks good, just want to have those regression tests on latest and ESR versions to make sure nothing is conflicting (I don't anticipate it to because since the JS bundle for the plugin happens in complete isolation from Mattermost's)
|
@nevyangelova Runnning |
|
Plugin test server update attempted, but encountered an issue: The test server is still available. You can manually download and install the updated plugin using the artifact link below. Updated with git commit
|
|
Test server destroyed |
Summary
Upgrades frontend dependencies to fix security vulnerabilities and modernize the codebase.
Upgraded React from 16.13.1 to 19.2.3
Upgraded react-dom from 16.13.1 to 19.2.3
Upgraded react-redux from 7.2.0 to 9.2.0
Upgraded redux from 4.0.5 to 5.0.0
Upgraded react-bootstrap from 1.3.0 to 2.10.0
Upgraded react-select from 3.1.0 to 5.10.2
Upgraded webpack from 5.91.0 to 5.104.1
Migrated test framework from Enzyme to React Testing Library
https://mattermost.atlassian.net/browse/MM-67120