Page MenuHomePhabricator
Differential D5538

[native] Update @react-navigation/drawer libdef for React Nav 6
ClosedPublic
Actions

Authored by ashoat on Nov 4 2022, 10:20 AM.
Tags
None
Referenced Files
F5252626: D5538.diff
Sun, Apr 6, 5:21 AM
Unknown Object (File)
Sun, Mar 30, 11:06 AM
Unknown Object (File)
Mar 3 2025, 1:46 AM
Unknown Object (File)
Mar 3 2025, 1:46 AM
Unknown Object (File)
Feb 25 2025, 10:57 PM
Unknown Object (File)
Feb 12 2025, 11:39 AM
Unknown Object (File)
Feb 12 2025, 9:38 AM
Unknown Object (File)
Feb 7 2025, 11:10 PM
View All Files
Subscribers
• abosh

Details

Reviewers
atul
inka
tomek
Commits
rCOMM7a442aeae8b8: [native] Update @react-navigation/drawer libdef for React Nav 6
Summary

See D5537 and ENG-2175 for context. Will describe changes and where they came from inline.

Depends on D5537

Test Plan

Flow

Diff Detail

Repository
rCOMM Comm
Branch
ashoat/react-navigation-drawer
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat created this revision.Nov 4 2022, 10:20 AM
Herald added a subscriber: • abosh. · View Herald TranscriptNov 4 2022, 10:20 AM
Harbormaster returned this revision to the author for changes because remote builds failed.Nov 4 2022, 10:29 AM
Harbormaster failed remote builds in B13184: Diff 18090!
ashoat updated this revision to Diff 18091.Nov 4 2022, 10:34 AM

Add headerShown

ashoat added a comment.Nov 4 2022, 10:38 AM

I referenced the TypeScript types when writing this diff.

You only need to review the drawer libdef, as the others are copy-pasted from there.

native/flow-typed/npm/@react-navigation/drawer_v6.x.x.js
745

This was changed in React Nav 6

1974–2004
  1. Some of these are new options migrated from drawerContentOptions in React Nav 6
  2. Some of these got their name changed at some point prior to React Nav 6. I found the new names via API docs and TypeScript types
1994–1995

These were removed in React Nav 6

2005

New property here in React Nav 6, details here

2045–2058

These were migrated to options in React Nav 6

2069

React Nav 6 removes this

2069

TypeScript types only have these three props. Upgrade notes only mention removing one prop, so the others were probably removed earlier and the libdef is probably just out of date

2071

New property here in React Nav 6, details here

2073–2086

Most of these props were moved to options at some point – you can see them in the TypeScript types. React Nav 6 upgrade docs don't mention this, so presumably the change occurred earlier

2082

React Nav 6 moves this to options

2297

This was changed in React Nav 6

Harbormaster returned this revision to the author for changes because remote builds failed.Nov 4 2022, 10:42 AM
Harbormaster failed remote builds in B13185: Diff 18091!
ashoat requested review of this revision.Nov 5 2022, 6:10 AM

Android CI is broken right now, so bypassing it

tomek accepted this revision.Nov 7 2022, 2:08 AM
atul accepted this revision.Nov 7 2022, 7:02 AM

Thanks for annotating the relevant changes

This revision is now accepted and ready to land.Nov 7 2022, 7:02 AM
ashoat updated this revision to Diff 18137.Nov 7 2022, 7:27 AM

Rebase

Harbormaster completed remote builds in B13217: Diff 18137.Nov 7 2022, 7:40 AM
Closed by commit rCOMM7a442aeae8b8: [native] Update @react-navigation/drawer libdef for React Nav 6 (authored by ashoat). · Explain WhyNov 7 2022, 8:04 AM
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMM7a442aeae8b8: [native] Update @react-navigation/drawer libdef for React Nav 6.

Revision Contents
Changeset List

