Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
8 changes: 8 additions & 0 deletions patches/react-native/details.md
Original file line number Diff line number Diff line change
Expand Up @@ -322,3 +322,11 @@
- E/App issue: 🛑 — backport of an upstream fix, no separate E/App issue was filed.
- PR introducing patch: https://github.com/Expensify/App/pull/98507
- 0.86.0 migration note: **drop this patch with the RN 0.87 upgrade** — upstream commit `45904c8` is absent from every 0.86.x release but ships in `v0.87.0`, and the patch will not apply against it. Two deviations from the upstream commit: the `scripts/cxx-api/api-snapshots/*.api` hunks are omitted (those files are not shipped in the npm package), and `fontSizeMultiplier` is declared *last* in `LayoutMetrics` rather than after `pointScaleFactor`, because `@rnmapbox/maps` initializes that struct positionally and inserting a field mid-struct breaks its iOS build.

### [react-native+0.86.0+038+log-soft-exception-if-viewState-not-found.patch](react-native+0.86.0+038+log-soft-exception-if-viewState-not-found.patch)

- Reason: Restores the Android `updateOverflowInset` half of the dropped `react-native+0.85.3+025+log-soft-exception-if-viewState-not-found.patch`. `SurfaceMountingManager.updateOverflowInset` still resolves its tag through the throwing `getViewState`, so an `INSTRUCTION_UPDATE_OVERFLOW_INSET` op for a view that was already unmounted throws `RetryableMountingLayerException` from inside `IntBufferBatchMountItem.execute`. `MountItemDispatcher.dispatchMountItems` only retries `DispatchCommandMountItem`s, and `RetryableMountingLayerException` is not a `ReactIgnorableMountingException`, so the exception is rethrown and every remaining instruction in that mount transaction is dropped — the incoming views are created but never added or laid out, leaving a blank screen. This patch resolves the tag with `getNullableViewState` and soft-logs + returns instead, matching what upstream already does for `addViewAt`, `updateProps` and `updateLayout`.
- Upstream PR/issue: [#49077](https://github.com/facebook/react-native/issues/49077) [#56762](https://github.com/facebook/react-native/pull/56762) [#7493](https://github.com/software-mansion/react-native-reanimated/issues/7493)
- E/App issues: [#82611](https://github.com/Expensify/App/issues/82611) [#93833](https://github.com/Expensify/App/issues/93833)
- PR introducing patch: [#84303](https://github.com/Expensify/App/pull/84303) (original 0.85.3 patch)
- 0.86.0 migration note: RN 0.86.0 upstreamed the `getNullableViewState` + soft-log guard for `addViewAt`, `updateProps`, `updateLayout` and `removeViewAt`, which is why the 0.85.3 patch was dropped during the upgrade — but it did **not** upstream the `updateOverflowInset` guard, so that one site regressed. Only that site is re-patched here; `updatePadding` and `updateState` still use the throwing `getViewState`, matching 0.85.3 behaviour. Re-check on the RN 0.87 upgrade whether `updateOverflowInset` has been guarded upstream, and drop this patch if so.
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
diff --git a/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.kt b/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.kt
--- a/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.kt

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Okay, after checking, I see that this is expected.

+++ b/node_modules/react-native/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/SurfaceMountingManager.kt
@@ -899,9 +899,18 @@ internal constructor(
if (isStopped) {
return
}

- val viewState = getViewState(reactTag)
+ val viewState = getNullableViewState(reactTag)
+ if (viewState == null) {
+ ReactSoftExceptionLogger.logSoftException(
+ ReactSoftExceptionLogger.Categories.SURFACE_MOUNTING_MANAGER_MISSING_VIEWSTATE,
+ ReactNoCrashSoftException(
+ "Unable to find viewState for tag $reactTag for updateOverflowInset"
+ ),
+ )
+ return
+ }
// Do not layout Root Views
if (viewState.isRoot) {
return
}
24 changes: 20 additions & 4 deletions src/hooks/useIsAgentAccount.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,16 +9,32 @@ import useOnyx from './useOnyx';

function useIsAgentAccount(): boolean | undefined {
const accountID = useCurrentUserPersonalDetails().accountID;
const [isCustomAgent, personalDetailsMetadata] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {
selector: (personalDetails: OnyxEntry<PersonalDetailsList>) => (accountID ? personalDetails?.[accountID]?.isCustomAgent : undefined),
const [personalDetail, personalDetailsMetadata] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🟡 Selector now subscribes to the whole personalDetail object, not just the field it needs

const [personalDetail, personalDetailsMetadata] = useOnyx(ONYXKEYS.PERSONAL_DETAILS_LIST, {
    selector: (personalDetails: OnyxEntry<PersonalDetailsList>) => (accountID ? personalDetails?.[accountID] : undefined),
});

useOnyx's selector caching compares outputs with fast-equals' deepEqual, not reference equality (useOnyx.js:70): if (!hasComputed || !deepEqual(lastOutput, newOutput) || dependenciesChanged). The old selector returned just isCustomAgent (a primitive), so the hook only recomputed when that one field changed. The new selector returns the entire personal-details record for the current user, so any field on it — avatar, displayName, status, timezone, pronouns, etc. — now causes deepEqual to see a different object and re-trigger every consumer of useIsAgentAccount().

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I’m not sure this is a required part of the change to fix the issue. Could you please confirm?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

@jakubstec would be great to follow up on this one it does feel unrelated

selector: (personalDetails: OnyxEntry<PersonalDetailsList>) => (accountID ? personalDetails?.[accountID] : undefined),
});
const [isLoadingApp, isLoadingAppMetadata] = useOnyx(ONYXKEYS.IS_LOADING_APP);
const [hasLoadedApp, hasLoadedAppMetadata] = useOnyx(ONYXKEYS.HAS_LOADED_APP);

if (isLoadingApp === true || isLoadingOnyxValue(personalDetailsMetadata, isLoadingAppMetadata)) {
if (isLoadingOnyxValue(personalDetailsMetadata, isLoadingAppMetadata, hasLoadedAppMetadata)) {
return undefined;
}

return !!isCustomAgent;
// Identity is unknown while a load is in flight AND we can't yet trust what we have. Two loads can leave us
// without a trustworthy value:
// - Cold start: HAS_LOADED_APP hasn't flipped true yet, so even though sign-in may have already merged a
// partial personal-details entry (login, name, ...), the isCustomAgent field itself is still in flight -
// its absence isn't meaningful yet.
// - Delegate/account switch: personal details are wiped (unlike HAS_LOADED_APP, which Delegate's atomic reset
// deliberately preserves - see KEYS_TO_PRESERVE_DELEGATE_ACCESS), so a stale HAS_LOADED_APP=true must not be
// trusted while the entry is missing.
// Once both HAS_LOADED_APP is true and a personal-details entry exists, a later OpenApp or ReconnectApp
// setting IS_LOADING_APP back to true won't hide the screen again, because the identity we already have for
// this account is still valid.
if (isLoadingApp !== false && (!hasLoadedApp || personalDetail === undefined)) {
return undefined;
}

return !!personalDetail?.isCustomAgent;
}

export default useIsAgentAccount;
25 changes: 25 additions & 0 deletions tests/unit/withAgentAccessDenied.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -181,4 +181,29 @@ describe('withAgentAccessDenied', () => {
expect(screen.getByTestId('protected-content')).toBeDefined();
});
});

it('keeps rendering the wrapped component when a mid-session OpenApp sets isLoadingApp back to true', async () => {
// enabling 2FA runs OpenApp again while
// the user is already deep in the app. Agent identity is known by then, so the guarded screen must stay
// mounted instead of blanking out for the length of that request.
await TestHelper.signInWithTestUser(1, 'user@expensify.com');
await Onyx.multiSet({
[ONYXKEYS.IS_LOADING_APP]: false,
[ONYXKEYS.HAS_LOADED_APP]: true,
});
await waitForBatchedUpdatesWithAct();

renderComponent();
await waitForBatchedUpdatesWithAct();

expect(screen.getByTestId('protected-content')).toBeDefined();

await act(async () => {
await Onyx.set(ONYXKEYS.IS_LOADING_APP, true);
});
await waitForBatchedUpdatesWithAct();

expect(screen.getByTestId('protected-content')).toBeDefined();
expect(Navigation.navigate).not.toHaveBeenCalled();
});
});
Loading