Page MenuHomePhabricator
Differential D5924

[native] [29/40] RN 0.70: Patch react-native-firebase to avoid importing ViewPropTypes
ClosedPublic
Actions

Authored by ashoat on Dec 18 2022, 8:00 PM.
Tags
None
Referenced Files
F6280150: D5924.id19869.diff
Fri, Apr 25, 7:09 PM
Unknown Object (File)
Thu, Apr 24, 7:20 AM
Unknown Object (File)
Wed, Apr 23, 2:04 PM
Unknown Object (File)
Wed, Apr 23, 11:38 AM
Unknown Object (File)
Sun, Apr 20, 11:18 AM
Unknown Object (File)
Sat, Apr 19, 3:18 PM
Unknown Object (File)
Sat, Apr 19, 2:37 PM
Unknown Object (File)
Sat, Apr 19, 1:53 PM
View All Files
Subscribers
atul

Details

Reviewers
marcin
tomek
bartek
Commits
rCOMM815777e12d23: [native] [29/40] RN 0.70: Patch react-native-firebase to avoid importing…
Summary

The new version of React Native finally removed this export. We're using an ancient version of react-native-firebase, context on why here.

The right long-term solution is to migrate all of our Android notifs logic to be using our own modules, but in order to unblock the React Native update I'm just patching this now.

Depends on D5923

Test Plan
  1. Confirm JS error due to ViewPropTypes import no longer occurs
  2. Tested along with whole stack: test plan (includes testing notifs on Android)

Diff Detail

Repository
rCOMM Comm
Lint
No Lint Coverage
Unit
No Test Coverage

Event Timeline

ashoat created this revision.Dec 18 2022, 8:00 PM
Herald added a subscriber: atul. · View Herald TranscriptDec 18 2022, 8:00 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Dec 18 2022, 8:01 PM
Harbormaster failed remote builds in B14253: Diff 19492!
ashoat added a child revision: D5925: [native] [30/40] RN 0.70: Patch react-native-firebase to avoid Android NativeEventEmitter warning.Dec 18 2022, 8:02 PM
ashoat retitled this revision from [native] [28/40] RN 0.70: Patch react-native-firebase to avoid importing ViewPropTypes to [native] [29/40] RN 0.70: Patch react-native-firebase to avoid importing ViewPropTypes.Dec 18 2022, 8:06 PM
ashoat updated this revision to Diff 19513.Dec 18 2022, 8:20 PM

Rebase

ashoat updated this revision to Diff 19540.Dec 18 2022, 8:33 PM

Rebase

Harbormaster failed remote builds in B14274: Diff 19513!Dec 18 2022, 8:34 PM
Harbormaster returned this revision to the author for changes because remote builds failed.Dec 18 2022, 9:35 PM
Harbormaster failed remote builds in B14301: Diff 19540!
ashoat updated this revision to Diff 19602.Dec 19 2022, 8:22 AM

Rebase

ashoat updated this revision to Diff 19635.Dec 19 2022, 8:38 AM

Rebase

Harbormaster failed remote builds in B14353: Diff 19602!Dec 19 2022, 9:08 AM
ashoat published this revision for review.Dec 19 2022, 9:11 AM

Please ignore CI until the end of the stack

Harbormaster failed remote builds in B14386: Diff 19635!Dec 19 2022, 10:10 AM
ashoat updated this revision to Diff 19708.Dec 19 2022, 12:30 PM

Rebase

Harbormaster completed remote builds in B14453: Diff 19708.Dec 19 2022, 12:52 PM
tomek accepted this revision.Dec 20 2022, 5:29 AM
This revision is now accepted and ready to land.Dec 20 2022, 5:29 AM
ashoat updated this revision to Diff 19766.Dec 20 2022, 6:22 AM

Rebase

Harbormaster failed remote builds in B14509: Diff 19766!Dec 20 2022, 6:27 AM
ashoat updated this revision to Diff 19828.Dec 20 2022, 11:02 AM

Rebase

Harbormaster completed remote builds in B14568: Diff 19828.Dec 20 2022, 11:42 AM
Closed by commit rCOMM815777e12d23: [native] [29/40] RN 0.70: Patch react-native-firebase to avoid importing… (authored by ashoat). · Explain WhyDec 20 2022, 12:01 PM
This revision was automatically updated to reflect the committed changes.
ashoat added a commit: rCOMM815777e12d23: [native] [29/40] RN 0.70: Patch react-native-firebase to avoid importing….

Revision Contents
Changeset List

PathSize
patches/
83 lines

Diff 19708

View Options

patches/react-native-firebase+5.6.0.patch

// @flow // @flow
import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js'; import { DATABASE_WORKER_PATH, SQLJS_FILE_PATH } from './utils/constants.js';
import { isSQLiteSupported } from './utils/db-utils.js'; import { isDesktopSafari, isSQLiteSupported } from './utils/db-utils.js';
import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js'; import WorkerConnectionProxy from './utils/WorkerConnectionProxy.js';
import type { AppState } from '../redux/redux-setup.js'; import type { AppState } from '../redux/redux-setup.js';
import { import {
workerRequestMessageTypes, workerRequestMessageTypes,
type WorkerRequestMessage, type WorkerRequestMessage,
type WorkerResponseMessage, type WorkerResponseMessage,
} from '../types/worker-types.js'; } from '../types/worker-types.js';
Show All 16 Linesclass DatabaseModule {
status: DatabaseStatus; status: DatabaseStatus;
constructor() { constructor() {
const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous const currentLoggedInUserID = preloadedState.currentUserInfo?.anonymous
? undefined ? undefined
: preloadedState.currentUserInfo?.id; : preloadedState.currentUserInfo?.id;
const isSupported = isSQLiteSupported(currentLoggedInUserID); const isSupported = isSQLiteSupported(currentLoggedInUserID);
if (!isSupported) { if (!isSupported || isDesktopSafari()) {
tomekUnsubmitted
Not Done
Inline Actions

Can we have a const instead of a function that tells if it is Safari? We can avoid having to detect the browser multiple times, as this fact doesn't change through the lifetime of the app.

tomek: Can we have a const instead of a function that tells if it is Safari? We can avoid having to…
kamilAuthorUnsubmitted
Done
Inline Actions

good point, thanks

kamil: good point, thanks
this.status = databaseStatuses.notSupported; this.status = databaseStatuses.notSupported;
kamilAuthorUnsubmitted
Done
Inline Actions

Naming might be confusing, but at this point database is not supported (we don't have a key to it), it will be supported after main thread will provide encryption key

kamil: Naming might be confusing, but at this point database is not supported (we don't have a key to…
tomekUnsubmitted
Not Done
Inline Actions

Do we change the status later?

tomek: Do we change the status later?
kamilAuthorUnsubmitted
Done
Inline Actions

yes, in initDBForLoggedInUser()

kamil: yes, in `initDBForLoggedInUser()`
} else { } else {
this.init(); this.init();
} }
} }
init(encryptionKey?: ?SubtleCrypto$JsonWebKey) { init(encryptionKey?: ?SubtleCrypto$JsonWebKey) {
this.status = databaseStatuses.initInProgress; this.status = databaseStatuses.initInProgress;
this.worker = new SharedWorker(DATABASE_WORKER_PATH); this.worker = new SharedWorker(DATABASE_WORKER_PATH);
▲ Show 20 Lines • Show All 77 Lines • Show Last 20 Lines