Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow hasDocs when mode = "off" and mobx computedRequiresReaction #81

Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 13 additions & 1 deletion src/Collection.ts
Original file line number Diff line number Diff line change
Expand Up @@ -209,6 +209,18 @@ class Collection<T extends ICollectionDocument = Document>
* @type {boolean}
*/
public get hasDocs(): boolean {
if (this.mode === Mode.Off) {
// Avoid computed property
return this.docs.length > 0;
}
return this._hasDocsComputed;
}

/**
* @private
* Internal getter so we can avoid computed properties when mobx configure computedRequiresReaction and mode is Off
*/
public get _hasDocsComputed(): boolean {
return this.docs.length > 0;
}

Expand Down Expand Up @@ -1028,6 +1040,6 @@ class Collection<T extends ICollectionDocument = Document>
}
}

decorate(Collection, { hasDocs: computed });
decorate(Collection, { _hasDocsComputed: computed });

export default Collection;
18 changes: 8 additions & 10 deletions test/Collection.hasDocs.test.ts
Original file line number Diff line number Diff line change
@@ -1,27 +1,25 @@
import { autorun, Collection, Mode } from "./init";
import { autorun, Collection } from "./init";

test("false when missing ref/path", async () => {
const col = new Collection();
const col = new Collection(undefined, { mode: "off" });
expect(col.hasDocs).toBeFalsy();
});

test("false when collection empty", async () => {
expect.assertions(1);
const col = new Collection("artists", {
mode: Mode.On,
mode: "off",
query: ref => ref.where("genre", "==", "none")
});
await col.ready();
expect(col.hasDocs).toBeFalsy();
col.mode = "off";
await col.fetch();
expect(col.hasDocs).toEqual(false);
});

test("true when collection not empty", async () => {
expect.assertions(1);
const col = new Collection("artists", { mode: "on" });
await col.ready();
expect(col.hasDocs).toBeTruthy();
col.mode = "off";
const col = new Collection("artists", { mode: "off" });
await col.fetch();
expect(col.hasDocs).toEqual(true);
});

test("should not react when number of docs changes", async () => {
Expand Down
5 changes: 2 additions & 3 deletions test/Collection.path.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,8 @@ test('change path to forbidden collection', async () => {
col.path = 'forbidden';
try {
await col.fetch();
}
catch (err) {
} catch (err) {
expect(err).toBeDefined();
}
expect(col.docs.length).toBe(0);
});
});
4 changes: 3 additions & 1 deletion test/Collection.reactivePath.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,9 @@ test('document data', async () => {
expect.assertions(9);
const doc = new Document('settings/setting');
expect(doc.isActive).toBe(false);
const col = new Collection(() => doc.hasData ? `artists/${doc.data.topArtistId}/albums` : undefined);
const col = new Collection(() =>
doc.hasData ? `artists/${doc.data.topArtistId}/albums` : undefined
);
expect(doc.isActive).toBe(false);
expect(col.isActive).toBe(false);
const dispose = autorun(() => {
Expand Down
3 changes: 1 addition & 2 deletions test/Document.path.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,8 +51,7 @@ test('change path to inaccessible document', async () => {
doc.path = 'forbidden/cantReadMe';
try {
await doc.fetch();
}
catch (err) {
} catch (err) {
expect(err).toBeDefined();
}
expect(doc.hasData).toBe(false);
Expand Down
8 changes: 6 additions & 2 deletions test/Document.update.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ const ArtistSchema = struct({
genre: 'string',
memberCount: 'number?',
members: 'object?',
topAlbumId: 'string',
topAlbumId: 'string'
});

test('update field', async () => {
Expand Down Expand Up @@ -83,6 +83,10 @@ test('update field with invalid schema', async () => {
blaat: 10
});
} catch (e) {
expect(e).toEqual(new Error('Invalid value at "blaat" for Document with id "FooFighters": Expected a value of type `undefined` for `blaat` but received `10`.'));
expect(e).toEqual(
new Error(
'Invalid value at "blaat" for Document with id "FooFighters": Expected a value of type `undefined` for `blaat` but received `10`.'
)
);
}
});
6 changes: 3 additions & 3 deletions test/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ beforeAll(() => {
const firestore = firebase.firestore();

// Configure mobx strict-mode
configure({ enforceActions: 'always' });
configure({ enforceActions: 'always', computedRequiresReaction: true });

// Initialize firestorter
initFirestorter({ firebase: firebase, app: firebaseApp });
Expand All @@ -45,9 +45,9 @@ export {
getFirebase,
Collection,
Document,
Mode,
isTimestamp,
autorun,
reaction,
observable,
Mode
observable
};
21 changes: 21 additions & 0 deletions wallaby.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
module.exports = function(w) {
return {
files: [
'src/*.ts',
'test/*.{ts,js}',
'!test/*.test.{ts,js}',
{ pattern: 'babel.config.js', instrument: false },
{ pattern: 'test/firebaseConfig.json', instrument: false }
],
tests: ['test/*.test.{ts,js}'],
env: {
type: 'node'
},
workers: {
// Since tests re-use the same collections
initial: 1,
regular: 1
},
testFramework: 'jest'
};
};