Skip to content

Commit

Permalink
fix(remote-config, getAll): init with empty config
Browse files Browse the repository at this point in the history
previously if you incorrectly called getAll before setting defaults or fetching any
remote config, we would crash.

Now we initialize values as an empty object, so we have a graceful failure

confirmed via code inspection that this matches expecations from firebase-js-sdk
https://github.com/firebase/firebase-js-sdk/blob/acc58102d4429ce0593ec22192e76018e9d16ed7/packages/remote-config/test/remote_config.test.ts#L333-L335

Fixes #5854
  • Loading branch information
mikehardy committed Nov 16, 2021
1 parent 0a326d9 commit 5ab2462
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 16 deletions.
22 changes: 8 additions & 14 deletions packages/remote-config/__tests__/remote-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,20 +81,6 @@ describe('remoteConfig()', function () {
});
});

// TODO set up a mock for getAll from issue 5854 and probe if result is empty
// describe('getAll() with remote', function () {
// it('should return an object of all available values', function () {
// const config = firebase.remoteConfig().getAll();
// config.number.asNumber().should.equal(1337);
// config.number.getSource().should.equal('remote');
// // firebase console stores as a string
// config.float.asNumber().should.equal(123.456);
// config.float.getSource().should.equal('remote');
// config.prefix_1.asNumber().should.equal(1);
// config.prefix_1.getSource().should.equal('remote');
// });
// });

describe('setDefaults()', function () {
it('it throws if defaults object not provided', function () {
expect(() => {
Expand All @@ -112,4 +98,12 @@ describe('remoteConfig()', function () {
}).toThrow('must be a string value');
});
});

describe('getAll() should not crash', function () {
it('should return an empty object pre-fetch, pre-defaults', function () {
const config = firebase.remoteConfig().getAll();
expect(config).toBeDefined();
expect(config).toEqual({});
});
});
});
3 changes: 1 addition & 2 deletions packages/remote-config/lib/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class FirebaseConfigModule extends FirebaseModule {
minimumFetchIntervalMillis: 43200000,
};
this._lastFetchTime = -1;
this._values = {};
}

getValue(key) {
Expand Down Expand Up @@ -89,9 +90,7 @@ class FirebaseConfigModule extends FirebaseModule {

getAll() {
const values = {};

Object.keys(this._values).forEach(key => (values[key] = this.getValue(key)));

return values;
}

Expand Down

0 comments on commit 5ab2462

Please sign in to comment.