better typings for server code

packages/graphql/extendModel.ts

@@ -9,30 +9,63 @@ import {
   VulcanGraphqlModel,
   MutationCallbackDefinitions,
   VulcanGraphqlSchema,
+  VulcanGraphqlModelServer,
 } from "./typings";
 import { VulcanModel, createModel, CreateModelOptions } from "@vulcanjs/model";
 import {
   getDefaultFragmentText,
   getDefaultFragmentName,
 } from "./fragments/defaultFragment";
-import { camelCaseify } from "@vulcanjs/utils";
+import { camelCaseify, Merge } from "@vulcanjs/utils";
 import {
   MutationResolverDefinitions,
   QueryResolverDefinitions,
 } from "./server/typings";
-
-interface CreateGraphqlModelSharedOptions {
+import {
+  buildDefaultMutationResolvers,
+  buildDefaultQueryResolvers,
+} from "./server/resolvers";
+/**
+ * Typing is tricky here:
+ * - we want to tell the user when they tried to use a server-only field client-side, and point them to the right solution (type: never)
+ * - we want the sever version to define those server only fields correctly (type: whatever)
+ * - we want to define functions that accept any of those types (eg when they use only the shared fields)
+ *
+ * This type is meant for internal use
+ *
+ * This SO question is similar but all answsers break inheritance, using types instead of cleaner interfaces
+ * @see https://stackoverflow.com/questions/41285211/overriding-interface-property-type-defined-in-typescript-d-ts-file
+ */
+interface CreateGraphqlModelOptions {
   typeName: string; // Canonical name of the model = its graphQL type name
   multiTypeName: string; // Plural version, to be defined manually (automated pluralization leads to unexpected results)
 }
-interface CreateGraphqlModelServerOptions {
+/**
+ * This type is meant to be exposed in the "default", shared helper
+ *
+ * It adds "never" types to help the user detecting when they do something wrong (trying to define server fields client side)
+ */
+export interface CreateGraphqlModelOptionsShared
+  extends CreateGraphqlModelOptions {
+  /** This is a server-only field. Please use "createGraphqlModelServer" if you want to create a model server-side */
+  queryResolvers?: never;
+  /** This is a server-only field. Please use "createGraphqlModelServer" if you want to create a model server-side */
+  mutationResolvers?: never;
+  /** This is a server-only field. Please use "createGraphqlModelServer" if you want to create a model server-side */
+  callbacks?: never;
+}
+/**
+ * This type is meant to be exposed server side
+ *  @server-only
+ */
+export interface CreateGraphqlModelOptionsServer
+  extends CreateGraphqlModelOptions {
+  /** Custom query resolvers (single, multi). Set to "null" if you don't want Vulcan to set any resolvers. Leave undefined if you want to use default resolvers. */
   queryResolvers?: Partial<QueryResolverDefinitions>;
+  /** Custom mutation resolvers (create, update, delete). Set to "null" if you don't want Vulcan to set any resolvers. Leave undefined if you want to use default resolvers. */
   mutationResolvers?: Partial<MutationResolverDefinitions>;
   callbacks?: MutationCallbackDefinitions;
 }
-interface CreateGraphqlModelOptions
-  extends CreateGraphqlModelSharedOptions,
-    CreateGraphqlModelServerOptions {}
 
 // Reusable model extension function
 const extendModel =
@@ -41,12 +74,7 @@ const extendModel =
   ) /*: ExtendModelFunc<VulcanGraphqlModel>*/ =>
   (model: VulcanModel): VulcanGraphqlModel => {
     const name = model.name;
-    const {
-      typeName = name,
-      multiTypeName,
-      queryResolvers,
-      mutationResolvers,
-    } = options;
+    const { typeName = name, multiTypeName } = options;
 
     const singleResolverName = camelCaseify(typeName);
     const multiResolverName = camelCaseify(multiTypeName);
@@ -75,14 +103,10 @@ const extendModel =
       );
     }
 