PathSize
native/
flow-typed/
npm/
@react-navigation/
78 lines
78 lines
92 lines
78 lines
78 lines
78 lines
78 lines

Diff 18091

View Options

native/flow-typed/npm/@react-navigation/bottom-tabs_v6.x.x.js

Show First 20 Lines • Show All 396 Lines • ▼ Show 20 Lines async ensureAndroidPushNotifsEnabled() {
let [hasPermission] = permissionPromisesResult; let [hasPermission] = permissionPromisesResult;
const [, canRequestPermission] = permissionPromisesResult; const [, canRequestPermission] = permissionPromisesResult;
if (!hasPermission && canRequestPermission) { if (!hasPermission && canRequestPermission) {
const permissionResponse = await (async () => { const permissionResponse = await (async () => {
// We issue a call to sleep to match iOS behavior where prompt // We issue a call to sleep to match iOS behavior where prompt
// doesn't appear immediately but after logged-out modal disappears // doesn't appear immediately but after logged-out modal disappears
await sleep(10); await sleep(10);
await this.requestAndroidNotificationsPermission(); return await this.requestAndroidNotificationsPermission();
marcinUnsubmitted
Not Done
Inline Actions

We definitely should return something from this lambda since otherwise it is useless to assign it to variable.

When we find we don't have Android notif permissions, we ignore the result of requestAndroidNotificationsPermission. It seems like that would cause us to fail to register notif permissions on Android.

If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.

This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.

marcin: We definitely should return something from this lambda since otherwise it is useless to assign…
})(); })();
hasPermission = permissionResponse; hasPermission = permissionResponse;
} }
if (!hasPermission) { if (!hasPermission) {
this.failedToRegisterPushPermissionsAndroid(!canRequestPermission); this.failedToRegisterPushPermissionsAndroid(!canRequestPermission);
return; return;
} }
▲ Show 20 Lines • Show All 328 Lines • Show Last 20 Lines
View Options

native/flow-typed/npm/@react-navigation/devtools_v6.x.x.js

Show First 20 Lines • Show All 396 Lines • ▼ Show 20 Lines async ensureAndroidPushNotifsEnabled() {
let [hasPermission] = permissionPromisesResult; let [hasPermission] = permissionPromisesResult;
const [, canRequestPermission] = permissionPromisesResult; const [, canRequestPermission] = permissionPromisesResult;
if (!hasPermission && canRequestPermission) { if (!hasPermission && canRequestPermission) {
const permissionResponse = await (async () => { const permissionResponse = await (async () => {
// We issue a call to sleep to match iOS behavior where prompt // We issue a call to sleep to match iOS behavior where prompt
// doesn't appear immediately but after logged-out modal disappears // doesn't appear immediately but after logged-out modal disappears
await sleep(10); await sleep(10);
await this.requestAndroidNotificationsPermission(); return await this.requestAndroidNotificationsPermission();
marcinUnsubmitted
Not Done
Inline Actions

We definitely should return something from this lambda since otherwise it is useless to assign it to variable.

When we find we don't have Android notif permissions, we ignore the result of requestAndroidNotificationsPermission. It seems like that would cause us to fail to register notif permissions on Android.

If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.

This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.

marcin: We definitely should return something from this lambda since otherwise it is useless to assign…
})(); })();
hasPermission = permissionResponse; hasPermission = permissionResponse;
} }
if (!hasPermission) { if (!hasPermission) {
this.failedToRegisterPushPermissionsAndroid(!canRequestPermission); this.failedToRegisterPushPermissionsAndroid(!canRequestPermission);
return; return;
} }
▲ Show 20 Lines • Show All 328 Lines • Show Last 20 Lines
View Options

native/flow-typed/npm/@react-navigation/drawer_v6.x.x.js

Show First 20 Lines • Show All 396 Lines • ▼ Show 20 Lines async ensureAndroidPushNotifsEnabled() {
let [hasPermission] = permissionPromisesResult; let [hasPermission] = permissionPromisesResult;
const [, canRequestPermission] = permissionPromisesResult; const [, canRequestPermission] = permissionPromisesResult;
if (!hasPermission && canRequestPermission) { if (!hasPermission && canRequestPermission) {
const permissionResponse = await (async () => { const permissionResponse = await (async () => {
// We issue a call to sleep to match iOS behavior where prompt // We issue a call to sleep to match iOS behavior where prompt
// doesn't appear immediately but after logged-out modal disappears // doesn't appear immediately but after logged-out modal disappears
await sleep(10); await sleep(10);
await this.requestAndroidNotificationsPermission(); return await this.requestAndroidNotificationsPermission();
marcinUnsubmitted
Not Done
Inline Actions

We definitely should return something from this lambda since otherwise it is useless to assign it to variable.

When we find we don't have Android notif permissions, we ignore the result of requestAndroidNotificationsPermission. It seems like that would cause us to fail to register notif permissions on Android.

If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.

This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.

marcin: We definitely should return something from this lambda since otherwise it is useless to assign…
})(); })();
hasPermission = permissionResponse; hasPermission = permissionResponse;
} }
if (!hasPermission) { if (!hasPermission) {
this.failedToRegisterPushPermissionsAndroid(!canRequestPermission); this.failedToRegisterPushPermissionsAndroid(!canRequestPermission);
return; return;
} }
▲ Show 20 Lines • Show All 328 Lines • Show Last 20 Lines
View Options

native/flow-typed/npm/@react-navigation/elements_v1.x.x.js

Show First 20 Lines • Show All 396 Lines • ▼ Show 20 Lines async ensureAndroidPushNotifsEnabled() {
let [hasPermission] = permissionPromisesResult; let [hasPermission] = permissionPromisesResult;
const [, canRequestPermission] = permissionPromisesResult; const [, canRequestPermission] = permissionPromisesResult;
if (!hasPermission && canRequestPermission) { if (!hasPermission && canRequestPermission) {
const permissionResponse = await (async () => { const permissionResponse = await (async () => {
// We issue a call to sleep to match iOS behavior where prompt // We issue a call to sleep to match iOS behavior where prompt
// doesn't appear immediately but after logged-out modal disappears // doesn't appear immediately but after logged-out modal disappears
await sleep(10); await sleep(10);
await this.requestAndroidNotificationsPermission(); return await this.requestAndroidNotificationsPermission();
marcinUnsubmitted
Not Done
Inline Actions

We definitely should return something from this lambda since otherwise it is useless to assign it to variable.

When we find we don't have Android notif permissions, we ignore the result of requestAndroidNotificationsPermission. It seems like that would cause us to fail to register notif permissions on Android.

If we find that we don't have permissions then just calling await this.requestAndroidNotificationsPermission(); would result in a prompt asking for notifications permissions. However if the user grants those permissions then hasPermissions is still falsy (since promise returned nothing), so deviceToken will be set to null. Nevertheless permissions are actually granted byt the OS, so the state on the device and keyserver would heal itself on next render.

This differential fixes this case so that if user grants permissions correct state is achieved immediately without need for additional re-render to heal the state.

marcin: We definitely should return something from this lambda since otherwise it is useless to assign…
})(); })();
hasPermission = permissionResponse; hasPermission = permissionResponse;
} }
if (!hasPermission) { if (!hasPermission) {
this.failedToRegisterPushPermissionsAndroid(!canRequestPermission); this.failedToRegisterPushPermissionsAndroid(!canRequestPermission);
return; return;
} }
▲ Show 20 Lines • Show All 328 Lines • Show Last 20 Lines
View Options

native/flow-typed/npm/@react-navigation/material-top-tabs_v6.x.x.js

Loading...
View Options

native/flow-typed/npm/@react-navigation/native_v6.x.x.js

Loading...
View Options

native/flow-typed/npm/@react-navigation/stack_v6.x.x.js

Loading...