Typescript generics and the need for superstruct and custom documents

[0x80]

For a while, since before the Typescript refactoring, I was using my own type definition for Document like this:

export class Document<T> {
    constructor(
      source?: firestore.DocumentReference | string | (() => string | void),
      options?: DocumentOptions
    );
    readonly data: T;
    ref: firestore.DocumentReference;
    readonly id: string;
    path?: string;
    mode: UpdatingMode;
    readonly active: boolean;
    readonly snapshot: firestore.DocumentSnapshot;
    readonly createTime: string;
    readonly updateTime: string;
    readonly readTime: string;
    update(fields: firestore.UpdateData): Promise<void>;
    set(
      data: T,
      options: firestore.SetOptions
    ): Promise<void>;
    delete(): Promise<void>;
    fetch(): Promise<Document<T>>;
    readonly fetching: boolean;
    readonly isLoading: boolean;
    ready(): Promise<void>;
  }

It allows me to write const user = new Document<User> and user.data would have type User. I see that something like this is not implemented. I don't think it would require any extra code, just a type definition update.

At the same time I don't see the need for superstruct runtime schema validation. I get that it predates Typescript, and makes sense as a guard if you don't have types in your code, but I have been fine just using compile-time type validation so far.

Lots of the API is about creating custom documents and collections with classes and extending them with methods. That's not how I've been working so far.

I create an interface for each of the data structures that live in Firestore. Then I write modules which are basically just groups of functions operating on that data.

If you were to use superstruct together with Typescript, you would basically have two type definitions of each document. One being an interface and one using superstruct's syntax.

For me the docs are confusing in that sense, because they seem to assume a certain style of programming. I am wondering if I'm missing the point. And if not, I think the docs could maybe explain this a bit better.

In any case Document<Type> is essential to the way I'm using Firestorter with Typescript, so I wonder if everyone else is using custom Document classes with superstruct schema's?

[IjzerenHein]

Hi Thijs. I very much get where you are coming from. I realize I have created some confusion in the TypeScript docs and implementation due to my limited experience with it. I've recently worked in a project using Flow and it has helped me to understand and appreciate that technology better now.

Firstly, you are completely correct regarding the Document types. Document should have a generic type for the data, and I have just added that.

The TypeScript types and superstruct do seem to be overkill. I added the superstruct validation because I wanted client-side validation and data integrity checks. TypeScript and Flow seem to fill the same space on the client-side validation part. As for data-integrity, I so see a need for this. Firestore doesn't provide data integrity conveniently, and not all users will be using TypeScript or Flow. Also, you can do things with schemas you can't do with types, like email-address validation or other custom client-side validation.

As for custom Document and Collection classes, I use that feature a lot. I prefer adding methods and getters to Documents that make them easier to use in my React components.

Lastly, I have updated the TypeScript docs and I hope they make a lot more sense (I think so). Could you please review them and let me know if they are still confusing.

Cheers, Hein

[0x80]

I think from a Typescript perspective this example is still a little odd:

type TodoType = {
	text: string;
	finished: boolean;
};

class TodoDoc extends Document<TodoType> {
	setFinished(val: boolean): Promise<void> {
		return this.update({
			finished: val
		});
	}
}

class Todos extends Collection<TodoDoc> {
	constructor(source: CollectionSource, options: ICollectionOptions) {
		super(source, {
			...(options || {}),
			createDocument: (source: CollectionSource, options: ICollectionOptions) =>
				new TodoDoc(source, options)
		});
	}
}

You are declaring a TodoType without a method. And then extend a Document of that type with a method. After that you pass the extended document class as a type to the Collection, but I think there could be just 1 document type that both the Document and Collection refer to.

Also the API for extending a collection with custom documents seems really verbose to me. Maybe I don't have the full picture, but I feel like all you would have to do is tell Collection what class to use when instantiating a document, and what type it is. All this passing around of options and types in the constructor seems unnecessary from a public API point of view. You could pass the class as an option to the constructor.

For example (not tested):

interface TodoType = {
	text: string;
	finished: boolean;
	setFinished: (boolean) => Promise<void>
};

class TodoDoc extends Document<TodoType> {
	setFinished(val: boolean): Promise<void> {
		return this.update({
			finished: val
		});
	}
}

const todos = new Collection<TodoType>('some/source', { documentClass: TodoDoc })

// At this point Collection knows what type it is dealing with, 
// and it knows what class to use for instantiating documents.

This API would be much cleaner I think. No need to import CollectionSource or ICollectionOptions, or pass sources and options around yourself.

In case you are not interested in adding custom methods to documents, like me, then the example can simply be:

type TodoType = {
	text: string;
	finished: boolean;
};

const todos = new Collection<TodoType>('some/source')

The Typescript docs still assume you want to use class methods, so I think it would help to include a simple example without it.

[IjzerenHein]

Thijs, thanks very much for the feedback. I haven't gotten around to process it, but will try to get back to you this or next week. Cheers