chore: Improve typings around model methods (#6324)

app/models/Collection.ts

@@ -1,3 +1,4 @@
+import invariant from "invariant";
 import trim from "lodash/trim";
 import { action, computed, observable, reaction, runInAction } from "mobx";
 import {
@@ -148,12 +149,13 @@ export default class Collection extends ParanoidModel {
 
     try {
       this.isFetching = true;
-      const { data } = await client.post("/collections.documents", {
+      const res = await client.post("/collections.documents", {
         id: this.id,
       });
+      invariant(res?.data, "Data should be available");
 
       runInAction("Collection#fetchDocuments", () => {
-        this.documents = data;
+        this.documents = res.data;
       });
     } finally {
       this.isFetching = false;

app/models/Document.ts

@@ -3,12 +3,13 @@ import i18n, { t } from "i18next";
 import floor from "lodash/floor";
 import { action, autorun, computed, observable, set } from "mobx";
 import { ExportContentType } from "@shared/types";
-import type { NavigationNode } from "@shared/types";
+import type { JSONObject, NavigationNode } from "@shared/types";
 import Storage from "@shared/utils/Storage";
 import { isRTL } from "@shared/utils/rtl";
 import slugify from "@shared/utils/slugify";
 import DocumentsStore from "~/stores/DocumentsStore";
 import User from "~/models/User";
+import type { Properties } from "~/types";
 import { client } from "~/utils/ApiClient";
 import { settingsPath } from "~/utils/routeHelpers";
 import Collection from "./Collection";
@@ -17,7 +18,7 @@ import ParanoidModel from "./base/ParanoidModel";
 import Field from "./decorators/Field";
 import Relation from "./decorators/Relation";
 
-type SaveOptions = {
+type SaveOptions = JSONObject & {
   publish?: boolean;
   done?: boolean;
   autosave?: boolean;
@@ -388,9 +389,9 @@ export default class Document extends ParanoidModel {
 
   @action
   save = async (
-    fields?: Partial<Document> | undefined,
+    fields?: Properties<typeof this>,
-    options?: SaveOptions | undefined
+    options?: SaveOptions
-  ) => {
+  ): Promise<Document> => {
     const params = fields ?? this.toAPI();
     this.isSaving = true;
 

app/models/base/Model.ts

@@ -1,5 +1,6 @@
 import pick from "lodash/pick";
 import { set, observable, action } from "mobx";
+import { JSONObject } from "@shared/types";
 import type Store from "~/stores/base/Store";
 import Logger from "~/utils/Logger";
 import { getFieldsForModel } from "../decorators/Field";
@@ -77,7 +78,7 @@ export default abstract class Model {
   save = async (
     params?: Record<string, any>,
     options?: Record<string, string | boolean | number | undefined>
-  ) => {
+  ): Promise<Model> => {
     this.isSaving = true;
 
     try {
@@ -108,7 +109,7 @@ export default abstract class Model {
     }
   };
 
-  updateData = action((data: any) => {
+  updateData = action((data: Partial<Model>) => {
     for (const key in data) {
       this[key] = data[key];
     }
@@ -117,7 +118,7 @@ export default abstract class Model {
     this.persistedAttributes = this.toAPI();
   });
 
-  fetch = (options?: any) => this.store.fetch(this.id, options);
+  fetch = (options?: JSONObject) => this.store.fetch(this.id, options);
 
   refresh = () =>
     this.fetch({

app/models/decorators/Field.ts

@@ -1,8 +1,8 @@
 import type Model from "../base/Model";
 
-const fields = new Map<string, string[]>();
+const fields = new Map<string, (string | number | symbol)[]>();
 
-export const getFieldsForModel = (target: Model) =>
+export const getFieldsForModel = <T extends Model>(target: T) =>
   fields.get(target.constructor.name) ?? [];
 
 /**
@@ -14,10 +14,7 @@ export const getFieldsForModel = (target: Model) =>
  */
 const Field = <T>(target: any, propertyKey: keyof T) => {
   const className = target.constructor.name;
-  fields.set(className, [
+  fields.set(className, [...(fields.get(className) || []), propertyKey]);
-    ...(fields.get(className) || []),
-    propertyKey as string,
-  ]);
 };
 
 export default Field;

app/scenes/Document/components/CommentThreadItem.tsx

@@ -8,6 +8,7 @@ import { toast } from "sonner";
 import styled, { css } from "styled-components";
 import breakpoint from "styled-components-breakpoint";
 import { s } from "@shared/styles";
+import { JSONObject } from "@shared/types";
 import { dateToRelative } from "@shared/utils/date";
 import { Minute } from "@shared/utils/time";
 import Comment from "~/models/Comment";
@@ -95,7 +96,7 @@ function CommentThreadItem({
   const [isEditing, setEditing, setReadOnly] = useBoolean();
   const formRef = React.useRef<HTMLFormElement>(null);
 
-  const handleChange = (value: (asString: boolean) => object) => {
+  const handleChange = (value: (asString: boolean) => JSONObject) => {
     setData(value(false));
   };
 

app/stores/CollectionsStore.ts

@@ -10,6 +10,7 @@ import {
   NavigationNode,
 } from "@shared/types";
 import Collection from "~/models/Collection";
+import { Properties } from "~/types";
 import { client } from "~/utils/ApiClient";
 import RootStore from "./RootStore";
 import Store from "./base/Store";
@@ -165,14 +166,14 @@ export default class CollectionsStore extends Store<Collection> {
     }
   };
 
-  async update(params: Record<string, any>): Promise<Collection> {
+  async update(params: Properties<Collection>): Promise<Collection> {
     const result = await super.update(params);
 
     // If we're changing sharing permissions on the collection then we need to
     // remove all locally cached policies for documents in the collection as they
     // are now invalid
     if (params.sharing !== undefined) {
-      this.rootStore.documents.inCollection(params.id).forEach((document) => {
+      this.rootStore.documents.inCollection(result.id).forEach((document) => {
         this.rootStore.policies.remove(document.id);
       });
     }

app/stores/CommentsStore.ts

@@ -87,7 +87,7 @@ export default class CommentsStore extends Store<Comment> {
         documentId,
         ...options,
       });
-      invariant(res && res.data, "Comment list not available");
+      invariant(res?.data, "Comment list not available");
 
       runInAction("CommentsStore#fetchDocumentComments", () => {
         res.data.forEach(this.add);

app/stores/DocumentsStore.ts

@@ -5,7 +5,12 @@ import find from "lodash/find";
 import omitBy from "lodash/omitBy";
 import orderBy from "lodash/orderBy";
 import { observable, action, computed, runInAction } from "mobx";
-import { DateFilter, NavigationNode, PublicTeam } from "@shared/types";
+import type {
+  DateFilter,
+  JSONObject,
+  NavigationNode,
+  PublicTeam,
+} from "@shared/types";
 import { subtractDate } from "@shared/utils/date";
 import { bytesToHumanReadable } from "@shared/utils/files";
 import naturalSort from "@shared/utils/naturalSort";
@@ -13,7 +18,12 @@ import RootStore from "~/stores/RootStore";
 import Store from "~/stores/base/Store";
 import Document from "~/models/Document";
 import env from "~/env";
-import { FetchOptions, PaginationParams, SearchResult } from "~/types";
+import type {
+  FetchOptions,
+  PaginationParams,
+  Properties,
+  SearchResult,
+} from "~/types";
 import { client } from "~/utils/ApiClient";
 import { extname } from "~/utils/files";
 
@@ -704,8 +714,8 @@ export default class DocumentsStore extends Store<Document> {
 
   @action
   async update(
-    params: Partial<Document>,
+    params: Properties<Document>,
-    options?: Record<string, string | boolean | number | undefined>
+    options?: JSONObject
   ): Promise<Document> {
     this.isSaving = true;
 

app/stores/NotificationsStore.ts

@@ -43,7 +43,7 @@ export default class NotificationsStore extends Store<Notification> {
   @action
   markAllAsRead = async () => {
     await client.post("/notifications.update_all", {
-      viewedAt: new Date(),
+      viewedAt: new Date().toISOString(),
     });
 
     runInAction("NotificationsStore#markAllAsRead", () => {

app/stores/SharesStore.ts

@@ -4,7 +4,10 @@ import find from "lodash/find";
 import isUndefined from "lodash/isUndefined";
 import sortBy from "lodash/sortBy";
 import { action, computed } from "mobx";
+import type { Required } from "utility-types";
+import type { JSONObject } from "@shared/types";
 import Share from "~/models/Share";
+import type { Properties } from "~/types";
 import { client } from "~/utils/ApiClient";
 import RootStore from "./RootStore";
 import Store, { RPCAction } from "./base/Store";
@@ -40,7 +43,7 @@ export default class SharesStore extends Store<Share> {
   };
 
   @action
-  async create(params: Record<string, any>) {
+  async create(params: Required<Properties<Share>, "documentId">) {
     const item = this.getByDocumentId(params.documentId);
     if (item) {
       return item;
@@ -49,10 +52,7 @@ export default class SharesStore extends Store<Share> {
   }
 
   @action
-  async fetch(
+  async fetch(documentId: string, options: JSONObject = {}): Promise<any> {
-    documentId: string,
-    options: Record<string, any> = {}
-  ): Promise<any> {
     const item = this.getByDocumentId(documentId);
     if (item && !options.force) {
       return item;

app/stores/UsersStore.ts

@@ -2,7 +2,7 @@ import invariant from "invariant";
 import filter from "lodash/filter";
 import orderBy from "lodash/orderBy";
 import { observable, computed, action, runInAction } from "mobx";
-import { UserRole } from "@shared/types";
+import { type JSONObject, UserRole } from "@shared/types";
 import User from "~/models/User";
 import { client } from "~/utils/ApiClient";
 import RootStore from "./RootStore";
@@ -179,7 +179,7 @@ export default class UsersStore extends Store<User> {
   };
 
   @action
-  async delete(user: User, options: Record<string, any> = {}) {
+  async delete(user: User, options: JSONObject = {}) {
     await super.delete(user, options);
 
     if (!user.isSuspended && user.lastActiveAt) {

app/stores/base/Store.ts

@@ -5,11 +5,12 @@ import orderBy from "lodash/orderBy";
 import { observable, action, computed, runInAction } from "mobx";
 import pluralize from "pluralize";
 import { Pagination } from "@shared/constants";
+import { type JSONObject } from "@shared/types";
 import RootStore from "~/stores/RootStore";
 import Policy from "~/models/Policy";
 import Model from "~/models/base/Model";
 import { getInverseRelationsForModelClass } from "~/models/decorators/Relation";
-import { PaginationParams, PartialWithId } from "~/types";
+import type { PaginationParams, PartialWithId, Properties } from "~/types";
 import { client } from "~/utils/ApiClient";
 import { AuthorizationError, NotFoundError } from "~/utils/errors";
 
@@ -125,12 +126,9 @@ export default abstract class Store<T extends Model> {
     this.data.delete(id);
   }
 
-  save(
+  save(params: Properties<T>, options: JSONObject = {}): Promise<T> {
-    params: Partial<T>,
-    options: Record<string, string | boolean | number | undefined> = {}
-  ): Promise<T> {
     const { isNew, ...rest } = options;
-    if (isNew || !params.id) {
+    if (isNew || !("id" in params)) {
       return this.create(params, rest);
     }
     return this.update(params, rest);
@@ -141,10 +139,7 @@ export default abstract class Store<T extends Model> {
   }
 
   @action
-  async create(
+  async create(params: Properties<T>, options?: JSONObject): Promise<T> {
-    params: Partial<T>,
-    options?: Record<string, string | boolean | number | undefined>
-  ): Promise<T> {
     if (!this.actions.includes(RPCAction.Create)) {
       throw new Error(`Cannot create ${this.modelName}`);
     }
@@ -168,10 +163,7 @@ export default abstract class Store<T extends Model> {
   }
 
   @action
-  async update(
+  async update(params: Properties<T>, options?: JSONObject): Promise<T> {
-    params: Partial<T>,
-    options?: Record<string, string | boolean | number | undefined>
-  ): Promise<T> {
     if (!this.actions.includes(RPCAction.Update)) {
       throw new Error(`Cannot update ${this.modelName}`);
     }
@@ -195,7 +187,7 @@ export default abstract class Store<T extends Model> {
   }
 
   @action
-  async delete(item: T, options: Record<string, any> = {}) {
+  async delete(item: T, options: JSONObject = {}) {
     if (!this.actions.includes(RPCAction.Delete)) {
       throw new Error(`Cannot delete ${this.modelName}`);
     }
@@ -218,7 +210,7 @@ export default abstract class Store<T extends Model> {
   }
 
   @action
-  async fetch(id: string, options: Record<string, any> = {}): Promise<T> {
+  async fetch(id: string, options: JSONObject = {}): Promise<T> {
     if (!this.actions.includes(RPCAction.Info)) {
       throw new Error(`Cannot fetch ${this.modelName}`);
     }

app/types.ts

@@ -1,5 +1,7 @@
+/* eslint-disable @typescript-eslint/ban-types */
 import { Location, LocationDescriptor } from "history";
 import { TFunction } from "i18next";
+import { JSONValue } from "@shared/types";
 import RootStore from "~/stores/RootStore";
 import Document from "./models/Document";
 import FileOperation from "./models/FileOperation";
@@ -198,3 +200,10 @@ export type WebsocketEvent =
 export type AwarenessChangeEvent = {
   states: { user?: { id: string }; cursor: any; scrollY: number | undefined }[];
 };
+
+// TODO: Can we make this type driven by the @Field decorator
+export type Properties<C> = {
+  [Property in keyof C as C[Property] extends JSONValue
+    ? Property
+    : never]?: C[Property];
+};

app/utils/ApiClient.ts

@@ -2,6 +2,7 @@ import retry from "fetch-retry";
 import trim from "lodash/trim";
 import queryString from "query-string";
 import EDITOR_VERSION from "@shared/editor/version";
+import { JSONObject } from "@shared/types";
 import stores from "~/stores";
 import Logger from "./Logger";
 import download from "./download";
@@ -23,11 +24,11 @@ type Options = {
   baseUrl?: string;
 };
 
-type FetchOptions = {
+interface FetchOptions {
   download?: boolean;
   credentials?: "omit" | "same-origin" | "include";
   headers?: Record<string, string>;
-};
+}
 
 const fetchWithRetry = retry(fetch);
 
@@ -38,12 +39,12 @@ class ApiClient {
     this.baseUrl = options.baseUrl || "/api";
   }
 
-  fetch = async (
+  fetch = async <T = any>(
     path: string,
     method: string,
-    data: Record<string, any> | FormData | undefined,
+    data: JSONObject | FormData | undefined,
     options: FetchOptions = {}
-  ) => {
+  ): Promise<T> => {
     let body: string | FormData | undefined;
     let modifiedPath;
     let urlToFetch;
@@ -123,9 +124,9 @@ class ApiClient {
         response.headers.get("content-disposition") || ""
       ).split("filename=")[1];
       download(blob, trim(fileName, '"'));
-      return;
+      return undefined as T;
     } else if (success && response.status === 204) {
-      return;
+      return undefined as T;
     } else if (success) {
       return response.json();
     }
@@ -133,7 +134,7 @@ class ApiClient {
     // Handle 401, log out user
     if (response.status === 401) {
       await stores.auth.logout(true, false);
-      return;
+      throw new AuthorizationError();
     }
 
     // Handle failed responses
@@ -168,7 +169,6 @@ class ApiClient {
     if (response.status === 403) {
       if (error.error === "user_suspended") {
         await stores.auth.logout(false, false);
-        return;
       }
 
       throw new AuthorizationError(error.message);
@@ -204,17 +204,17 @@ class ApiClient {
     throw err;
   };
 
-  get = (
+  get = <T = any>(
     path: string,
-    data: Record<string, any> | undefined,
+    data: JSONObject | undefined,
     options?: FetchOptions
-  ) => this.fetch(path, "GET", data, options);
+  ) => this.fetch<T>(path, "GET", data, options);
 
-  post = (
+  post = <T = any>(
     path: string,
-    data?: Record<string, any> | undefined,
+    data?: JSONObject | FormData | undefined,
     options?: FetchOptions
-  ) => this.fetch(path, "POST", data, options);
+  ) => this.fetch<T>(path, "POST", data, options);
 }
 
 export const client = new ApiClient();

shared/types.ts

@@ -250,5 +250,15 @@ export type Unfurl<T = OEmbedType> = {
   meta?: Record<string, string>;
 };
 
-// eslint-disable-next-line @typescript-eslint/no-explicit-any
+export type JSONValue =
-export type ProsemirrorData = Record<string, any>;
+  | string
+  | number
+  | boolean
+  | undefined
+  | null
+  | { [x: string]: JSONValue }
+  | Array<JSONValue>;
+
+export type JSONObject = { [x: string]: JSONValue };
+
+export type ProsemirrorData = JSONObject;