diff --git a/package.json b/package.json index d0fd322..3e77667 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "firestore-mobx", - "version": "2.0.0-13", + "version": "2.0.0-14", "description": "Observable Firestore documents and collections using MobX", "main": "dist/index.js", "module": "dist/index.es.js", diff --git a/src/__test/document.test.ts b/src/__test/document.test.ts index 4045144..4a76422 100644 --- a/src/__test/document.test.ts +++ b/src/__test/document.test.ts @@ -1,4 +1,4 @@ -import { autorun, configure } from "mobx"; +import { autorun, configure, toJS } from "mobx"; import { ObservableDocument } from "../document"; import { first } from "../utils"; import { @@ -41,7 +41,8 @@ describe("Document", () => { expect(document.id).toBe("__no_id"); expect(document.isLoading).toBe(false); expect(document.hasData).toBe(false); - expect(document.data).toBe(undefined); + expect(() => document.data).toThrow(); + expect(() => document.document).toThrow(); }); it("Can observe a document by ref", async () => { @@ -54,7 +55,6 @@ describe("Document", () => { expect(document.isLoading).toBe(true); expect(document.hasData).toBe(false); - expect(document.data).toBeUndefined(); await document.ready().then((doc) => console.log(doc)); @@ -78,7 +78,6 @@ describe("Document", () => { expect(document.isLoading).toBe(true); expect(document.hasData).toBe(false); - expect(document.data).toBeUndefined(); const disposeListeners = autorun(() => { console.log("isLoading", document.isLoading); @@ -106,7 +105,6 @@ describe("Document", () => { expect(document.isLoading).toBe(false); expect(document.hasData).toBe(false); - expect(document.data).toBeUndefined(); document.attachTo(first(snapshot.docs)?.id); @@ -132,9 +130,6 @@ describe("Document", () => { expect(document.isLoading).toBe(false); expect(document.hasData).toBe(false); - expect(document.data).toBeUndefined(); - - // consoleInspect('snapshot', snapshot.docs.map(doc => doc.data)) document.attachTo(first(snapshot.docs)?.id); @@ -154,7 +149,6 @@ describe("Document", () => { const data = await document.ready(); - // consoleInspect('data', doc?.data) expect(data).toEqual(first(collectionData)); disposeListeners(); diff --git a/src/collection.ts b/src/collection.ts index db8a364..646cc8b 100644 --- a/src/collection.ts +++ b/src/collection.ts @@ -59,7 +59,7 @@ export class ObservableCollection { * if the current listeners belong to that combination or we need to update * them */ - public constructor( + constructor( /** * Ref is optional because for sub-collections you might not know the full * path in advance. Pass undefined if you want to supply the other @@ -75,6 +75,11 @@ export class ObservableCollection { isEmpty: computed, hasDocuments: computed, attachTo: action, + /** + * attachTo being an action doesn't seem to be sufficient to prevent + * strict mode errors + */ + _changeSource: action, }); this.initializeReadyPromise(); @@ -109,11 +114,11 @@ export class ObservableCollection { } } - public get isEmpty(): boolean { + get isEmpty(): boolean { return this.documents.length === 0; } - public get hasDocuments(): boolean { + get hasDocuments(): boolean { return this.documents.length > 0; } @@ -121,23 +126,23 @@ export class ObservableCollection { return this.observedCount > 0; } - public get path(): string | undefined { + get path(): string | undefined { return this.collectionRef ? this.collectionRef.path : undefined; } - public get ref(): FirebaseFirestore.CollectionReference | undefined { + get ref(): FirebaseFirestore.CollectionReference | undefined { return this.collectionRef; } - public attachTo(newRef: FirebaseFirestore.CollectionReference | undefined) { - this.changeSource(newRef); + attachTo(newRef: FirebaseFirestore.CollectionReference | undefined) { + this._changeSource(newRef); /** * Return this so we can chain ready() */ return this; } - private changeSource(newRef?: FirebaseFirestore.CollectionReference) { + _changeSource(newRef?: FirebaseFirestore.CollectionReference) { if (!this.collectionRef && !newRef) { // this.logDebug("Ignore change source"); return; @@ -178,7 +183,7 @@ export class ObservableCollection { } } - public async add(data: T) { + async add(data: T) { if (!hasReference(this.collectionRef)) { this.handleError( new Error(`Can not add a document to a collection that has no ref`), @@ -191,7 +196,7 @@ export class ObservableCollection { return this.collectionRef.add(data); } - public ready() { + ready() { const isListening = !!this.onSnapshotUnsubscribeFn; if (!isListening) { @@ -342,7 +347,7 @@ export class ObservableCollection { }); } - public set query(queryCreatorFn: QueryCreatorFn | undefined) { + set query(queryCreatorFn: QueryCreatorFn | undefined) { this.logDebug("Set query"); this.queryCreatorFn = queryCreatorFn; diff --git a/src/document.ts b/src/document.ts index b0e6f49..1a24b63 100644 --- a/src/document.ts +++ b/src/document.ts @@ -1,12 +1,12 @@ import { action, computed, - makeAutoObservable, makeObservable, observable, onBecomeObserved, onBecomeUnobserved, runInAction, + toJS, } from "mobx"; import { assert, createUniqueId } from "./utils"; @@ -20,14 +20,14 @@ export interface Document { ref: FirebaseFirestore.DocumentReference; } -function isDocumentReference( - source: SourceType, +function isDocumentReference( + source: SourceType, ): source is FirebaseFirestore.DocumentReference { return (source as FirebaseFirestore.DocumentReference).set !== undefined; } -function isCollectionReference( - source: SourceType, +function isCollectionReference( + source: SourceType, ): source is FirebaseFirestore.CollectionReference { return (source as FirebaseFirestore.CollectionReference).doc !== undefined; } @@ -40,10 +40,9 @@ function getPathFromCollectionRef( const NO_DATA = "__no_data" as const; -type SourceType = +type SourceType = | FirebaseFirestore.DocumentReference - | FirebaseFirestore.CollectionReference - | Document; + | FirebaseFirestore.CollectionReference; export class ObservableDocument { _data: T | typeof NO_DATA = NO_DATA; @@ -64,7 +63,7 @@ export class ObservableDocument { onError?: (err: Error) => void; - public constructor(source?: SourceType, options?: Options) { + constructor(source?: SourceType, options?: Options) { makeObservable(this, { _data: observable, isLoading: observable, @@ -72,6 +71,11 @@ export class ObservableDocument { document: computed, attachTo: action, hasData: computed, + /** + * attachTo being an action doesn't seem to be sufficient to prevent + * strict mode errors. + */ + _changeLoadingState: action, }); if (options) { @@ -97,20 +101,7 @@ export class ObservableDocument { * In this case we have data to wait on from the start. So initialize the * promise and resolve function. */ - this.changeLoadingState(true); - } else { - assert(source.ref, "Missing ref in source"); - /** - * Source is type Document, typically passed in from the docs data of - * an ObservableCollection instance. - */ - this.documentRef = source.ref; - // not sure why ref can be undefined here. Maybe a bug in gemini - this.collectionRef = source.ref?.parent; - this.sourcePath = source.ref?.path; - this.logDebug("Constructor from Document"); - - this._data = source.data; + this._changeLoadingState(true); } onBecomeObserved(this, "_data", () => this.resumeUpdates()); @@ -120,13 +111,11 @@ export class ObservableDocument { onBecomeUnobserved(this, "isLoading", () => this.suspendUpdates()); } - public get id(): string { + get id(): string { return this.documentRef ? this.documentRef.id : "__no_id"; } - public attachTo( - documentIdOrRef?: string | FirebaseFirestore.DocumentReference, - ) { + attachTo(documentIdOrRef?: string | FirebaseFirestore.DocumentReference) { if (!documentIdOrRef || typeof documentIdOrRef === "string") { this.changeSourceViaId(documentIdOrRef); } else { @@ -139,14 +128,15 @@ export class ObservableDocument { return this; } - public get data(): T | undefined { - if (!this.documentRef || this._data === NO_DATA) return; + get data(): T /* | undefined */ { + assert(this.documentRef && this._data !== NO_DATA, "No data available"); + // if (!this.documentRef || this._data === NO_DATA) return; - return this._data; + return toJS(this._data); } - public get document(): Document | undefined { - if (!this.documentRef || this._data === NO_DATA) return; + get document(): Document { + assert(this.documentRef && this._data !== NO_DATA, "No document available"); /** * For document we return the data as non-observable by converting it to a @@ -155,7 +145,7 @@ export class ObservableDocument { */ return { id: this.documentRef.id, - data: this._data, + data: this.data, ref: this.documentRef, }; } @@ -164,19 +154,16 @@ export class ObservableDocument { return this.observedCount > 0; } - public get ref(): FirebaseFirestore.DocumentReference | undefined { + get ref(): FirebaseFirestore.DocumentReference { + assert(this.documentRef, "No document ref available"); return this.documentRef; } - public set ref(newRef: FirebaseFirestore.DocumentReference | undefined) { - this.changeSourceViaRef(newRef); - } - - public get path(): string | undefined { + get path(): string | undefined { return this.documentRef ? this.documentRef.path : undefined; } - public async update( + async update( fields: FirebaseFirestore.UpdateData, precondition?: FirebaseFirestore.Precondition, ) { @@ -186,21 +173,21 @@ export class ObservableDocument { return this.documentRef.update(fields, precondition); } - public async set(data: Partial, options?: FirebaseFirestore.SetOptions) { + async set(data: Partial, options?: FirebaseFirestore.SetOptions) { if (!this.documentRef) { throw Error("Can not set data on document with undefined ref"); } return this.documentRef.set(data, options || {}); } - public delete() { + delete() { if (!this.documentRef) { throw Error("Can not delete document with undefined ref"); } return this.documentRef.delete(); } - public ready(): Promise { + ready(): Promise { const isListening = !!this.onSnapshotUnsubscribeFn; if (!isListening && this.documentRef) { @@ -220,7 +207,7 @@ export class ObservableDocument { return this.readyPromise; } - public get hasData(): boolean { + get hasData(): boolean { return this._data !== NO_DATA; } @@ -233,14 +220,14 @@ export class ObservableDocument { this.logDebug("Call ready resolve"); - readyResolve(this.data); + readyResolve(this.hasData ? this.data : undefined); /** * After the first promise has been resolved we want subsequent calls to * ready() to immediately return with the available data. Ready is only * meant to be used for initial data fetching */ - this.readyPromise = Promise.resolve(this.data); + this.readyPromise = Promise.resolve(this.hasData ? this.data : undefined); } } @@ -299,7 +286,7 @@ export class ObservableDocument { runInAction(() => { this._data = snapshot.exists ? (snapshot.data() as T) : NO_DATA; - this.changeLoadingState(false); + this._changeLoadingState(false); }); } @@ -340,14 +327,14 @@ export class ObservableDocument { } this._data = NO_DATA; - this.changeLoadingState(false); + this._changeLoadingState(false); } else { if (this.isObserved) { this.logDebug("Change document -> update listeners"); this.updateListeners(true); } - this.changeLoadingState(true); + this._changeLoadingState(true); } } @@ -391,14 +378,14 @@ export class ObservableDocument { } this._data = NO_DATA; - this.changeLoadingState(false); + this._changeLoadingState(false); } else { if (this.isObserved) { this.logDebug("Change document -> update listeners"); this.updateListeners(true); } - this.changeLoadingState(true); + this._changeLoadingState(true); } } @@ -452,7 +439,7 @@ export class ObservableDocument { } } - private changeLoadingState(isLoading: boolean) { + _changeLoadingState(isLoading: boolean) { this.logDebug(`Change loading state: ${isLoading}`); this.changeReady(!isLoading); this.isLoading = isLoading;