-    // server-only
     const extendedGraphqlModel = {
       ...graphqlModel,
       defaultFragment,
       defaultFragmentName,
-      // server-only
-      queryResolvers,
-      mutationResolvers,
     };
     const finalModel: VulcanGraphqlModel = {
       ...model,
@@ -92,11 +116,57 @@ const extendModel =
   };
 
 /**
- * Helper to simplify the syntax
+ * Adds server-only fields as well
+ * @param options
+ * @returns
+ */
+export const extendModelServer =
+  (
+    options: CreateGraphqlModelOptionsServer
+  ) /*: ExtendModelFunc<VulcanGraphqlModel>*/ =>
+  (model: VulcanModel): VulcanGraphqlModelServer => {
+    const extendedModel = extendModel(options)(model);
+    const {
+      mutationResolvers: mutationResolversFromOptions,
+      queryResolvers: queryResolversFromOptions,
+    } = options;
+    let mutationResolvers = mutationResolversFromOptions,
+      queryResolvers = queryResolversFromOptions;
+    // NOTE: we use default only if the value is "undefined", if it is explicitely "null" we leave it empty (user will define resolvers manually)
+    if (typeof mutationResolvers === "undefined") {
+      mutationResolvers = buildDefaultMutationResolvers({
+        typeName: extendedModel.graphql.typeName,
+      });
+    }
+    if (typeof queryResolvers === "undefined") {
+      queryResolvers = buildDefaultQueryResolvers({
+        typeName: extendedModel.graphql.typeName,
+      });
+    }
+    /**
+     * If mutationResolvers and queryResolvers are not explicitely null,
+     * use the default ones
+     */
+    const finalModel = extendedModel as VulcanGraphqlModelServer;
+    finalModel.graphql = {
+      ...extendedModel.graphql,
+      mutationResolvers: mutationResolvers ? mutationResolvers : undefined,
+      queryResolvers: queryResolvers ? queryResolvers : undefined,
+    };
+    const name = model.name;
+    return finalModel;
+  };
+
+/**
+ * Let's you create a full-fledged graphql model
+ *
+ * Equivalent to a Vulcan Meteor createCollection
  */
 export const createGraphqlModel = (
   options: CreateModelOptions<VulcanGraphqlSchema> & {
-    graphql: CreateGraphqlModelOptions;
+    // we use the "Shared" version of the type, that is meant to be used for exported functions
+    // => it will display nicer messages when you try to mistakenly use a server-only field
+    graphql: CreateGraphqlModelOptionsShared;
   }
 ): VulcanGraphqlModel => {
   // TODO:
@@ -108,6 +178,23 @@ export const createGraphqlModel = (
   return model;
 };
 
+/**
+ * @server-only
+ */
+export const createGraphqlModelServer = (
+  options: CreateModelOptions<VulcanGraphqlSchema> & {
+    graphql: CreateGraphqlModelOptionsServer;
+  }
+): VulcanGraphqlModelServer => {
+  // TODO:
+  const { graphql, ...baseOptions } = options;
+  const model = createModel({
+    ...baseOptions,
+    extensions: [extendModelServer(options.graphql)],
+  }) as VulcanGraphqlModelServer;
+  return model;
+};
+
 //// CODE FROM CREATE COLLECTION
 //import { Mongo } from "meteor/mongo";
 //import SimpleSchema from "simpl-schema";

packages/graphql/server/parseModel.ts

@@ -26,7 +26,7 @@ import {
 
 import _isEmpty from "lodash/isEmpty";
 import _initial from "lodash/initial";
-import { VulcanGraphqlModel } from "../typings";
+import { VulcanGraphqlModel, VulcanGraphqlModelServer } from "../typings";
 import { ModelResolverMap, AnyResolverMap } from "./typings";
 import {
   ParsedModelMutationResolvers,
@@ -203,7 +203,9 @@ interface ParseModelOutput
   schemaResolvers?: Array<AnyResolverMap>;
   resolvers?: ModelResolverMap;
 }
-export const parseModel = (model: VulcanGraphqlModel): ParseModelOutput => {
+export const parseModel = (
+  model: VulcanGraphqlModelServer
+): ParseModelOutput => {
   const typeDefs: Array<string> = [];
 
   // const {
@@ -216,10 +218,11 @@ export const parseModel = (model: VulcanGraphqlModel): ParseModelOutput => {
   const { schema, name: modelName } = model;
   const { typeName, multiTypeName } = model.graphql;
 
-  const { nestedFieldsList, fields, resolvers: schemaResolvers } = parseSchema(
-    schema,
-    typeName
-  );
+  const {
+    nestedFieldsList,
+    fields,
+    resolvers: schemaResolvers,
+  } = parseSchema(schema, typeName);
 
   const { mainType } = fields;
 
@@ -258,8 +261,8 @@ export const parseModel = (model: VulcanGraphqlModel): ParseModelOutput => {
   const resolvers: ModelResolverMap = {};
   let queries;
   let mutations;
-  const queryDefinitions = model.graphql.queryResolvers; // TODO: get from Model?
-  const mutationDefinitions = model.graphql.mutationResolvers; // TODO: get from Model?
+  const queryDefinitions = model.graphql?.queryResolvers; // TODO: get from Model?
+  const mutationDefinitions = model.graphql?.mutationResolvers; // TODO: get from Model?
   if (queryDefinitions) {
     const parsedQueries = parseQueryResolvers({
       queryResolverDefinitions: queryDefinitions,

packages/graphql/server/resolvers/mutators.ts

@@ -41,15 +41,19 @@ import { throwError } from "./errors";
 import { ModelMutationPermissionsOptions } from "@vulcanjs/model";
 import { isMemberOf } from "@vulcanjs/permissions";
 import { getModelConnector } from "./context";
-import { UpdateInput, DeleteInput, FilterableInput } from "../../typings";
+import {
+  UpdateInput,
+  DeleteInput,
+  FilterableInput,
+  VulcanGraphqlModelServer,
+} from "../../typings";
 import { deprecate } from "@vulcanjs/utils";
 import cloneDeep from "lodash/cloneDeep";
 import isEmpty from "lodash/isEmpty";
 import { ContextWithUser } from "./typings";
 import { VulcanDocument } from "@vulcanjs/schema";
 import { DefaultMutatorName, VulcanGraphqlModel } from "../../typings";
 import { restrictViewableFields } from "@vulcanjs/permissions";
-import { Options } from "graphql/utilities/extendSchema";
 
 /**
  * Throws if some data are invalid
@@ -62,7 +66,7 @@ const validateMutationData = async ({
   context,
   properties,
 }: {
-  model: VulcanGraphqlModel; // data model
+  model: VulcanGraphqlModelServer; // data model
   mutatorName: DefaultMutatorName;
   context: Object; // Graphql context
   properties: Object; // arguments of the callback, can vary depending on the mutator
@@ -186,7 +190,7 @@ async function getSelector({
 }
 
 interface CreateMutatorInput {
-  model: VulcanGraphqlModel;
+  model: VulcanGraphqlModelServer;
   document?: VulcanDocument;
   data: VulcanDocument;
   context?: ContextWithUser;
@@ -269,8 +273,6 @@ export const createMutator = async <TModel extends VulcanDocument>({
       let autoValue;
       // TODO: run for nested
       if (schema[fieldName].onCreate) {
-        // TS is triggering an error for unknown reasons because there is already an if
-        // @ts-expect-error Cannot invoke an object which is possibly 'undefined'.
         autoValue = await schema[fieldName].onCreate(properties); // eslint-disable-line no-await-in-loop
       }
       if (typeof autoValue !== "undefined") {
@@ -315,7 +317,7 @@ export const createMutator = async <TModel extends VulcanDocument>({
 };
 
 interface UpdateMutatorCommonInput {
-  model: VulcanGraphqlModel;
+  model: VulcanGraphqlModelServer;
   /**
    * Using a "set" syntax
    * @deprecated
@@ -494,8 +496,6 @@ export const updateMutator = async <TModel extends VulcanDocument>({
   for (let fieldName of Object.keys(schema)) {
     let autoValue;
     if (schema[fieldName].onUpdate) {
-      // TS is triggering an error for unknown reasons because there is already an if
-      // @ts-expect-error Cannot invoke an object which is possibly 'undefined'.
       autoValue = await schema[fieldName].onUpdate(properties); // eslint-disable-line no-await-in-loop
     }
     if (typeof autoValue !== "undefined") {
@@ -567,7 +567,7 @@ export const updateMutator = async <TModel extends VulcanDocument>({
 };
 
 interface DeleteMutatorCommonInput {
-  model: VulcanGraphqlModel;
+  model: VulcanGraphqlModelServer;
   currentUser?: any;
   context?: ContextWithUser;
   validate?: boolean;
@@ -664,8 +664,6 @@ export const deleteMutator = async <TModel extends VulcanDocument>({
   /* Run fields onDelete */
   for (let fieldName of Object.keys(schema)) {
     if (schema[fieldName].onDelete) {
-      // TS is triggering an error for unknown reasons because there is already an if
-      // @ts-expect-error Cannot invoke an object which is possibly 'undefined'.
       await schema[fieldName].onDelete(properties); // eslint-disable-line no-await-in-loop
     }
   }

packages/graphql/server/tests/mutators/callbacks.test.ts

@@ -1,4 +1,7 @@
-import { createGraphqlModel } from "../../../extendModel";
+import {
+  createGraphqlModel,
+  createGraphqlModelServer,
+} from "../../../extendModel";
 import { VulcanGraphqlModel } from "../../../typings";
 import { createMutator } from "../../resolvers/mutators";
 import { Connector } from "../../resolvers";
@@ -77,7 +80,7 @@ describe("graphql/resolvers/mutators callbacks", function () {
       test("run asynchronous 'after' callback before document is returned", async function () {
         const after = jest.fn(async (doc) => ({ ...doc, createdAfter: 1 }));
         const create = jest.fn(async (data) => ({ _id: 1, ...data }));
-        const Foo = createGraphqlModel({
+        const Foo = createGraphqlModelServer({
           schema,
           name: "Foo",
           graphql: merge({}, modelGraphqlOptions, {
@@ -109,7 +112,7 @@ describe("graphql/resolvers/mutators callbacks", function () {
       test("run asynchronous 'before' callback before document is saved", async function () {
         const before = jest.fn(async (doc) => ({ ...doc, createdBefore: 1 }));
         const create = jest.fn(async (data) => ({ _id: 1, ...data }));
-        const Foo = createGraphqlModel({
+        const Foo = createGraphqlModelServer({
           schema,
           name: "Foo",
           graphql: merge({}, modelGraphqlOptions, {
@@ -150,7 +153,7 @@ describe("graphql/resolvers/mutators callbacks", function () {
               setTimeout(() => resolve(true), 10000)
             )
         );
-        const Foo = createGraphqlModel({
+        const Foo = createGraphqlModelServer({
           ...defaultModelOptions,
           graphql: merge({}, modelGraphqlOptions, {
             callbacks: {
@@ -179,9 +182,9 @@ describe("graphql/resolvers/mutators callbacks", function () {
       test("return a custom validation error", async () => {
         const errSpy = jest
           .spyOn(console, "error")
-          .mockImplementationOnce(() => { }); // silences console.error
+          .mockImplementationOnce(() => {}); // silences console.error
         const validate = jest.fn(() => ["ERROR"]);
-        const Foo = createGraphqlModel({
+        const Foo = createGraphqlModelServer({
           ...defaultModelOptions,
           graphql: merge({}, modelGraphqlOptions, {
             callbacks: {
@@ -207,7 +210,7 @@ describe("graphql/resolvers/mutators callbacks", function () {
             data,
             validate: true, // enable document validation
           });
-        } catch (e) { }
+        } catch (e) {}
         expect(validate).toHaveBeenCalledTimes(1);
         expect(errSpy).toHaveBeenCalled();
       });

packages/graphql/server/tests/mutators/mutators.test.ts

@@ -204,7 +204,9 @@ describe("graphql/resolvers/mutators", function () {
             }),
             findOne: async () => ({ id: "1" }),
             update: async () => ({ id: "1" }),
-            delete: async () => {},
+            delete: async () => {
+              return true;
+            },
           },
         },
       };