Skip to content

Commit

Permalink
fix(apps): move the getConverter logic into the base class
Browse files Browse the repository at this point in the history
Moving the getConverter logic into the base class as it returns an object which while not typed as
each individual firestore data converter, it matches the type signatures and removes the duplicated
code.
  • Loading branch information
josephperrott committed Apr 20, 2022
1 parent 0469835 commit e3386cc
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 42 deletions.
22 changes: 3 additions & 19 deletions apps/shared/models/app-models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,29 +17,13 @@ function forApp<
const staticModel = model as unknown as typeof BaseModel;
staticModel.markAsDecorated();

/** The converter object for performing conversions in and out of Firestore. */
const converter = {
fromFirestore: (snapshot: any) => {
return new model(snapshot.data());
},
toFirestore: (model: any) => {
return model.data;
},
};

/**
* Class method to get the converter object, ensuring that the converter returned is always
* the converter from the specific class definition rather than a parent class.
*/
model.prototype.getConverter = function () {
return converter;
};

/**
* Gets the model referenced by the provided FirestoreReference, returning the same reference
* as previously queried if the instance cache finds an entry.
*/
model.prototype.getByReference = function (ref: FirestoreReference<TBase>) {
return getDoc(doc(getFirestore(), fromFirestoreReference(ref)).withConverter(converter));
return getDoc(
doc(getFirestore(), fromFirestoreReference(ref)).withConverter(staticModel.getConverter()),
);
};
}
20 changes: 16 additions & 4 deletions apps/shared/models/base.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,14 +29,26 @@ export abstract class BaseModel<T> {
}

/** A function to get the converter for the model */
static getConverter() {
return this.prototype.getConverter();
static getConverter<T>() {
return this.prototype.getConverter<T>(this as unknown as Constructor<T>);
}
protected getConverter(): {

/**
* Class method to get the converter object, ensuring that the converter returned is always
* the converter from the specific class definition rather than a parent class.
*/
private getConverter<T>(model: Constructor<T>): {
fromFirestore: (snapshot: any) => T;
toFirestore: (model: any) => any;
} {
throw Error('This was not implemented');
return {
fromFirestore: (snapshot: any) => {
return new model(snapshot.data());
},
toFirestore: (model: any) => {
return model.data;
},
};
}

/**
Expand Down
20 changes: 1 addition & 19 deletions apps/shared/models/server-models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,29 +29,11 @@ function forServer<
const staticModel = model as unknown as typeof GithubBaseModel;
staticModel.markAsDecorated();

/** The converter object for performing conversions in and out of Firestore. */
const converter = {
fromFirestore: (snapshot: any) => {
return new model(snapshot.data());
},
toFirestore: (model: any) => {
return model.data;
},
};

/**
* Class method to get the converter object, ensuring that the converter returned is always
* the converter from the specific class definition rather than a parent class.
*/
model.prototype.getConverter = function () {
return converter;
};

/**
* Gets the model referenced by the provided FirestoreReference.
*/
model.prototype.getByReference = async function (ref: FirestoreReference<TBase>) {
return firestore().doc(fromFirestoreReference(ref)).withConverter(converter);
return firestore().doc(fromFirestoreReference(ref)).withConverter(this.getConverter());
};

if (staticModel.githubHelpers !== undefined) {
Expand Down

0 comments on commit e3386cc

Please sign in to comment.