Skip to content

Commit ce716fe

Browse files
authored
Using our subject/cache to fix the init race I introduced (#218)
* Using our subject/cache to fix the init race I introduced with the refactor * Naming * Await init on non-contextual app, if one is passed * void | Promise<any> better describes the contract * Global cache and test
1 parent 32c2bb3 commit ce716fe

File tree

6 files changed

+132
-59
lines changed

6 files changed

+132
-59
lines changed

reactfire/firebaseApp/index.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export * from './sdk';
66
type FirebaseAppContextValue = firebase.app.App;
77

88
// INVESTIGATE I don't like magic strings, can we have export this in js-sdk?
9-
export const DEFAULT_APP_NAME = '[DEFAULT]';
9+
const DEFAULT_APP_NAME = '[DEFAULT]';
1010

1111
const FirebaseAppContext = React.createContext<
1212
FirebaseAppContextValue | undefined

reactfire/firebaseApp/sdk.tsx

Lines changed: 65 additions & 56 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
import { DEFAULT_APP_NAME } from '../index';
21
import { useFirebaseApp } from '.';
32
import * as firebase from 'firebase/app';
3+
import { Observable } from 'rxjs';
4+
import { preloadObservable } from '../useObservable';
45

56
type ComponentName =
67
| 'analytics'
@@ -62,17 +63,19 @@ function proxyComponent(
6263
componentName: ComponentName
6364
): FirebaseNamespaceComponent {
6465
let contextualApp: App | undefined;
65-
const useComponent = () => {
66+
const useComponent = (app?: App) => {
6667
contextualApp = useFirebaseApp();
67-
if (!firebase[componentName]) {
68-
throw importSDK(componentName);
68+
const sdkSubject = preload(componentName, app || contextualApp);
69+
if (!sdkSubject.hasValue) {
70+
throw sdkSubject.firstEmission;
6971
}
72+
sdkSubject.value; // get value to throw if there's an error
7073
return firebase[componentName];
7174
};
7275
return new Proxy(useComponent, {
7376
get: (target, p) => target()[p],
7477
apply: (target, _this, args) => {
75-
const component = target().bind(_this);
78+
const component = target(args[0]).bind(_this);
7679
// If they don't pass an app, assume the app in context rather than [DEFAULT]
7780
if (!args[0]) {
7881
args[0] = contextualApp;
@@ -102,94 +105,100 @@ export const performance = usePerformance;
102105
export const remoteConfig = useRemoteConfig;
103106
export const storage = useStorage;
104107

105-
function preload(
108+
function preloadFactory(
106109
componentName: 'auth'
107110
): (
108111
firebaseApp?: App,
109-
settingsCallback?: (instanceFactory: App['auth']) => any
112+
settingsCallback?: (instanceFactory: App['auth']) => void | Promise<any>
110113
) => Promise<App['auth']>;
111-
function preload(
114+
function preloadFactory(
112115
componentName: 'analytics'
113116
): (
114117
firebaseApp?: App,
115-
settingsCallback?: (instanceFactory: App['analytics']) => any
118+
settingsCallback?: (instanceFactory: App['analytics']) => void | Promise<any>
116119
) => Promise<App['analytics']>;
117-
function preload(
120+
function preloadFactory(
118121
componentName: 'database'
119122
): (
120123
firebaseApp?: App,
121-
settingsCallback?: (instanceFactory: App['database']) => any
124+
settingsCallback?: (instanceFactory: App['database']) => void | Promise<any>
122125
) => Promise<App['database']>;
123-
function preload(
126+
function preloadFactory(
124127
componentName: 'firestore'
125128
): (
126129
firebaseApp?: App,
127-
settingsCallback?: (instanceFactory: App['firestore']) => any
130+
settingsCallback?: (instanceFactory: App['firestore']) => void | Promise<any>
128131
) => Promise<App['firestore']>;
129-
function preload(
132+
function preloadFactory(
130133
componentName: 'functions'
131134
): (
132135
firebaseApp?: App,
133-
settingsCallback?: (instanceFactory: App['functions']) => any
136+
settingsCallback?: (instanceFactory: App['functions']) => void | Promise<any>
134137
) => Promise<App['functions']>;
135-
function preload(
138+
function preloadFactory(
136139
componentName: 'messaging'
137140
): (
138141
firebaseApp?: App,
139-
settingsCallback?: (instanceFactory: App['messaging']) => any
142+
settingsCallback?: (instanceFactory: App['messaging']) => void | Promise<any>
140143
) => Promise<App['messaging']>;
141-
function preload(
144+
function preloadFactory(
142145
componentName: 'performance'
143146
): (
144147
firebaseApp?: App,
145-
settingsCallback?: (instanceFactory: App['performance']) => any
148+
settingsCallback?: (instanceFactory: App['performance']) => void | Promise<any>
146149
) => Promise<App['performance']>;
147-
function preload(
150+
function preloadFactory(
148151
componentName: 'remoteConfig'
149152
): (
150153
firebaseApp?: App,
151-
settingsCallback?: (instanceFactory: App['remoteConfig']) => any
154+
settingsCallback?: (instanceFactory: App['remoteConfig']) => void | Promise<any>
152155
) => Promise<App['remoteConfig']>;
153-
function preload(
156+
function preloadFactory(
154157
componentName: 'storage'
155158
): (
156159
firebaseApp?: App,
157-
settingsCallback?: (instanceFactory: App['storage']) => any
160+
settingsCallback?: (instanceFactory: App['storage']) => void | Promise<any>
158161
) => Promise<App['storage']>;
159-
function preload(componentName: ComponentName) {
160-
return async (
162+
function preloadFactory(componentName: ComponentName) {
163+
return (
161164
firebaseApp?: App,
162165
settingsCallback?: (instanceFactory: FirebaseInstanceFactory) => any
163-
) => {
164-
const app = firebaseApp || useFirebaseApp();
165-
const initialized = !!app[componentName];
166-
if (!initialized) {
167-
await importSDK(componentName);
168-
}
169-
const instanceFactory = app[componentName].bind(
170-
app
171-
) as FirebaseInstanceFactory;
172-
if (initialized) {
173-
if (settingsCallback) {
174-
console.warn(
175-
`${componentName} was already initialized on ${
176-
app.name == DEFAULT_APP_NAME ? 'the default app' : app.name
177-
}, ignoring settingsCallback`
178-
);
179-
}
180-
} else if (settingsCallback) {
181-
await Promise.resolve(settingsCallback(instanceFactory));
182-
}
183-
return instanceFactory;
184-
};
166+
) => preload(componentName, firebaseApp, settingsCallback).toPromise();
167+
}
168+
169+
function preload(
170+
componentName: ComponentName,
171+
firebaseApp?: App,
172+
settingsCallback: (instanceFactory: FirebaseInstanceFactory) => any = () => {}
173+
) {
174+
const app = firebaseApp || useFirebaseApp();
175+
return preloadObservable(
176+
new Observable(emitter => {
177+
importSDK(componentName)
178+
.then(() => {
179+
const instanceFactory = app[componentName].bind(
180+
app
181+
) as FirebaseInstanceFactory;
182+
Promise.resolve(settingsCallback(instanceFactory)).then(() => {
183+
emitter.next(instanceFactory);
184+
emitter.complete();
185+
});
186+
})
187+
.catch(e => {
188+
emitter.error(e);
189+
emitter.complete();
190+
});
191+
}),
192+
`firebase-sdk:${componentName}:${app.name}`
193+
);
185194
}
186195

187-
export const preloadAuth = preload('auth');
188-
export const preloadAnalytics = preload('analytics');
189-
export const preloadDatabase = preload('database');
190-
export const preloadFirestore = preload('firestore');
191-
export const preloadFunctions = preload('functions');
192-
export const preloadMessaging = preload('messaging');
193-
export const preloadPerformance = preload('performance');
194-
export const preloadRemoteConfig = preload('remoteConfig');
195-
export const preloadStorage = preload('storage');
196+
export const preloadAuth = preloadFactory('auth');
197+
export const preloadAnalytics = preloadFactory('analytics');
198+
export const preloadDatabase = preloadFactory('database');
199+
export const preloadFirestore = preloadFactory('firestore');
200+
export const preloadFunctions = preloadFactory('functions');
201+
export const preloadMessaging = preloadFactory('messaging');
202+
export const preloadPerformance = preloadFactory('performance');
203+
export const preloadRemoteConfig = preloadFactory('remoteConfig');
204+
export const preloadStorage = preloadFactory('storage');

reactfire/firestore/firestore.test.tsx

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,11 @@ import {
1010
useFirestoreCollectionData,
1111
useFirestoreDocData,
1212
useFirestoreDocDataOnce,
13-
useFirestoreDocOnce
13+
useFirestoreDocOnce,
14+
useFirestore
1415
} from '..';
1516
import { firestore } from 'firebase/app';
17+
import { preloadFirestore } from '../firebaseApp';
1618

1719
describe('Firestore', () => {
1820
let app: import('firebase').app.App;
@@ -41,6 +43,48 @@ describe('Firestore', () => {
4143
.add({ a: 'hello' });
4244
});
4345

46+
describe('useFirestore', () => {
47+
48+
it('awaits the preloadFirestore setup', async () => {
49+
const app2 = firebase.initializeTestApp({
50+
projectId: '123456',
51+
databaseName: 'my-database',
52+
auth: { uid: 'alice' }
53+
});
54+
55+
let firestore: firebase.firestore.Firestore;
56+
let preloadResolved = false;
57+
let preloadResolve: (v?: unknown) => void;
58+
preloadFirestore(app2, () => new Promise(resolve => preloadResolve = resolve)).then(() => preloadResolved = true);
59+
60+
const Firestore = () => {
61+
const firestore = useFirestore(app2);
62+
return (
63+
<div data-testid="success"></div>
64+
);
65+
};
66+
const { getByTestId } = render(
67+
<FirebaseAppProvider firebase={app2}>
68+
<React.Suspense fallback={<h1 data-testid="fallback">Fallback</h1>}>
69+
<Firestore />
70+
</React.Suspense>
71+
</FirebaseAppProvider>
72+
);
73+
74+
await waitForElement(() => getByTestId('fallback'));
75+
expect(preloadResolved).toEqual(false);
76+
77+
await waitForElement(() => getByTestId('success')).then(() => fail('expected throw')).catch(() => {});
78+
expect(preloadResolved).toEqual(false);
79+
80+
preloadResolve();
81+
82+
await waitForElement(() => getByTestId('success'));
83+
expect(preloadResolved).toEqual(true);
84+
85+
});
86+
});
87+
4488
describe('useFirestoreDoc', () => {
4589
it('can get a Firestore document [TEST REQUIRES EMULATOR]', async () => {
4690
const mockData = { a: 'hello' };

reactfire/useObservable/index.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,22 @@ import * as React from 'react';
22
import { Observable } from 'rxjs';
33
import { SuspenseSubject } from './SuspenseSubject';
44

5+
const globalThis = function() {
6+
if (typeof self !== 'undefined') { return self; }
7+
if (typeof window !== 'undefined') { return window; }
8+
if (typeof global !== 'undefined') { return global; }
9+
throw new Error('unable to locate global object');
10+
}();
11+
12+
const PRELOADED_OBSERVABLES = '_reactFirePreloadedObservables';
513
const DEFAULT_TIMEOUT = 30_000;
6-
const preloadedObservables = new Map<string, SuspenseSubject<unknown>>();
14+
15+
// Since we're side-effect free, we need to ensure our observable cache is global
16+
const preloadedObservables = globalThis[PRELOADED_OBSERVABLES] || new Map<string, SuspenseSubject<unknown>>();
17+
18+
if (!globalThis[PRELOADED_OBSERVABLES]) {
19+
globalThis[PRELOADED_OBSERVABLES] = preloadedObservables;
20+
}
721

822
// Starts listening to an Observable.
923
// Call this once you know you're going to render a

sample/package.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
"dependencies": {
66
"@types/jest": "^24.9.0",
77
"firebase": "^7.9.1",
8+
"js-levenshtein": "^1.1.6",
89
"react": "0.0.0-experimental-5de5b6150",
910
"react-dom": "0.0.0-experimental-5de5b6150",
1011
"react-firebaseui": "^4.0.0",

yarn.lock

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7723,6 +7723,11 @@ join-path@^1.1.1:
77237723
url-join "0.0.1"
77247724
valid-url "^1"
77257725

7726+
js-levenshtein@^1.1.6:
7727+
version "1.1.6"
7728+
resolved "https://registry.yarnpkg.com/js-levenshtein/-/js-levenshtein-1.1.6.tgz#c6cee58eb3550372df8deb85fad5ce66ce01d59d"
7729+
integrity sha512-X2BB11YZtrRqY4EnQcLX5Rh373zbK4alC1FW7D7MBhL2gtcC17cTnr6DmfHZeS0s2rTHjUTMMHfG7gO8SSdw+g==
7730+
77267731
"js-tokens@^3.0.0 || ^4.0.0", js-tokens@^4.0.0:
77277732
version "4.0.0"
77287733
resolved "https://registry.yarnpkg.com/js-tokens/-/js-tokens-4.0.0.tgz#19203fb59991df98e3a287050d4647cdeaf32499"

0 commit comments

Comments
 (0)