Skip to content

Commit

Permalink
Refactoring use/preload and more (#215)
Browse files Browse the repository at this point in the history
* Drop `initPerformance` (closes #207)
* Cleaning up the use/preload types
* Use my good friend `Proxy` to allow access to both the `serviceProps` and the initialized instance without double calling (e.g, `useAuth()()`)
* More checks against double initializing
* Added binding to the initialize call (closes #213)
* `SuspenseWithPerf` now lazy loads performance monitoring and uses the user timing API
* Fixed the CJS build with a better external test (closes #214)
  • Loading branch information
jamesdaniels authored Feb 20, 2020
1 parent aa0f822 commit 3cebaa9
Show file tree
Hide file tree
Showing 16 changed files with 262 additions and 388 deletions.
1 change: 0 additions & 1 deletion reactfire/.npmignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,2 @@
cjs/index.esm-*
**/*.test.d.ts
**/*.test.js
4 changes: 2 additions & 2 deletions reactfire/auth/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { from } from 'rxjs';
export function preloadUser(firebaseApp: firebase.app.App) {
return preloadAuth(firebaseApp).then(auth => {
const result = preloadObservable(
user(auth() as firebase.auth.Auth),
user(auth()),
`auth:user:${firebaseApp.name}`
);
return result.toPromise();
Expand All @@ -30,7 +30,7 @@ export function useUser<T = unknown>(
auth?: auth.Auth,
options?: ReactFireOptions<T>
): User | T {
auth = auth || useAuth()();
auth = auth || useAuth();
const currentUser = auth.currentUser || options?.startWithValue;
return useObservable(user(auth), `auth:user:${auth.app.name}`, currentUser);
}
Expand Down
19 changes: 0 additions & 19 deletions reactfire/firebaseApp/firebaseApp.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,25 +29,6 @@ describe('FirebaseAppProvider', () => {
spy.mockRestore();
});

it('initializes fireperf if specified', async () => {
const mockPerf = jest.fn();
firebase['performance' as any] = mockPerf;
const app: firebase.app.App = { performance: mockPerf } as any;

render(<FirebaseAppProvider firebaseApp={app} initPerformance />);

expect(mockPerf).toBeCalled();
});

it('does not initialize fireperf if not specified', async () => {
const mockPerf = jest.fn();
firebase['performance' as any] = mockPerf;
const app: firebase.app.App = { performance: mockPerf } as any;

render(<FirebaseAppProvider firebaseApp={app} />);

expect(mockPerf).not.toBeCalled();
});
});

describe('useFirebaseApp', () => {
Expand Down
18 changes: 2 additions & 16 deletions reactfire/firebaseApp/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,14 +6,13 @@ export * from './sdk';
type FirebaseAppContextValue = firebase.app.App;

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

const FirebaseAppContext = React.createContext<
FirebaseAppContextValue | undefined
>(undefined);

type Props = {
initPerformance?: boolean;
firebaseApp?: firebase.app.App;
firebaseConfig?: Object;
appName?: string;
Expand All @@ -24,7 +23,7 @@ const shallowEq = (a: Object, b: Object) =>
[...Object.keys(a), ...Object.keys(b)].every(key => a[key] == b[key]);

export function FirebaseAppProvider(props: Props & { [key: string]: unknown }) {
const { firebaseConfig, appName, initPerformance = false } = props;
const { firebaseConfig, appName } = props;
const firebaseApp: firebase.app.App =
props.firebaseApp ||
React.useMemo(() => {
Expand All @@ -43,19 +42,6 @@ export function FirebaseAppProvider(props: Props & { [key: string]: unknown }) {
}
}, [firebaseConfig, appName]);

React.useMemo(() => {
if (initPerformance === true) {
if (firebaseApp.performance) {
// initialize Performance Monitoring
firebaseApp.performance();
} else {
throw new Error(
'firebase.performance not found. Did you forget to import it?'
);
}
}
}, [initPerformance, firebaseApp]);

return <FirebaseAppContext.Provider value={firebaseApp} {...props} />;
}

Expand Down
Loading

0 comments on commit 3cebaa9

Please sign in to comment.