refactor(Example): TabsContainer and StackContainer refactor by sgaczol · Pull Request #3925 · software-mansion/react-native-screens

sgaczol · 2026-04-22T11:57:07Z

Description

Previously, we were storing the actual React Component references directly inside the useReducer state (within TabRoute and StackRoute objects). Because useReducer preserves its state and ignores updates to components after the first mount, even if Fast Refresh was triggered, it did not result in UI update.

We now strictly separate the navigation state from the Components. During the render phase, the containers dynamically resolve the correct Component by matching the route name from the state against a componentsByName Map (built via useMemo from the routeConfigs prop), rather than storing Component references in reducer state.

There was one more issue related to StackContainer: often in our environment, we wrap StackContainer in an intermediate component (e.g. StackSetup) to consume contexts such as useToast(). If that wrapper is not explicitly exported from its file, Fast Refresh causes useReducer inside StackContainer to re-fire its initialization function, resetting the stack to the first screen. To solve this, a custom ESLint rule local-rules/require-top-level-exports has been added — it enforces that all top-level declarations in test files are exported, preventing this class of Fast Refresh issues. The rule is implemented as a local ESLint plugin (eslint-plugin-local-rules) linked via file: dependency — no external packages needed. All existing test files have been updated to comply.

Changes

Update TabRoute and StackRoute types to omit Component from the route state
Update TabsContainer and StackContainer to resolve components via a componentsByName Map during render
Add safe-stringify utility for debug logging (handles circular refs, functions, and React elements)
Add local-rules/require-top-level-exports ESLint rule (applied to apps/src/tests/**)
Update all existing test files to export top-level declarations

Before & after - visual documentation

Before

TabsContainer	StackContainer
TabsContainerBefore.mov	StackContainerBefore.mov

After

TabsContainer	StackContainer
TabsContainerAfter.mov	StackContainerAfterWithExport.mov

Test plan

Check if Fast Refresh works properly in tests.

Checklist

Included code example that can be used to test this change.
For visual changes, included screenshots / GIFs / recordings documenting the change.
For API changes, updated relevant public types.
Ensured that CI passes

Copilot

Pull request overview

Refactors the Gamma TabsContainer and StackContainer to keep reducer/navigation state free of React component references, and instead resolve the latest Component from routeConfigs during render to improve Fast Refresh behavior.

Changes:

Update TabRoute / StackRoute runtime types to omit Component from reducer state.
Strip Component when creating routes in the tabs/stack reducers.
Resolve the correct Component per rendered route by matching route.name against routeConfigs.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
apps/src/shared/gamma/containers/tabs/reducer.tsx	Omits `Component` from tab route state when initializing routes.
apps/src/shared/gamma/containers/tabs/TabsContainerItem.types.ts	Adds `Component` prop to `TabsContainerItem` props.
apps/src/shared/gamma/containers/tabs/TabsContainerItem.tsx	Renders using the resolved `Component` prop instead of reading from route state.
apps/src/shared/gamma/containers/tabs/TabsContainer.types.tsx	Updates `TabRoute` type to omit `Component`.
apps/src/shared/gamma/containers/tabs/TabsContainer.tsx	Resolves `Component` from `routeConfigs` by route name at render time.
apps/src/shared/gamma/containers/stack/reducer.tsx	Omits `Component` from stack route state when creating routes.
apps/src/shared/gamma/containers/stack/StackContainer.types.tsx	Updates `StackRoute` type to omit `Component` (and adjusts field delimiter).
apps/src/shared/gamma/containers/stack/StackContainer.tsx	Resolves `Component` from `routeConfigs` by route name at render time.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-22T12:02:47Z

  pendingForUpdate: boolean;
+  Component: React.ComponentType;
 }


React.ComponentType is referenced here but this file doesn't import React, which will fail typechecking in this codebase (other files importing React when using the namespace). Import React as a type, or switch the prop type to an imported ComponentType/TabRouteConfig['Component'].

+1, import React from 'react';

done: 047e2d1

Copilot · 2026-04-22T12:02:48Z

  activityMode: StackScreenProps['activityMode'];
  routeKey: StackScreenProps['screenKey'];
-  isMarkedForDismissal: Boolean, // whether this route is during or after dismissal process
+  isMarkedForDismissal: Boolean; // whether this route is during or after dismissal process


This uses the boxed Boolean type. Prefer the primitive boolean here to avoid allowing new Boolean(false)-style truthy values and to match typical TS usage.

Suggested change

isMarkedForDismissal: Boolean; // whether this route is during or after dismissal process

isMarkedForDismissal: boolean; // whether this route is during or after dismissal process

hes right, treat is as a typo and fix it.

done: 047e2d1

Copilot · 2026-04-22T12:02:48Z

+        const matchingConfig = routeConfigs.find(
+          config => config.name === route.name,
+        );
+        if (!matchingConfig) {
+          throw new Error(
+            `[Tabs] No config matches the "${route.name}" route name`,
+          );
+        }
+
+        const Component = matchingConfig.Component;
+


routeConfigs.find(...) is executed once per route on every render, making this O(n²) in the number of tabs. Consider precomputing a Map/object lookup (e.g., configByName) with useMemo and doing O(1) lookups inside the .map.

Copilot · 2026-04-22T12:02:48Z

+        const matchingConfig = routeConfigs.find(
+          config => config.name === name,
+        );
+        if (!matchingConfig) {
+          throw new Error(`[Stack] No config matches the "${name}" route name`);
+        }
+
+        const Component = matchingConfig.Component;
+


routeConfigs.find(...) runs for every rendered stack screen on each render, which becomes O(n²) as the stack grows. Consider memoizing a name->config map and doing constant-time lookups here.

Copilot · 2026-04-22T12:02:49Z

+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  const { Component, ...rest } = config;


Avoid using eslint-disable to drop Component from the returned route. A small refactor can keep linting enabled (e.g., rename the destructured field to an underscore-prefixed identifier if that's allowed by the lint config, or explicitly consume it with void), while still omitting it from rest.

Suggested change

// eslint-disable-next-line @typescript-eslint/no-unused-vars

const { Component, ...rest } = config;

const { Component, ...rest } = config;

void Component;

Copilot · 2026-04-22T12:02:49Z

+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  const { Component, ...rest } = config;


Avoid the eslint-disable here; it’s better to omit Component without disabling lint (e.g., rename the destructured field to an ignored identifier per lint rules, or explicitly consume it) while still returning ...rest.

Suggested change

// eslint-disable-next-line @typescript-eslint/no-unused-vars

const { Component, ...rest } = config;

const { Component: _Component, ...rest } = config;

I'd leave it in the current form, because it's on the example app's side and we also did the same in other places.

t0maboro · 2026-04-22T14:05:29Z

For stack v5 tests, I'm observing that fast refresh is resetting the stack to the initial route. I'm on this commit: ced4f0c

stack-v5.mov

t0maboro · 2026-04-22T14:06:42Z

+  // eslint-disable-next-line @typescript-eslint/no-unused-vars
+  const { Component, ...rest } = config;


I'd leave it in the current form, because it's on the example app's side and we also did the same in other places.

Copilot

Pull request overview

Copilot reviewed 268 out of 269 changed files in this pull request and generated no new comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

kkafar

I haven't gone through each test one by one, but I've left few general remarks below. Please answer them.

kkafar · 2026-04-27T11:20:42Z

+  const componentsByName = React.useMemo(() => {
+    const map = new Map<string, StackRouteConfig['Component']>();
+    for (const config of routeConfigs) {
+      map.set(config.name, config.Component);
+    }
+    return map;
+  }, [routeConfigs]);


Move it to separate helper hook. Similarly to useSanitizeRouteConfigs - you can define it in the bottom of this file.

If this is reused between different containers (Stack & Tabs) -> then you can move ti to standalone file and share it.

+1 for generic helper

kkafar · 2026-04-27T11:22:04Z

  activityMode: StackScreenProps['activityMode'];
  routeKey: StackScreenProps['screenKey'];
-  isMarkedForDismissal: Boolean, // whether this route is during or after dismissal process
+  isMarkedForDismissal: Boolean; // whether this route is during or after dismissal process


hes right, treat is as a typo and fix it.

kkafar · 2026-04-27T11:24:00Z

+  const componentsByName = React.useMemo(() => {
+    const map = new Map<string, TabRouteConfig['Component']>();
+    for (const config of routeConfigs) {
+      map.set(config.name, config.Component);
+    }
+    return map;
+  }, [routeConfigs]);


Yep, it is shared. Move it to standalone file and share it between implementations if possible. If you'd have to do some weird things to satisfy type constraints -> then make a hook per container, but definitively move it to a hook.

kkafar · 2026-04-27T11:26:30Z

  pendingForUpdate: boolean;
+  Component: React.ComponentType;
 }


+1, import React from 'react';

kkafar · 2026-04-27T11:30:22Z

 }

-const STACK_ROUTE_CONFIGS: StackRouteConfig[] = [
+export const STACK_ROUTE_CONFIGS: StackRouteConfig[] = [


Do we need to export that, or only components?

I'm aware that we said to literally export everything during the meeting, but realising now that I had hidden assumption and thought only of components.

Ok, I think that exporting components is all we need there, so I'll change it

kkafar · 2026-04-27T11:32:03Z

Do we need to apply these exports rules also to places where we had no problems? What do you think?

cc @t0maboro?

if examples touching stack v4 are working fine, we should consider applying rules only to specific directories

It makes sense, we need to refactor issue-tests then, so tests regarding different components are in different directories to implement such approach. However I think that such change should be added in a separate PR and if so, an addition of a new ESLint rule should be moved to that PR. What do you think?

kkafar · 2026-04-27T11:34:08Z

It would be nice to not make top-level directory out of it. Is it possible to put it inside apps?

explicitly disabling in top-level .eslintrc and overriding that rule in the nested directory .eslintrc should work, we may consider adding it deeper, even for SFTs and CITs only, because issue tests won't be actively updated once written. wdyt?

The problem is that issue tests regarding Tabs or Stackv5 created in the future may seem to be broken, if we don't apply that rule to at least some of the issue tests. We could divide files in issue-tests into a few directories and apply the rule only to some of them.

now, instead of keeping a Component in state, we find one in routeConfigs prop, basing on routeKey

also small change in TabsContainer

Copilot

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot

Pull request overview

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

+        const Component = componentsByName.get(route.name);
+        if (!Component) {
+          throw new Error(
+            `[Tabs] None config matches the "${route.name}" route name`,


kligarski

I haven't checked the runtime but it looks good.

Remember to update the PR description.

sgaczol requested review from Copilot, kkafar, kligarski, kmichalikk and t0maboro April 22, 2026 11:57

Copilot started reviewing on behalf of sgaczol April 22, 2026 11:57 View session

Copilot AI reviewed Apr 22, 2026

View reviewed changes

sgaczol force-pushed the @sgaczol/containers-refactor branch from 881b3f0 to ced4f0c Compare April 22, 2026 12:03

t0maboro reviewed Apr 22, 2026

View reviewed changes

sgaczol requested review from Copilot and t0maboro April 27, 2026 09:43

Copilot started reviewing on behalf of sgaczol April 27, 2026 09:43 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

kkafar reviewed Apr 27, 2026

View reviewed changes

sgaczol added 3 commits April 27, 2026 13:41

TabsContainer refactor

e0bb90f

now, instead of keeping a Component in state, we find one in routeConfigs prop, basing on routeKey

StackContainer refactor analagous to TabsContainer

1d462c0

also small change in TabsContainer

componentsByName added

9d9912e

sgaczol force-pushed the @sgaczol/containers-refactor branch from b73fe14 to 48b37b7 Compare April 27, 2026 11:55

sgaczol requested a review from Copilot April 27, 2026 12:29

Copilot started reviewing on behalf of sgaczol April 27, 2026 12:29 View session

Copilot AI reviewed Apr 27, 2026

View reviewed changes

types update, StackContainer.types.tsx extension update to .ts

0cca1e0

sgaczol force-pushed the @sgaczol/containers-refactor branch 2 times, most recently from 4990b5c to 3ba527a Compare April 27, 2026 18:09

add useComponentsByName custom hook, shared among both containers

88a0d86

sgaczol force-pushed the @sgaczol/containers-refactor branch from 3ba527a to 88a0d86 Compare April 27, 2026 18:23

sgaczol requested review from Copilot and kkafar April 30, 2026 11:10

Copilot started reviewing on behalf of sgaczol April 30, 2026 11:11 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

Comment thread apps/src/shared/gamma/containers/tabs/TabsContainer.tsx

const Component = componentsByName.get(route.name);

if (!Component) {

throw new Error(

`[Tabs] None config matches the "${route.name}" route name`,

kligarski approved these changes Apr 30, 2026

View reviewed changes

	isMarkedForDismissal: Boolean; // whether this route is during or after dismissal process
	isMarkedForDismissal: boolean; // whether this route is during or after dismissal process

		// eslint-disable-next-line @typescript-eslint/no-unused-vars
		const { Component, ...rest } = config;

	// eslint-disable-next-line @typescript-eslint/no-unused-vars
	const { Component, ...rest } = config;
	const { Component: _Component, ...rest } = config;

Uh oh!

Conversation

sgaczol commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Changes

Before & after - visual documentation

Before

After

Test plan

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 22, 2026

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t0maboro commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

kkafar left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

t0maboro Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

sgaczol Apr 27, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

sgaczol commented Apr 22, 2026 •

edited

Loading

t0maboro commented Apr 22, 2026 •

edited

Loading

t0maboro Apr 27, 2026 •

edited

Loading

sgaczol Apr 27, 2026 •

edited

Loading

t0maboro Apr 27, 2026 •

edited

Loading