diff --git a/REACT_17_MIGRATION_CHECKLIST.md b/REACT_17_MIGRATION_CHECKLIST.md new file mode 100644 index 0000000000..0933256241 --- /dev/null +++ b/REACT_17_MIGRATION_CHECKLIST.md @@ -0,0 +1,267 @@ +# React 17 Migration Checklist + +This document contains the detailed list of all callsites that need to be addressed for the React 16.9 → 17 upgrade. + +--- + +## Priority 1: Deprecated Lifecycle Methods (Must Fix) + +These methods are deprecated and will log warnings in React 17. They should be migrated to avoid console noise and prepare for React 18 where they may be removed. + +### 1.1 `componentWillMount` → `componentDidMount` + +| File | Line | Current Code | Migration Strategy | +|------|------|--------------|-------------------| +| `app/internal_packages/message-list/lib/message-item-body.tsx` | 84 | Subscribes to `MessageBodyProcessor` | Move subscription to `componentDidMount`. The subscription already handles initial callback via `needInitialCallback` flag. | +| `app/src/components/billing-modal.tsx` | 29 | Fetches SSO URL if not provided | Move to `componentDidMount`. Add loading state if needed. | +| `app/internal_packages/main-calendar/lib/core/mailspring-calendar.tsx` | 98 | Subscribes to calendars and actions | Move to `componentDidMount`. Subscriptions should work the same. | +| `app/src/components/metadata-composer-toggle-button.tsx` | 57 | Checks feature usage and enables plugin | Move to `componentDidMount`. May need to handle the enabled state differently. | + +#### Migration Pattern + +```typescript +// BEFORE +componentWillMount() { + this._subscription = SomeStore.subscribe(this._onUpdate); +} + +// AFTER +componentDidMount() { + this._subscription = SomeStore.subscribe(this._onUpdate); +} +``` + +**Note:** If the subscription needs data before first render, initialize state in constructor using synchronous methods, or show a loading state. + +--- + +## Priority 2: Legacy Context API (Should Fix) + +The legacy context API (`childContextTypes`, `getChildContext`, `contextTypes`) is deprecated. While it still works in React 17, migrating to the new Context API is recommended. + +### 2.1 `TabGroupRegion` - Tab Focus Management + +**File:** `app/src/components/tab-group-region.tsx` + +**Lines:** +- Line 21: `static childContextTypes = { parentTabGroup: PropTypes.object };` +- Line 70-72: `getChildContext() { return { parentTabGroup: this }; }` + +**Purpose:** Provides tab group reference to child components for focus management. + +**Migration:** + +```typescript +// Create context file: app/src/components/tab-group-context.ts +import React from 'react'; +import type { TabGroupRegion } from './tab-group-region'; + +export const TabGroupContext = React.createContext(null); + +// In tab-group-region.tsx +import { TabGroupContext } from './tab-group-context'; + +export class TabGroupRegion extends React.Component<...> { + render() { + return ( + +
+ {this.props.children} +
+
+ ); + } +} + +// Consumers use: +// - Class components: static contextType = TabGroupContext; +// - Function components: useContext(TabGroupContext) +``` + +### 2.2 `SheetToolbar` - Sheet Depth Context + +**File:** `app/src/sheet-toolbar.tsx` + +**Lines:** +- Line 225-227: `static childContextTypes = { sheetDepth: PropTypes.number };` +- Line 237-241: `getChildContext() { return { sheetDepth: this.props.depth }; }` + +**Purpose:** Provides sheet depth to toolbar children. + +### 2.3 `Sheet` - Sheet Depth Context + +**File:** `app/src/sheet.tsx` + +**Lines:** +- Line 42-44: `static childContextTypes = { sheetDepth: PropTypes.number };` +- Line 53-57: `getChildContext() { return { sheetDepth: this.props.depth }; }` + +**Purpose:** Provides sheet depth to sheet children. + +**Migration for 2.2 & 2.3:** + +Since both `Sheet` and `SheetToolbar` provide the same context (`sheetDepth`), create a shared context: + +```typescript +// Create: app/src/sheet-context.ts +import React from 'react'; + +export const SheetDepthContext = React.createContext(0); + +// In sheet.tsx and sheet-toolbar.tsx +import { SheetDepthContext } from './sheet-context'; + +// Replace getChildContext with: +render() { + return ( + + {/* existing render content */} + + ); +} +``` + +--- + +## Priority 3: Package Updates (Required) + +### 3.1 Core React Packages + +**File:** `app/package.json` + +| Package | Current | Target | +|---------|---------|--------| +| `react` | 16.9.0 | ^17.0.2 | +| `react-dom` | 16.9.0 | ^17.0.2 | +| `react-test-renderer` | 16.9.0 | ^17.0.2 | + +### 3.2 TypeScript Type Definitions + +**File:** `package.json` (root) + +| Package | Current | Target | +|---------|---------|--------| +| `@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 | + +### 3.3 Test Adapter + +**File:** `app/package.json` + +| Package | Current | Target | +|---------|---------|--------| +| `enzyme-adapter-react-16` | ^1.15.8 | Remove | +| `@wojtekmaj/enzyme-adapter-react-17` | - | ^0.8.0 (Add) | + +**Note:** The community adapter `@wojtekmaj/enzyme-adapter-react-17` is not officially maintained but is widely used. Consider planning migration to React Testing Library for long-term stability. + +### 3.4 Related Packages to Verify + +These packages should be checked for React 17 compatibility: + +| Package | Current | Notes | +|---------|---------|-------| +| `react-transition-group` | 1.2.1 | Very old, update to ^4.4.5 | +| `react-color` | ^2.19.3 | Should be compatible | +| `slate-react` | custom fork | May need testing | + +--- + +## Priority 4: Test Infrastructure Updates + +### 4.1 Enzyme Adapter Setup + +**File:** Update test setup to use new adapter + +```typescript +// In test setup file +import Enzyme from 'enzyme'; +import Adapter from '@wojtekmaj/enzyme-adapter-react-17'; + +Enzyme.configure({ adapter: new Adapter() }); +``` + +### 4.2 Type Definition Update + +**File:** `package.json` (root) + +```diff +- "@types/enzyme-adapter-react-16": "^1.0.5", ++ "@types/enzyme-adapter-react-17": "^0.6.0", +``` + +--- + +## Priority 5: Potential Runtime Issues (Review) + +These patterns may need review but likely won't break: + +### 5.1 Document-Level Event Listeners + +React 17 attaches events to the root container instead of `document`. Code that uses `document.addEventListener` with `e.stopPropagation()` in React handlers may behave differently. + +**Files to review:** + +| File | Line | Pattern | +|------|------|---------| +| `app/src/components/resizable-region.tsx` | 188-189 | `document.addEventListener('mousemove/mouseup')` | +| `app/src/window-event-handler.ts` | 158+ | Various document listeners | +| `app/src/components/composer-editor/patch-chrome-ime.ts` | 30-99 | Document-level input handling | + +**Mitigation:** If issues occur, use capture phase: `addEventListener('click', handler, { capture: true })` + +### 5.2 Effect Cleanup Timing + +React 17 runs effect cleanup asynchronously. Code relying on synchronous cleanup may have timing issues. + +**Pattern to watch:** +```typescript +componentWillUnmount() { + // If this expects immediate cleanup before unmount completes + this.subscription.dispose(); +} +``` + +--- + +## Migration Order + +Recommended order of changes: + +1. **Update deprecated lifecycles** (4 files) - Low risk, prevents warnings +2. **Update packages** - Required for React 17 +3. **Update test adapter** - Required for tests to pass +4. **Run full test suite** - Identify any breakages +5. **Replace legacy context** (3 files) - Optional but recommended +6. **Verify runtime behavior** - Manual testing + +--- + +## Verification Checklist + +After migration, verify: + +- [ ] `npm install` completes without errors +- [ ] `npm run lint` passes +- [ ] `npm test` passes +- [ ] `npm start` launches app without React warnings +- [ ] No deprecation warnings in console +- [ ] Tab navigation works (TabGroupRegion context) +- [ ] Sheet/toolbar rendering works (SheetDepth context) +- [ ] Calendar loads and displays events +- [ ] Message bodies render correctly +- [ ] Billing modal opens and functions +- [ ] Composer toggle buttons work + +--- + +## Files Changed Summary + +| Category | Files | Changes | +|----------|-------|---------| +| Deprecated Lifecycles | 4 | Rename `componentWillMount` → `componentDidMount` | +| Legacy Context | 3 | Replace with Context API | +| Package Updates | 2 | Update versions in package.json | +| Test Setup | 1-2 | Update enzyme adapter | +| **Total** | **~10** | | diff --git a/REACT_UPGRADE_INVESTIGATION.md b/REACT_UPGRADE_INVESTIGATION.md new file mode 100644 index 0000000000..69dbfd17cf --- /dev/null +++ b/REACT_UPGRADE_INVESTIGATION.md @@ -0,0 +1,298 @@ +# React Upgrade Investigation: 16.9 → 17/18 + +## Current Setup + +- **React version**: 16.9.0 +- **React DOM version**: 16.9.0 +- **Test library**: enzyme with enzyme-adapter-react-16 +- **TypeScript types**: @types/react@^16.8.5, @types/react-dom@^16.8.2 + +--- + +## Summary + +Upgrading from React 16.9 to React 17 or 18 is **possible but requires significant refactoring**. The codebase heavily uses patterns that are deprecated or removed in newer React versions. + +| Effort Level | React 17 | React 18 | +|--------------|----------|----------| +| **Estimated** | Medium | High | +| **Breaking Changes** | Mostly warnings | Hard blockers | + +--- + +## React 17 Breaking Changes Analysis + +### 1. Event Delegation Changes (Low Risk) +React 17 attaches event handlers to the root DOM container instead of `document`. + +**Impact on Mailspring:** +- ~30 usages of `document.addEventListener` found +- Most are for global events (keydown, mouseup, mousemove) that should continue to work +- **Potential issue**: Code using `e.stopPropagation()` in React handlers may behave differently with `document`-level listeners + +**Files to review:** +- `app/src/components/resizable-region.tsx` +- `app/src/components/composer-editor/patch-chrome-ime.ts` +- `app/src/window-event-handler.ts` + +### 2. Effect Cleanup Timing (Medium Risk) +Effect cleanup now runs asynchronously in React 17. + +**Impact on Mailspring:** +- Components using refs in cleanup functions need review +- Any code assuming synchronous cleanup may break + +### 3. onScroll No Longer Bubbles (Low Risk) +The `onScroll` event no longer bubbles to parent elements. + +**Impact on Mailspring:** +- ~40 scroll handler usages found +- Most appear to be direct handlers, not relying on bubbling +- Review `app/src/components/scroll-region.tsx` for any bubbling assumptions + +### 4. Returning `undefined` Throws (Low Risk) +Components wrapped in `forwardRef` or `memo` that return `undefined` now throw. + +**No instances found** - components return `null` appropriately. + +### 5. Event Pooling Removed (No Impact) +`e.persist()` is no longer needed. + +**No usages of `e.persist()` found** - no changes needed. + +--- + +## React 18 Breaking Changes Analysis + +### 1. New Root API (REQUIRED) 🔴 + +`ReactDOM.render()` is deprecated; must use `createRoot()`. + +**Files requiring changes (11 usages):** + +| File | Line | Usage | +|------|------|-------| +| `app/src/app-env.ts` | 786 | Main app render | +| `app/src/flux/stores/popover-store.tsx` | 22, 31, 59 | Popover rendering | +| `app/src/flux/stores/modal-store.tsx` | 23, 36, 59 | Modal rendering | +| `app/internal_packages/onboarding/lib/decorators/create-page-for-form.tsx` | 195 | Form page render | +| `app/spec/spec-runner/gui-reporter.tsx` | 298 | Test reporter | +| `app/spec/spec-runner/react-test-utils-extensions.ts` | 26 | Test utils | + +**Migration pattern:** +```typescript +// Before (React 16/17) +ReactDOM.render(, container); + +// After (React 18) +import { createRoot } from 'react-dom/client'; +const root = createRoot(container); +root.render(); +``` + +### 2. ReactDOM.findDOMNode Deprecation (HIGH IMPACT) 🔴 + +`findDOMNode` is deprecated and doesn't work in Strict Mode. + +**~140 usages across the codebase!** + +**Most affected files:** +- `app/src/components/evented-iframe.tsx` (12 usages) +- `app/src/components/scroll-region.tsx` (5 usages) +- `app/src/components/tokenizing-text-field.tsx` (4 usages) +- `app/src/components/editable-table.tsx` (4 usages) +- `app/src/components/key-commands-region.tsx` (4 usages) +- `app/src/components/attachment-items.tsx` (4 usages) +- `app/src/components/decorators/has-tutorial-tip.tsx` (5 usages) + +**Migration pattern:** +```typescript +// Before +ReactDOM.findDOMNode(this) + +// After - use refs +class MyComponent extends React.Component { + private myRef = React.createRef(); + + render() { + return
...
; + } +} +``` + +### 3. String Refs Deprecation (HIGH IMPACT) 🔴 + +String refs (`ref="myRef"` / `this.refs.myRef`) are deprecated. + +**42 usages across 13 files:** +- `app/src/components/tokenizing-text-field.tsx` (7 usages) +- `app/src/components/editable-table.tsx` (7 usages) +- `app/internal_packages/thread-list/lib/thread-list.tsx` (5 usages) +- `app/src/components/scroll-region.tsx` (4 usages) +- `app/src/components/injected-component.tsx` (4 usages) + +**Migration pattern:** +```typescript +// Before + +this.refs.myInput.focus(); + +// After +private inputRef = React.createRef(); + +this.inputRef.current?.focus(); +``` + +### 4. Legacy Context API (MEDIUM IMPACT) 🟡 + +`childContextTypes` and `getChildContext` are deprecated. + +**3 files affected:** +- `app/src/components/tab-group-region.tsx` +- `app/src/sheet-toolbar.tsx` +- `app/src/sheet.tsx` + +**Migration pattern:** +```typescript +// Before +static childContextTypes = { theme: PropTypes.object }; +getChildContext() { return { theme: this.state.theme }; } + +// After +const ThemeContext = React.createContext(defaultValue); + + {children} + +``` + +### 5. Deprecated Lifecycle Methods (MEDIUM IMPACT) 🟡 + +`componentWillMount`, `componentWillReceiveProps`, `componentWillUpdate` are removed. + +**5 files still using deprecated methods:** +- `app/internal_packages/message-list/lib/message-item-body.tsx` - `componentWillMount` +- `app/internal_packages/main-calendar/lib/core/mailspring-calendar.tsx` +- `app/src/components/billing-modal.tsx` - `componentWillMount` +- `app/src/components/bind-global-commands.ts` +- `app/src/components/metadata-composer-toggle-button.tsx` + +**Note:** Previous PR #2546 addressed many of these, but 5 remain. + +### 6. Strict Mode Double-Invocation (TESTING IMPACT) 🟡 + +React 18 Strict Mode unmounts/remounts components to test resilience. + +**Impact:** May expose bugs in components with side effects in lifecycle methods. + +### 7. TypeScript: Children Prop Must Be Explicit 🟡 + +In React 18 types, `children` must be explicitly declared. + +**Impact:** ~72 class components may need type updates. + +### 8. Test Infrastructure Overhaul Required 🔴 + +**Current setup:** +- `enzyme` with `enzyme-adapter-react-16` +- Custom test utils using `ReactDOM.render` and `unmountComponentAtNode` + +**Problems:** +- No official enzyme adapter for React 17/18 +- Community adapter (`@wojtekmaj/enzyme-adapter-react-17`) exists but is unmaintained +- `react-dom/test-utils` removed `renderIntoDocument` in React 18 + +**Recommended migration:** Switch to React Testing Library + +--- + +## Dependency Updates Required + +### For React 17: +```json +{ + "react": "^17.0.2", + "react-dom": "^17.0.2", + "react-test-renderer": "^17.0.2", + "@types/react": "^17.0.0", + "@types/react-dom": "^17.0.0" +} +``` + +### For React 18: +```json +{ + "react": "^18.2.0", + "react-dom": "^18.2.0", + "react-test-renderer": "^18.2.0", + "@types/react": "^18.2.0", + "@types/react-dom": "^18.2.0" +} +``` + +### Related packages needing updates: +- `enzyme-adapter-react-16` → Remove or find alternative +- `react-transition-group` → Update to latest (compatible) +- `slate-react` (custom fork) → May need updates +- `react-color` → Check compatibility + +--- + +## Recommended Upgrade Path + +### Phase 1: Preparation (Can be done now on React 16.9) +1. ✅ Replace `componentWillReceiveProps` (PR #2546 done, 5 remaining) +2. Replace `componentWillMount` with `componentDidMount` +3. Replace string refs with `createRef` +4. Replace `findDOMNode` with explicit refs +5. Replace legacy context with Context API + +### Phase 2: React 17 Upgrade +1. Update React packages to 17.x +2. Run full test suite, fix any event-related issues +3. Address any effect cleanup timing issues +4. Update or replace enzyme adapter + +### Phase 3: React 18 Upgrade +1. Migrate `ReactDOM.render` to `createRoot` +2. Migrate `unmountComponentAtNode` to `root.unmount()` +3. Update TypeScript types for explicit children +4. Test with Strict Mode enabled +5. Consider migrating tests to React Testing Library + +--- + +## Effort Estimates + +| Task | Files | Complexity | +|------|-------|------------| +| Replace `ReactDOM.render` | 6 | Low | +| Replace `findDOMNode` | ~50 | High | +| Replace string refs | 13 | Medium | +| Replace legacy context | 3 | Medium | +| Fix deprecated lifecycles | 5 | Low | +| Update test infrastructure | ~30 spec files | High | + +**Total estimated scope:** +- React 17: ~20 files, mostly mechanical changes +- React 18: ~80+ files, significant refactoring of refs pattern + +--- + +## Recommendation + +**Short-term:** Upgrade to **React 17** first. This provides: +- Better error handling +- Gradual upgrade path to 18 +- Most deprecated APIs still work (with warnings) +- Lower risk + +**Long-term:** Plan for **React 18** upgrade but address the ~140 `findDOMNode` usages first, as this is the largest blocker. + +--- + +## References + +- [React 17 Release Notes](https://legacy.reactjs.org/blog/2020/10/20/react-v17.html) +- [React 18 Upgrade Guide](https://react.dev/blog/2022/03/08/react-18-upgrade-guide) +- [Migrating from findDOMNode](https://react.dev/reference/react-dom/findDOMNode#alternatives) +- [React Testing Library](https://testing-library.com/docs/react-testing-library/intro/) diff --git a/app/internal_packages/category-picker/lib/toolbar-category-picker.tsx b/app/internal_packages/category-picker/lib/toolbar-category-picker.tsx index 3279b8e2d8..fde6ef2aab 100644 --- a/app/internal_packages/category-picker/lib/toolbar-category-picker.tsx +++ b/app/internal_packages/category-picker/lib/toolbar-category-picker.tsx @@ -7,6 +7,7 @@ import { AccountStore, WorkspaceStore, Thread, + SheetDepthContext, } from 'mailspring-exports'; import { RetinaImg, KeyCommandsRegion } from 'mailspring-component-kit'; @@ -19,7 +20,8 @@ class MovePicker extends React.Component<{ items: Thread[] }> { static containerRequired = false; static propTypes = { items: PropTypes.array }; - static contextTypes = { sheetDepth: PropTypes.number }; + static contextType = SheetDepthContext; + declare context: number; _account: Account; _labelEl: HTMLElement; @@ -44,7 +46,7 @@ class MovePicker extends React.Component<{ items: Thread[] }> { if (!(this.props.items.length > 0)) { return; } - if (this.context.sheetDepth !== WorkspaceStore.sheetStack().length - 1) { + if (this.context !== WorkspaceStore.sheetStack().length - 1) { return; } Actions.openPopover(, { @@ -57,7 +59,7 @@ class MovePicker extends React.Component<{ items: Thread[] }> { if (!(this.props.items.length > 0)) { return; } - if (this.context.sheetDepth !== WorkspaceStore.sheetStack().length - 1) { + if (this.context !== WorkspaceStore.sheetStack().length - 1) { return; } Actions.openPopover(, { diff --git a/app/internal_packages/composer/lib/composer-header.tsx b/app/internal_packages/composer/lib/composer-header.tsx index 4077dea0a4..267171dd28 100644 --- a/app/internal_packages/composer/lib/composer-header.tsx +++ b/app/internal_packages/composer/lib/composer-header.tsx @@ -12,6 +12,8 @@ import { KeyCommandsRegion, ParticipantsTextField, ListensToFluxStore, + TabGroupContext, + TabGroupContextType, } from 'mailspring-component-kit'; import AccountContactField from './account-contact-field'; import ComposerHeaderActions from './composer-header-actions'; @@ -44,9 +46,8 @@ export class ComposerHeader extends React.Component { if (ReactDOM.findDOMNode(this._els[fieldName]).contains(document.activeElement)) { - this.context.parentTabGroup.shiftFocus(-1); + this.context?.shiftFocus(-1); } const enabledFields = this.state.enabledFields.filter(n => n !== fieldName); diff --git a/app/internal_packages/main-calendar/lib/core/mailspring-calendar.tsx b/app/internal_packages/main-calendar/lib/core/mailspring-calendar.tsx index d89f8b08c6..bf72a7d1f8 100644 --- a/app/internal_packages/main-calendar/lib/core/mailspring-calendar.tsx +++ b/app/internal_packages/main-calendar/lib/core/mailspring-calendar.tsx @@ -61,7 +61,7 @@ interface MailspringCalendarState { view: CalendarView; selectedEvents: EventOccurrence[]; focusedEvent: FocusedEventInfo | null; - accounts?: Account[]; + accounts: Account[]; calendars: Calendar[]; focusedMoment: Moment; disabledCalendars: string[]; @@ -87,6 +87,7 @@ export class MailspringCalendar extends React.Component< super(props); this.state = { calendars: [], + accounts: [], focusedEvent: null, selectedEvents: [], view: CalendarView.WEEK, @@ -95,7 +96,7 @@ export class MailspringCalendar extends React.Component< }; } - componentWillMount() { + componentDidMount() { this._disposable = this._subscribeToCalendars(); this._unlisten = Actions.focusCalendarEvent.listen(this._focusEvent); } diff --git a/app/internal_packages/message-list/lib/message-item-body.tsx b/app/internal_packages/message-list/lib/message-item-body.tsx index 8aafa3f2c9..32134df247 100644 --- a/app/internal_packages/message-list/lib/message-item-body.tsx +++ b/app/internal_packages/message-list/lib/message-item-body.tsx @@ -81,7 +81,8 @@ export default class MessageItemBody extends React.Component< }; } - componentWillMount() { + componentDidMount() { + this._mounted = true; const needInitialCallback = this.state.processedBody === null; this._unsub = MessageBodyProcessor.subscribe( this.props.message, @@ -90,10 +91,6 @@ export default class MessageItemBody extends React.Component< ); } - componentDidMount() { - this._mounted = true; - } - componentDidUpdate(prevProps: MessageItemBodyProps) { if (this.props.message.id !== prevProps.message.id) { if (this._unsub) { diff --git a/app/package.json b/app/package.json index 60106460de..91bfa2aa08 100644 --- a/app/package.json +++ b/app/package.json @@ -25,7 +25,7 @@ "dompurify": "^3.3.1", "emoji-data": "^0.2.0", "enzyme": "^3.11.0", - "enzyme-adapter-react-16": "^1.15.8", + "@wojtekmaj/enzyme-adapter-react-17": "^0.8.0", "event-kit": "^1.0.2", "fs-plus": "^2.3.2", "getmac": "^1.2.1", @@ -51,10 +51,10 @@ "pick-react-known-prop": "0.x.x", "proxyquire": "1.3.1", "raven": "2.1.2", - "react": "16.9.0", + "react": "^17.0.2", "react-color": "^2.19.3", - "react-dom": "16.9.0", - "react-test-renderer": "16.9.0", + "react-dom": "^17.0.2", + "react-test-renderer": "^17.0.2", "react-transition-group": "1.2.1", "reflux": "0.1.13", "rimraf": "2.5.2", diff --git a/app/spec/spec-runner/spec-runner.ts b/app/spec/spec-runner/spec-runner.ts index 59c9cbd4aa..ac9e424110 100644 --- a/app/spec/spec-runner/spec-runner.ts +++ b/app/spec/spec-runner/spec-runner.ts @@ -1,7 +1,7 @@ /* eslint global-require:0 */ import _ from 'underscore'; import Enzyme from 'enzyme'; -import Adapter from 'enzyme-adapter-react-16'; +import Adapter from '@wojtekmaj/enzyme-adapter-react-17'; import ReactTestUtils from 'react-dom/test-utils'; import Config from '../../src/config'; diff --git a/app/src/components/billing-modal.tsx b/app/src/components/billing-modal.tsx index 73c2a582c7..f7f6b9f1a2 100644 --- a/app/src/components/billing-modal.tsx +++ b/app/src/components/billing-modal.tsx @@ -26,16 +26,9 @@ export default class BillingModal extends React.Component { - if (!this._mounted) return; - this.setState({ src: url }); - }); - } - } - componentDidMount() { + this._mounted = true; + // Due to a bug in Electron, opening a webview with a non 100% size when // the app has a custom zoomLevel scales it's contents incorrectly and no // CSS will fix it. Fix this by just temporarily reverting zoom to 1.0 @@ -44,7 +37,14 @@ export default class BillingModal extends React.Component { + if (!this._mounted) return; + this.setState({ src: url }); + }); + } } /** diff --git a/app/src/components/date-picker.tsx b/app/src/components/date-picker.tsx index 6874a2796a..10346c7b17 100644 --- a/app/src/components/date-picker.tsx +++ b/app/src/components/date-picker.tsx @@ -1,8 +1,9 @@ import moment from 'moment'; import classnames from 'classnames'; import React from 'react'; -import { PropTypes, DateUtils } from 'mailspring-exports'; +import { DateUtils } from 'mailspring-exports'; import { MiniMonthView } from 'mailspring-component-kit'; +import { TabGroupContext, TabGroupContextType } from './tab-group-context'; type DatePickerProps = { value?: number; @@ -16,9 +17,8 @@ type DatePickerState = { export class DatePicker extends React.Component { static displayName = 'DatePicker'; - static contextTypes = { - parentTabGroup: PropTypes.object, - }; + static contextType = TabGroupContext; + declare context: TabGroupContextType | null; static defaultProps = { dateFormat: null, // Default to valueOf @@ -57,7 +57,7 @@ export class DatePicker extends React.Component { this._onChange(moment(newTimestamp)); - this.context.parentTabGroup.shiftFocus(1); + this.context?.shiftFocus(1); }; _renderMiniMonthView() { diff --git a/app/src/components/metadata-composer-toggle-button.tsx b/app/src/components/metadata-composer-toggle-button.tsx index 409294eb30..546eb356b2 100644 --- a/app/src/components/metadata-composer-toggle-button.tsx +++ b/app/src/components/metadata-composer-toggle-button.tsx @@ -54,7 +54,8 @@ export default class MetadataComposerToggleButton extends React.Component< }; } - componentWillMount() { + componentDidMount() { + // Auto-enable plugin if it should be on by default and isn't already enabled if (this._isEnabledByDefault() && !this._isEnabled()) { if (FeatureUsageStore.isUsable(this.props.pluginId)) { this._setEnabled(true); diff --git a/app/src/components/tab-group-context.ts b/app/src/components/tab-group-context.ts new file mode 100644 index 0000000000..97b486ec7b --- /dev/null +++ b/app/src/components/tab-group-context.ts @@ -0,0 +1,19 @@ +import React from 'react'; + +/** + * Interface for the TabGroup context value. + * Provides focus management within a group of tabbable elements. + */ +export interface TabGroupContextType { + /** + * Shifts focus to the next (dir=1) or previous (dir=-1) tabbable element + * within the tab group. + */ + shiftFocus: (dir: number) => void; +} + +/** + * Context for managing tab focus within a group of elements. + * Used by TabGroupRegion to provide focus navigation to child components. + */ +export const TabGroupContext = React.createContext(null); diff --git a/app/src/components/tab-group-region.tsx b/app/src/components/tab-group-region.tsx index 4fe7048d7c..e6efac0cbe 100644 --- a/app/src/components/tab-group-region.tsx +++ b/app/src/components/tab-group-region.tsx @@ -7,7 +7,7 @@ */ import React from 'react'; import ReactDOM from 'react-dom'; -import PropTypes from 'prop-types'; +import { TabGroupContext, TabGroupContextType } from './tab-group-context'; let compositionActive = false; document.addEventListener('compositionstart', () => { @@ -18,7 +18,10 @@ document.addEventListener('compositionend', () => { }); export class TabGroupRegion extends React.Component> { - static childContextTypes = { parentTabGroup: PropTypes.object }; + // Stable context value object to avoid unnecessary re-renders of consumers + private _contextValue: TabGroupContextType = { + shiftFocus: this.shiftFocus, + }; _onKeyDown = event => { if (event.key !== 'Tab' || event.defaultPrevented) return; @@ -67,15 +70,13 @@ export class TabGroupRegion extends React.Component - {this.props.children} - + +
+ {this.props.children} +
+
); } } diff --git a/app/src/components/time-picker.tsx b/app/src/components/time-picker.tsx index d3d7f4e73b..a5680f89eb 100644 --- a/app/src/components/time-picker.tsx +++ b/app/src/components/time-picker.tsx @@ -3,6 +3,7 @@ import ReactDOM from 'react-dom'; import PropTypes from 'prop-types'; import moment from 'moment'; import classnames from 'classnames'; +import { TabGroupContext, TabGroupContextType } from './tab-group-context'; require('moment-round'); // overrides moment @@ -27,9 +28,8 @@ export default class TimePicker extends React.Component require('../sheet-context').SheetDepthContext); + // Decorators lazyLoad(`InflatesDraftClientId`, 'decorators/inflates-draft-client-id'); diff --git a/app/src/sheet-context.ts b/app/src/sheet-context.ts new file mode 100644 index 0000000000..59d950109d --- /dev/null +++ b/app/src/sheet-context.ts @@ -0,0 +1,8 @@ +import React from 'react'; + +/** + * Context for providing sheet depth information to child components. + * Used by Sheet and SheetToolbar to communicate the current sheet's + * depth in the workspace stack. + */ +export const SheetDepthContext = React.createContext(0); diff --git a/app/src/sheet-toolbar.tsx b/app/src/sheet-toolbar.tsx index f581ffcba5..8dcefa6430 100644 --- a/app/src/sheet-toolbar.tsx +++ b/app/src/sheet-toolbar.tsx @@ -5,6 +5,7 @@ import PropTypes from 'prop-types'; import ReactDOM from 'react-dom'; import { localized, isRTL, Actions, ComponentRegistry, WorkspaceStore } from 'mailspring-exports'; +import { SheetDepthContext } from './sheet-context'; import { SheetDeclaration } from './flux/stores/workspace-store'; import { Flexbox } from './components/flexbox'; import { RetinaImg } from './components/retina-img'; @@ -217,15 +218,6 @@ let lastReportedToolbarHeight = 0; export default class Toolbar extends React.Component { static displayName = 'Toolbar'; - static propTypes = { - data: PropTypes.object, - depth: PropTypes.number, - }; - - static childContextTypes = { - sheetDepth: PropTypes.number, - }; - mounted = false; unlisteners: Array<() => void> = []; @@ -234,12 +226,6 @@ export default class Toolbar extends React.Component this.state = this._getStateFromStores(); } - getChildContext() { - return { - sheetDepth: this.props.depth, - }; - } - componentDidMount() { this.mounted = true; this.unlisteners = []; @@ -394,18 +380,20 @@ export default class Toolbar extends React.Component )); return ( -
- {toolbars} -
+ +
+ {toolbars} +
+
); } } diff --git a/app/src/sheet.tsx b/app/src/sheet.tsx index 552ab42ce7..d5fd637411 100644 --- a/app/src/sheet.tsx +++ b/app/src/sheet.tsx @@ -1,7 +1,7 @@ import React, { CSSProperties } from 'react'; -import PropTypes from 'prop-types'; import { Utils, ComponentRegistry, WorkspaceStore } from 'mailspring-exports'; +import { SheetDepthContext } from './sheet-context'; import { InjectedComponentSet } from './components/injected-component-set'; import { ResizableRegion } from './components/resizable-region'; import { Flexbox } from './components/flexbox'; @@ -39,10 +39,6 @@ export default class Sheet extends React.Component { onColumnSizeChanged: () => {}, }; - static childContextTypes = { - sheetDepth: PropTypes.number, - }; - private unlisteners = []; constructor(props) { @@ -50,12 +46,6 @@ export default class Sheet extends React.Component { this.state = this._buildState(); } - getChildContext() { - return { - sheetDepth: this.props.depth, - }; - } - componentDidMount() { this.unlisteners = [ ComponentRegistry.listen(() => this.setState(this._buildState())), @@ -224,16 +214,18 @@ export default class Sheet extends React.Component { // http://philipwalton.com/articles/what-no-one-told-you-about-z-index/ return ( -
- - {this._columnFlexboxElements()} - -
+ +
+ + {this._columnFlexboxElements()} + +
+
); } } diff --git a/package.json b/package.json index 2156698196..59a201ff0d 100644 --- a/package.json +++ b/package.json @@ -18,7 +18,6 @@ "@types/debug": "^4.1.2", "@types/deep-extend": "^0.4.31", "@types/enzyme": "^3.9.0", - "@types/enzyme-adapter-react-16": "^1.0.5", "@types/event-kit": "^2.4.0", "@types/fs-plus": "^3.0.1", "@types/graceful-fs": "^4.1.3", @@ -30,10 +29,10 @@ "@types/optimist": "^0.0.29", "@types/proxyquire": "^1.3.28", "@types/raven": "^2.5.3", - "@types/react": "^16.8.5", + "@types/react": "^17.0.0", "@types/react-color": "^2.14.1", - "@types/react-dom": "^16.8.2", - "@types/react-test-renderer": "^16.8.1", + "@types/react-dom": "^17.0.0", + "@types/react-test-renderer": "^17.0.0", "@types/react-transition-group": "^1.1.6", "@types/reflux": "^6.4.2", "@types/rimraf": "^2.0.2",