Skip to content

Commit

Permalink
fix(Firebase): Only set timestampsInSnapshots when explicitly set (#42)
Browse files Browse the repository at this point in the history
As of firebase@5.8.x timestampsInSnapshots is true implicitly
#39

Furthermore: calling settings() multiple times is not supported.
If your app interacts with Firestore prior to using <FirestoreProvider>,
FirestoreProvider's settings() call will get you into trouble:

Uncaught FirebaseError: Firestore has already been started
and its settings can no longer be changed. You can only call settings()
before calling any other methods on a Firestore object
  • Loading branch information
tomsun authored and green-arrow committed Apr 6, 2019
1 parent ef0cfe5 commit e75e03d
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 8 deletions.
10 changes: 5 additions & 5 deletions src/FirestoreProvider.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@ export default class FirestoreProvider extends Component {
useTimestampsInSnapshots: PropTypes.bool,
};

static defaultProps = {
useTimestampsInSnapshots: false,
};
static defaultProps = {};

constructor(props) {
super(props);

const { firebase, useTimestampsInSnapshots } = props;
const settings = { timestampsInSnapshots: useTimestampsInSnapshots };
const firestore = firebase.firestore();
firestore.settings(settings);
if (typeof useTimestampsInSnapshots !== 'undefined') {
const settings = { timestampsInSnapshots: useTimestampsInSnapshots };
firestore.settings(settings);
}

this.state = {
firestoreDatabase: firestore,
Expand Down
14 changes: 11 additions & 3 deletions src/__tests__/provider.firestore-settings.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,19 +15,27 @@ test('sets timestampsInSnapshots correctly', () => {
</FirestoreProvider>,
);

expect(firestore.settings).toHaveBeenCalledTimes(0);

mount(
<FirestoreProvider firebase={firebase} useTimestampsInSnapshots>
<div>Test</div>
</FirestoreProvider>,
);

expect(firestore.settings).toHaveBeenCalledTimes(1);
expect(firestore.settings).toHaveBeenCalledWith({
timestampsInSnapshots: false,
timestampsInSnapshots: true,
});

mount(
<FirestoreProvider firebase={firebase} useTimestampsInSnapshots>
<FirestoreProvider firebase={firebase} useTimestampsInSnapshots={false}>
<div>Test</div>
</FirestoreProvider>,
);

expect(firestore.settings).toHaveBeenCalledTimes(2);
expect(firestore.settings).toHaveBeenCalledWith({
timestampsInSnapshots: true,
timestampsInSnapshots: false,
});
});

0 comments on commit e75e03d

Please sign in to comment.