Skip to content

fix(core): rebuild nested-frame fragment when its view was destroyed on reattach #11214

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
dangrima90 wants to merge 1 commit into
base: main
Choose a base branch
from
+28 −3
Open

fix(core): rebuild nested-frame fragment when its view was destroyed on reattach #11214

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions packages/core/ui/frame/frame-helper-for-android.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -236,8 +236,15 @@ export class FragmentCallbacksImplementation implements AndroidFragmentCallbacks
// [nested frames / fragments] see https://github.com/NativeScript/NativeScript/issues/6629
// retaining reference to a destroyed fragment here somehow causes a cryptic
// "IllegalStateException: Failure saving state: active fragment has cleared index: -1"
// in a specific mixed parent / nested frame navigation scenario
entry.fragment = null;
// in a specific mixed parent / nested frame navigation scenario.
//
// Only null out entry.fragment if it still points to THIS fragment. _ensureEntryFragment
// may have already replaced it with a new fragment (when recovering one whose view was
// destroyed) — if THIS old fragment is destroyed afterwards, we must not overwrite the
// newer reference.
if (entry.fragment === fragment) {
entry.fragment = null;
}

const page = entry.resolvedPage;
if (!page) {
Expand Down
20 changes: 19 additions & 1 deletion packages/core/ui/frame/index.android.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,12 @@ export class Frame extends FrameBase {
private _cachedTransitionState: TransitionState;
private _frameCreateTimeout: NodeJS.Timeout;

// True while inside an onLoaded()-driven _ensureEntryFragment call. _ensureEntryFragment
// also runs from _onAttachedToWindow during outgoing navigations, where the fragment's
// view is null because it's being torn down on purpose — we must not rebuild it then.
// This flag lets _ensureEntryFragment tell the two cases apart.
private _pendingFragmentCheck = false;

Comment on lines +87 to +92
Copy link
Copy Markdown

@coderabbitai coderabbitai Bot May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major | ⚡ Quick win

Capture the rebuild-origin flag before scheduling the timeout.

_pendingFragmentCheck is read inside the delayed callback, but it can be changed by later lifecycle calls before that callback runs. That can incorrectly enable/disable fragment rebuilds across overlapping _ensureEntryFragment() invocations.

Proposed fix 
 onUnloaded() {
 	super.onUnloaded();
+	this._pendingFragmentCheck = false;

 	if (typeof this._frameCreateTimeout === 'number') {
 		clearTimeout(this._frameCreateTimeout);
 		this._frameCreateTimeout = null;
 	}
 }

 private _ensureEntryFragment(): void {
+	const shouldRebuildMissingView = this._pendingFragmentCheck;
+	this._pendingFragmentCheck = false;
+
 	// in case the activity is "reset" using resetRootView or disposed we must wait for
 	// the attachedToWindow event to make the first navigation or it will crash
 	// https://github.com/NativeScript/NativeScript/commit/9dd3e1a8076e5022e411f2f2eeba34aabc68d112
 	// though we should not do it on app "start"
 	// or it will create a "flash" to activity background color
 	if (this._isReset && !this._attachedToWindow) {
-		this._pendingFragmentCheck = false;
 		return;
 	}

 	this._frameCreateTimeout = setTimeout(() => {
 		// there's a bug with nested frames where sometimes the nested fragment is not recreated at all
 		// so we manually check on loaded event if the fragment is not recreated and recreate it
 		const currentEntry = this._currentEntry || this._executingContext?.entry;
 		if (currentEntry) {
 			const fragmentNeedsRebuild =
-				!currentEntry.fragment || (currentEntry.fragment.getView() == null && this._pendingFragmentCheck);
+				!currentEntry.fragment || (currentEntry.fragment.getView() == null && shouldRebuildMissingView);
 			if (fragmentNeedsRebuild) {
 				const manager = this._getFragmentManager();
 				const transaction = manager.beginTransaction();
 				currentEntry.fragment = this.createFragment(currentEntry, currentEntry.fragmentTag);
 				_updateTransitions(currentEntry);
 				transaction.replace(this.containerViewId, currentEntry.fragment, currentEntry.fragmentTag);
 				transaction.commitAllowingStateLoss();
 			}
 		}

-		this._pendingFragmentCheck = false;
 		this._frameCreateTimeout = null;
 	}, 0);
 }

Also applies to: 254-257, 287-288, 291-313

🤖 Prompt for 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.

In `@packages/core/ui/frame/index.android.ts` around lines 87 - 92, The delayed
callback in _ensureEntryFragment reads the instance field _pendingFragmentCheck
which may change before the timeout fires; capture the current value into a
local constant (e.g., const pendingAtSchedule = this._pendingFragmentCheck)
immediately before scheduling the timeout and use that local within the timeout
to decide whether to rebuild the fragment; update every similar scheduling site
(the calls around lines with _ensureEntryFragment/_onAttachedToWindow handling
and the other occurrences you noted) to use the captured local instead of
reading this._pendingFragmentCheck inside the async callback.

constructor() {
super();
this._android = new AndroidFrame(this);
Expand Down Expand Up @@ -245,6 +251,10 @@ export class Frame extends FrameBase {
}

onLoaded(): void {
// Tell _ensureEntryFragment this pass is from onLoaded (an incoming reattach)
// rather than from _onAttachedToWindow during an outgoing transition. See the
// field declaration of _pendingFragmentCheck for why this matters.
this._pendingFragmentCheck = true;
if (this._originalBackground) {
this.backgroundColor = null;
this.backgroundColor = this._originalBackground;
Expand Down Expand Up @@ -274,6 +284,7 @@ export class Frame extends FrameBase {
// though we should not do it on app "start"
// or it will create a "flash" to activity background color
if (this._isReset && !this._attachedToWindow) {
this._pendingFragmentCheck = false;
return;
}

Expand All @@ -282,7 +293,13 @@ export class Frame extends FrameBase {
// so we manually check on loaded event if the fragment is not recreated and recreate it
const currentEntry = this._currentEntry || this._executingContext?.entry;
if (currentEntry) {
if (!currentEntry.fragment) {
// Rebuild if either:
// (a) there is no fragment at all, or
// (b) the fragment exists but its view was destroyed AND we got here via
// onLoaded (not via _onAttachedToWindow during an outgoing transition,
// where a null view is expected and a rebuild would be wrong).
const fragmentNeedsRebuild = !currentEntry.fragment || (currentEntry.fragment.getView() == null && this._pendingFragmentCheck);
if (fragmentNeedsRebuild) {
const manager = this._getFragmentManager();
const transaction = manager.beginTransaction();
currentEntry.fragment = this.createFragment(currentEntry, currentEntry.fragmentTag);
Expand All @@ -292,6 +309,7 @@ export class Frame extends FrameBase {
}
}

this._pendingFragmentCheck = false;
this._frameCreateTimeout = null;
}, 0);
}
Expand Down
Loading