refactor: migrate OutsideStack to React Navigation static config - #7410
Conversation
Convert the OutsideStack login-flow navigators from the dynamic JSX API to
React Navigation's static config (createNativeStackNavigator({ screens })).
Theme-dependent header options move to a .with() wrapper that reads
ThemeContext, and a .getComponent() bridge lets the still-dynamic root embed
the static navigator. Param lists are now derived from the config via
StaticParamList, replacing the hand-written lists in stacks/types.ts.
Bumps @react-navigation/* to the latest 7.x, required for the static-config
.with() and .getComponent() APIs.
GitHub Actions
Inactive
WalkthroughReact Navigation packages are bumped to newer versions. ChangesOutside Stack static API migration
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested labels
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Warning Review ran into problems🔥 ProblemsErrors were encountered while retrieving linked issues. Errors (1)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
GitHub Actions
Failure
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/stacks/OutsideStack.tsx`:
- Around line 16-25: The NewServerView screen is registered without the
createNativeStackScreen wrapper and defaultHeader options, while all other
screens in the Outside navigator (WorkspaceView, LoginView, ForgotPasswordView,
SendEmailConfirmationView, RegisterView, and LegalView) are wrapped with
createNativeStackScreen and include the defaultHeader options. To fix this, wrap
NewServerView with createNativeStackScreen and apply the defaultHeader options
the same way the other screens are configured to ensure consistent header
styling across the Outside stack.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 0ab27dd7-b342-4eb9-94bd-634e211f34a9
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (10)
app/stacks/OutsideStack.tsxapp/stacks/types.tsapp/views/AuthenticationWebView.tsxapp/views/ForgotPasswordView.tsxapp/views/LoginView/index.tsxapp/views/NewServerView/definitions.tsapp/views/NewServerView/index.tsxapp/views/RegisterView/index.tsxapp/views/SendEmailConfirmationView.tsxpackage.json
💤 Files with no reviewable changes (1)
- app/views/NewServerView/definitions.ts
📜 Review details
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ESLint and Test / run-eslint-and-test
- GitHub Check: format
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{js,ts,jsx,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{js,ts,jsx,tsx}: Use descriptive names for functions, variables, and classes that clearly convey their purpose
Write comments that explain the 'why' behind code decisions, not the 'what'
Keep functions small and focused on a single responsibility
Use const by default, let when reassignment is needed, and avoid var
Prefer async/await over .then() chains for handling asynchronous operations
Use explicit error handling with try/catch blocks for async operations
Avoid deeply nested code; refactor complex logic into helper functions
Files:
app/stacks/OutsideStack.tsxapp/views/AuthenticationWebView.tsxapp/views/ForgotPasswordView.tsxapp/views/LoginView/index.tsxapp/views/SendEmailConfirmationView.tsxapp/stacks/types.tsapp/views/NewServerView/index.tsxapp/views/RegisterView/index.tsx
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Use TypeScript for type safety; add explicit type annotations to function parameters and return types
Prefer interfaces over type aliases for defining object shapes in TypeScript
Use enums for sets of related constants rather than magic strings or numbersUse TypeScript with strict mode enabled
Files:
app/stacks/OutsideStack.tsxapp/views/AuthenticationWebView.tsxapp/views/ForgotPasswordView.tsxapp/views/LoginView/index.tsxapp/views/SendEmailConfirmationView.tsxapp/stacks/types.tsapp/views/NewServerView/index.tsxapp/views/RegisterView/index.tsx
**/*.{js,jsx,ts,tsx,json}
📄 CodeRabbit inference engine (CLAUDE.md)
Use Prettier formatting with tabs, single quotes, 130 character line width, no trailing commas, and avoid arrow function parentheses
Files:
app/stacks/OutsideStack.tsxapp/views/AuthenticationWebView.tsxapp/views/ForgotPasswordView.tsxapp/views/LoginView/index.tsxapp/views/SendEmailConfirmationView.tsxpackage.jsonapp/stacks/types.tsapp/views/NewServerView/index.tsxapp/views/RegisterView/index.tsx
**/*.{js,jsx,ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Enforce ESLint rules from
@rocket.chat/eslint-configwith React, React Native, TypeScript, and Jest plugins
Files:
app/stacks/OutsideStack.tsxapp/views/AuthenticationWebView.tsxapp/views/ForgotPasswordView.tsxapp/views/LoginView/index.tsxapp/views/SendEmailConfirmationView.tsxapp/stacks/types.tsapp/views/NewServerView/index.tsxapp/views/RegisterView/index.tsx
app/stacks/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place navigation stacks in 'app/stacks/' directory (InsideStack for authenticated, OutsideStack for login/register, MasterDetailStack for tablets, ShareExtensionStack)
Files:
app/stacks/OutsideStack.tsxapp/stacks/types.ts
app/views/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Place screen components in 'app/views/' directory
Files:
app/views/AuthenticationWebView.tsxapp/views/ForgotPasswordView.tsxapp/views/LoginView/index.tsxapp/views/SendEmailConfirmationView.tsxapp/views/NewServerView/index.tsxapp/views/RegisterView/index.tsx
🧠 Learnings (3)
📚 Learning: 2026-04-30T17:07:51.020Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7274
File: app/lib/services/voip/MediaCallEvents.ts:0-0
Timestamp: 2026-04-30T17:07:51.020Z
Learning: In this Rocket.Chat React Native codebase, the ESLint rule `no-void: error` is enforced. When you see a promise returned from an async call that is not awaited (a “floating promise”), do not silence it with the `void somePromise()` pattern. Instead, handle the promise explicitly by attaching `.catch(...)` (or otherwise awaiting/handling the error) so unhandled-rejection risks are addressed in a way that satisfies the existing ESLint configuration.
Applied to files:
app/stacks/OutsideStack.tsxapp/views/AuthenticationWebView.tsxapp/views/ForgotPasswordView.tsxapp/views/LoginView/index.tsxapp/views/SendEmailConfirmationView.tsxapp/stacks/types.tsapp/views/NewServerView/index.tsxapp/views/RegisterView/index.tsx
📚 Learning: 2026-02-05T13:55:00.974Z
Learnt from: Rohit3523
Repo: RocketChat/Rocket.Chat.ReactNative PR: 6930
File: package.json:101-101
Timestamp: 2026-02-05T13:55:00.974Z
Learning: In this repository, the dependency on react-native-image-crop-picker should reference the RocketChat fork (RocketChat/react-native-image-crop-picker) with explicit commit pins, not the upstream ivpusic/react-native-image-crop-picker. Update package.json dependencies (and any lockfile) to point to the fork URL and a specific commit, ensuring edge-to-edge Android fixes are included. This pattern should apply to all package.json files in the repo that declare this dependency.
Applied to files:
package.json
📚 Learning: 2026-05-07T17:47:14.516Z
Learnt from: diegolmello
Repo: RocketChat/Rocket.Chat.ReactNative PR: 7303
File: package.json:5-5
Timestamp: 2026-05-07T17:47:14.516Z
Learning: When reviewing pnpm `packageManager` version pins in any `package.json` (e.g., `"packageManager": "pnpm@<version>"`), don’t rely solely on web-search results to determine whether a version exists. For very recently published versions, cross-check the target version against the official pnpm release page (https://github.com/pnpm/pnpm/releases) and the npm registry page for pnpm (https://www.npmjs.com/package/pnpm) before flagging the pinned version as non-existent.
Applied to files:
package.json
🔇 Additional comments (9)
package.json (1)
46-49: React Navigation package versions verified—all valid with no security vulnerabilities.All four React Navigation packages at the specified versions exist on npm and have no known security advisories:
@react-navigation/drawer7.12.2 ✓@react-navigation/elements2.9.25 ✓@react-navigation/native7.3.3 ✓@react-navigation/native-stack7.17.5 ✓app/stacks/OutsideStack.tsx (1)
2-3: LGTM!Also applies to: 26-53
app/stacks/types.ts (1)
302-302: LGTM!app/views/NewServerView/index.tsx (1)
6-7: LGTM!Also applies to: 26-31
app/views/LoginView/index.tsx (1)
2-2: LGTM!Also applies to: 12-12, 14-19
app/views/ForgotPasswordView.tsx (1)
3-3: LGTM!Also applies to: 28-28, 30-30, 40-40
app/views/SendEmailConfirmationView.tsx (1)
2-2: LGTM!Also applies to: 15-15, 17-17
app/views/RegisterView/index.tsx (1)
8-9: LGTM!Also applies to: 33-33, 35-36
app/views/AuthenticationWebView.tsx (1)
1-1: LGTM!Also applies to: 45-45, 47-47, 56-56
2 participants
Proposed changes
Migrates the OutsideStack (login flow) navigators from React Navigation's dynamic JSX API to the static config API, the first step of moving the app's navigation tree to static config.
OutsideStack/OutsideModalare now declared withcreateNativeStackNavigator({ screens })/createNativeStackScreen({ screen, options })..with(({ Navigator }) => …)wrapper that readsThemeContext; the staticdefaultHeaderstays in the config and React Navigation shallow-merges the two..getComponent()bridge lets the still-dynamic root embed the static navigator, so the root and the other stacks are untouched.StaticParamList<typeof Nav>, replacing the hand-writtenOutsideParamList/OutsideModalParamListinstacks/types.ts. Param-bearing screens type theirroutewithStaticScreenProps. The existing explicit-generic call-site idiom is kept.@react-navigation/*to the latest 7.x, required for the static-config.with()and.getComponent()APIs.No behavior change — same screens, same headers, same transitions.
Issue(s)
https://rocketchat.atlassian.net/browse/NATIVE-1301
How to test or reproduce
Walk the login flow and confirm navigation and headers are unchanged:
Verify each screen pushes/dismisses correctly and headers render with the current theme (light/dark/black).
Screenshots
Types of changes
Checklist
Further comments
The migration is incremental and leaf-first: the root navigator stays dynamic and embeds this static stack through
.getComponent(). That bridge is removed once the root itself becomes static in a later change. The theme is applied via.with()rather than the navigation-theme callback because the app palette (light/dark/black) is not part of the React Navigation theme object the headers need.Summary by CodeRabbit
Refactor
Chores