Upgrade to React 17, fix usage of contextTypes#2548
Open
Conversation
Document the analysis of upgrading React from 16.9 to 17 or 18, including all breaking changes, affected files, and recommendations. Key findings: - ~140 usages of deprecated findDOMNode - 42 string ref usages across 13 files - 11 ReactDOM.render calls need createRoot migration - 5 files still using deprecated lifecycle methods - 3 files using legacy context API - Test infrastructure needs overhaul (enzyme-adapter-react-16) Recommendation: Upgrade to React 17 first (medium effort), then plan React 18 upgrade after addressing findDOMNode usage.
Comprehensive checklist with specific file paths and line numbers for all callsites that need to be addressed: Priority 1 - Deprecated Lifecycle Methods (4 files): - message-item-body.tsx:84 - componentWillMount - billing-modal.tsx:29 - componentWillMount - mailspring-calendar.tsx:98 - componentWillMount - metadata-composer-toggle-button.tsx:57 - componentWillMount Priority 2 - Legacy Context API (3 files): - tab-group-region.tsx:21,70 - parentTabGroup context - sheet-toolbar.tsx:225,237 - sheetDepth context - sheet.tsx:42,53 - sheetDepth context Priority 3 - Package Updates: - react, react-dom, react-test-renderer → ^17.0.2 - @types/react, @types/react-dom → ^17.0.0 - enzyme-adapter-react-16 → @wojtekmaj/enzyme-adapter-react-17 Includes migration patterns and verification checklist.
This commit implements the React 17 upgrade with the following changes: ## Deprecated Lifecycle Methods (4 files) - message-item-body.tsx: componentWillMount → componentDidMount - billing-modal.tsx: componentWillMount → componentDidMount - mailspring-calendar.tsx: componentWillMount → componentDidMount - metadata-composer-toggle-button.tsx: componentWillMount → componentDidMount ## Legacy Context API Migration (3 providers, 4 consumers) Created new React Context APIs to replace legacy childContextTypes: ### TabGroupContext (for focus management) - Provider: tab-group-region.tsx - Consumers: date-picker.tsx, time-picker.tsx, composer-header.tsx ### SheetDepthContext (for sheet depth tracking) - Providers: sheet.tsx, sheet-toolbar.tsx - Consumer: toolbar-category-picker.tsx ## Package Updates app/package.json: - react: 16.9.0 → ^17.0.2 - react-dom: 16.9.0 → ^17.0.2 - react-test-renderer: 16.9.0 → ^17.0.2 - enzyme-adapter-react-16 → @wojtekmaj/enzyme-adapter-react-17 package.json (root): - @types/react: ^16.8.5 → ^17.0.0 - @types/react-dom: ^16.8.2 → ^17.0.0 - @types/react-test-renderer: ^16.8.1 → ^17.0.0 - Removed @types/enzyme-adapter-react-16 ## New Files - app/src/components/tab-group-context.ts - app/src/sheet-context.ts ## Test Infrastructure - Updated spec-runner.ts to use @wojtekmaj/enzyme-adapter-react-17
The migration from componentWillMount to componentDidMount caused the component to render with accounts: undefined before the subscription fired. CalendarSourceList would then crash trying to call accounts.find() on undefined. Fixed by: - Initializing accounts: [] in constructor - Making accounts required in the state interface (was optional)
bengotow
changed the title
bengotow
changed the title
bengotow
force-pushed
the
master
branch
2 times, most recently
from
0453e6a to
13dc018
Compare
|
whats the status here ? |
Collaborator
Author
|
Ahh I think this one is going to take some testing but is mergeable - it looks like we can likely go 16 => 17, but 17 => 18 will be more difficult because of the Lexical rich text library. The newer version of Lexical was effectively a full re-write (honestly they should have renamed it I think). Feel free to pull down this PR and give it a spin! sidenote, I don't consider the old version of React to be a security vulnerability the way Node-facing old packages can be -- I updated the app to React 16.9 so that we could use React Hooks (which are sort of essential these days!) It'd be nice to move to the highest React version we can, but there's not much urgency on this one. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Document the analysis of upgrading React from 16.9 to 17 or 18,
including all breaking changes, affected files, and recommendations.
Key findings:
Recommendation: Upgrade to React 17 first (medium effort),
then plan React 18 upgrade after addressing findDOMNode usage.