feat: deprecate plugin onInstall return for consistent API (#637)

src/builder.ts

@@ -129,7 +129,6 @@ import {
   runAbstractTypeRuntimeChecks,
   UNKNOWN_TYPE_SCALAR,
   unwrapNexusDef,
-  validateOnInstallHookResult,
   wrapAsNexusType,
 } from './utils'
 
@@ -675,9 +674,15 @@ export class SchemaBuilder {
       }
       const { config: pluginConfig } = obj
       if (pluginConfig.onInstall) {
-        const installResult = pluginConfig.onInstall(this.builderLens)
-        validateOnInstallHookResult(pluginConfig.name, installResult)
-        installResult.types.forEach((t) => this.addType(t))
+        // TODO(tim): remove anys/warning at 1.0
+        const installResult = pluginConfig.onInstall(this.builderLens) as any
+        if (Array.isArray(installResult?.types)) {
+          console.warn(
+            `Since v0.19.0 Nexus no longer supports a return value from onInstall, you should instead use the hasType/addType api (seen in plugin ${pluginConfig.name}). ` +
+              `In the next major version of Nexus this will be a runtime error.`
+          )
+          installResult.types.forEach((t: any) => this.addType(t))
+        }
       }
       if (pluginConfig.onCreateFieldResolver) {
         this.onCreateResolverFns.push(pluginConfig.onCreateFieldResolver)

src/plugin.ts

@@ -1,5 +1,5 @@
 import { GraphQLFieldResolver, GraphQLResolveInfo, GraphQLSchema } from 'graphql'
-import { NexusAcceptedTypeDef, PluginBuilderLens, SchemaConfig } from './builder'
+import { PluginBuilderLens, SchemaConfig } from './builder'
 import {
   Maybe,
   NexusGraphQLFieldConfig,
@@ -81,7 +81,7 @@ export interface PluginConfig {
    * for types the user has defined top level in their app, and any types added by
    * upstream plugins.
    */
-  onInstall?: (builder: PluginBuilderLens) => { types: NexusAcceptedTypeDef[] }
+  onInstall?: (builder: PluginBuilderLens) => void
   /**
    * Executed once, just after types have been walked but also before the schema definition
    * types are materialized into GraphQL types. Use this opportunity to add / modify / remove

src/plugins/connectionPlugin.ts

@@ -593,9 +593,6 @@ export const connectionPlugin = (connectionPluginConfig?: ConnectionPluginConfig
           },
         })
       )
-
-      // TODO: Deprecate this syntax
-      return { types: [] }
     },
   })
 }

src/utils.ts

@@ -52,7 +52,6 @@ import {
   NexusTypes,
   withNexusSymbol,
 } from './definitions/_types'
-import { PluginConfig } from './plugin'
 import { AllInputTypes } from './typegenTypeHelpers'
 
 export const isInterfaceField = (type: GraphQLObjectType, fieldName: string) => {
@@ -456,21 +455,6 @@ export function venn<T>(xs: Iterable<T>, ys: Iterable<T>): [Set<T>, Set<T>, Set<
   return [lefts, boths, rights]
 }
 
-/**
- * Validate that the data returned from a plugin from the `onInstall` hook is valid.
- */
-export function validateOnInstallHookResult(
-  pluginName: string,
-  hookResult: ReturnType<Exclude<PluginConfig['onInstall'], undefined>>
-): void {
-  if (!Array.isArray(hookResult?.types)) {
-    throw new Error(
-      `Plugin "${pluginName}" returned invalid data for "onInstall" hook:\n\nexpected structure:\n\n  { types: NexusAcceptedTypeDef[] }\n\ngot:\n\n  ${hookResult}`
-    )
-  }
-  // TODO we should validate that the array members all fall under NexusAcceptedTypeDef
-}
-
 export const UNKNOWN_TYPE_SCALAR = decorateType(
   new GraphQLScalarType({
     name: 'NEXUS__UNKNOWN__TYPE',

tests/__snapshots__/plugins.spec.ts.snap

@@ -41,41 +41,3 @@ exports[`runtime config validation checks name is not empty 1`] = `"Plugin \\"\\
 exports[`runtime config validation checks name is string 1`] = `"Plugin \\"1\\" is giving an invalid value for property name: expected \\"string\\" type, got number type"`;
 
 exports[`runtime config validation checks name present 1`] = `"Plugin \\"undefined\\" is missing required properties: name"`;
-
-exports[`runtime onInstall hook handler does not validate types array members yet 1`] = `"Cannot read property 'name' of null"`;
-
-exports[`runtime onInstall hook handler validates return value against shallow schema 1`] = `
-"Plugin \\"x\\" returned invalid data for \\"onInstall\\" hook:
-
-expected structure:
-
-  { types: NexusAcceptedTypeDef[] }
-
-got:
-
-  null"
-`;
-
-exports[`runtime onInstall hook handler validates return value against shallow schema 2`] = `
-"Plugin \\"x\\" returned invalid data for \\"onInstall\\" hook:
-
-expected structure:
-
-  { types: NexusAcceptedTypeDef[] }
-
-got:
-
-  [object Object]"
-`;
-
-exports[`runtime onInstall hook handler validates return value against shallow schema 3`] = `
-"Plugin \\"x\\" returned invalid data for \\"onInstall\\" hook:
-
-expected structure:
-
-  { types: NexusAcceptedTypeDef[] }
-
-got:
-
-  [object Object]"
-`;

tests/plugins.spec.ts

@@ -46,27 +46,6 @@ describe('runtime config validation', () => {
   })
 })
 
-describe('runtime onInstall hook handler', () => {
-  const whenGiven = (onInstall: any) => () =>
-    makeSchema({
-      outputs: false,
-      types: [],
-      plugins: [createPlugin({ name: 'x', onInstall })],
-    })
-
-  it('validates return value against shallow schema', () => {
-    expect(whenGiven(() => null)).toThrowErrorMatchingSnapshot()
-
-    expect(whenGiven(() => ({ types: null }))).toThrowErrorMatchingSnapshot()
-
-    expect(whenGiven(() => ({}))).toThrowErrorMatchingSnapshot()
-  })
-
-  it('does not validate types array members yet', () => {
-    expect(whenGiven(() => ({ types: [null, 1, 'bad'] }))).toThrowErrorMatchingSnapshot()
-  })
-})
-
 describe('a plugin may', () => {
   const whenGiven = (pluginConfig: PluginConfig) => () =>
     makeSchema({
@@ -106,25 +85,27 @@ describe('onInstall plugins', () => {
   })
 
   it('may return an empty array of types', () => {
-    expect(whenGiven({ onInstall: () => ({ types: [] }) }))
+    expect(whenGiven({ onInstall: () => {} }))
   })
 
   it('may contribute types', () => {
     expect(
       whenGiven({
-        onInstall: () => ({
-          types: [queryField],
-        }),
+        onInstall: (builder) => {
+          builder.addType(queryField)
+        },
       })
     ).toMatchSnapshot()
   })
 
   it('has access to top-level types', () => {
     expect(
       whenGiven({
-        onInstall: (builder) => ({
-          types: builder.hasType('foo') ? [] : [queryField],
-        }),
+        onInstall: (builder) => {
+          if (!builder.hasType('foo')) {
+            builder.addType(queryField)
+          }
+        },
         appTypes: [fooObject],
       })
     ).toMatchSnapshot()
@@ -134,8 +115,8 @@ describe('onInstall plugins', () => {
     expect(
       whenGiven({
         onInstall(builder) {
-          return {
-            types: builder.hasType('Query') ? [queryField] : [],
+          if (builder.hasType('Query')) {
+            builder.addType(queryField)
           }
         },
       })
@@ -145,9 +126,11 @@ describe('onInstall plugins', () => {
   it('does not have access to inline types', () => {
     expect(
       whenGiven({
-        onInstall: (builder) => ({
-          types: builder.hasType('Inline') ? [queryField] : [],
-        }),
+        onInstall: (builder) => {
+          if (builder.hasType('Inline')) {
+            builder.addType(queryField)
+          }
+        },
         appTypes: [
           queryType({
             definition(t) {