Skip to content

Commit

Permalink
chore: resolveType warning, restore declarativeWrapping list behavior (
Browse files Browse the repository at this point in the history
  • Loading branch information
tgriesser committed Nov 23, 2020
1 parent 306dbaa commit bff7008
Show file tree
Hide file tree
Showing 10 changed files with 40 additions and 175 deletions.
2 changes: 1 addition & 1 deletion docs/content/015-api/020-union-type.mdx
Expand Up @@ -15,8 +15,8 @@ const MediaType = unionType({
description: 'Any container type that can be rendered into the feed',
definition(t) {
t.members('Post', 'Image', 'Card')
t.resolveType(item => item.name)
},
resolveType: (item) => item.name,
})
```

Expand Down
21 changes: 4 additions & 17 deletions src/builder.ts
Expand Up @@ -1078,6 +1078,7 @@ export class SchemaBuilder {

config.definition(
new UnionDefinitionBlock({
typeName: config.name,
addUnionMembers: (unionMembers) => (members = unionMembers),
})
)
Expand Down Expand Up @@ -1243,12 +1244,7 @@ export class SchemaBuilder {
const fieldExtension = new NexusFieldExtension(fieldConfig)
const nonNullDefault = this.getNonNullDefault(typeConfig.extensions?.nexus?.config, 'output')
const { namedType, wrapping } = unwrapNexusDef(fieldConfig.type)
const finalWrap = finalizeWrapping(
`${typeConfig.name}.${fieldConfig.name}`,
nonNullDefault,
wrapping,
fieldConfig.wrapping
)
const finalWrap = finalizeWrapping(nonNullDefault, wrapping, fieldConfig.wrapping)
const builderFieldConfig: Omit<NexusGraphQLFieldConfig, 'resolve' | 'subscribe'> = {
name: fieldConfig.name,
type: rewrapAsGraphQLType(
Expand Down Expand Up @@ -1295,12 +1291,7 @@ export class SchemaBuilder {
): GraphQLInputFieldConfig {
const nonNullDefault = this.getNonNullDefault(typeConfig.extensions?.nexus?.config, 'input')
const { namedType, wrapping } = unwrapNexusDef(fieldConfig.type)
const finalWrap = finalizeWrapping(
`${typeConfig.name}.${fieldConfig.name}`,
nonNullDefault,
wrapping,
fieldConfig.wrapping
)
const finalWrap = finalizeWrapping(nonNullDefault, wrapping, fieldConfig.wrapping)
return {
type: rewrapAsGraphQLType(
this.getInputType(namedType as PossibleInputType),
Expand Down Expand Up @@ -1334,11 +1325,7 @@ export class SchemaBuilder {
}
})
const { namedType, wrapping } = unwrapNexusDef(finalArgDef.type)
const finalWrap = finalizeWrapping(
`${typeConfig.name}.${fieldName} arg ${argName}`,
nonNullDefault,
wrapping
)
const finalWrap = finalizeWrapping(nonNullDefault, wrapping)
allArgs[argName] = {
type: rewrapAsGraphQLType(
this.getInputType(namedType as PossibleInputType),
Expand Down
6 changes: 6 additions & 0 deletions src/definitions/interfaceType.ts
Expand Up @@ -3,6 +3,7 @@ import { GetGen, InterfaceFieldsFor, FieldResolver, ModificationType } from '../
import { AbstractTypes, NexusTypes, NonNullConfig, RootTypingDef, withNexusSymbol } from './_types'
import { OutputDefinitionBlock, OutputDefinitionBuilder } from './definitionBlocks'
import { ArgsRecord } from './args'
import { messages } from '../messages'

export type Implemented = GetGen<'interfaceNames'> | NexusInterfaceTypeDef<any>

Expand Down Expand Up @@ -79,6 +80,11 @@ export class InterfaceDefinitionBlock<TypeName extends string> extends OutputDef
) {
this.typeBuilder.addModification({ ...modifications, field })
}

/* istanbul ignore next */
protected resolveType() {
throw new Error(messages.removedResolveType(this.typeBuilder.typeName))
}
}

export class NexusInterfaceTypeDef<TypeName extends string> {
Expand Down
7 changes: 7 additions & 0 deletions src/definitions/unionType.ts
Expand Up @@ -2,8 +2,10 @@ import { assertValidName } from 'graphql'
import { GetGen } from '../typegenTypeHelpers'
import { AbstractTypes, NexusTypes, RootTypingDef, withNexusSymbol } from './_types'
import { NexusObjectTypeDef } from './objectType'
import { messages } from '../messages'

export interface UnionDefinitionBuilder {
typeName: string
addUnionMembers(members: UnionMembers): void
}

Expand All @@ -18,6 +20,11 @@ export class UnionDefinitionBlock {
members(...unionMembers: UnionMembers) {
this.typeBuilder.addUnionMembers(unionMembers)
}

/* istanbul ignore next */
protected resolveType() {
throw new Error(messages.removedResolveType(this.typeBuilder.typeName))
}
}

export type NexusUnionTypeConfig<TypeName extends string> = {
Expand Down
6 changes: 0 additions & 6 deletions src/definitions/wrapping.ts
Expand Up @@ -295,17 +295,11 @@ export function applyNexusWrapping(toWrap: any, wrapping: NexusWrapKind[]) {
* to determine the proper list of wrapping to apply to the field
*/
export function finalizeWrapping(
location: string,
nonNullDefault: boolean,
typeWrapping: NexusWrapKind[] | ReadonlyArray<NexusWrapKind>,
chainWrapping?: NexusWrapKind[]
): NexusFinalWrapKind[] {
let finalChain: NexusFinalWrapKind[] = []
if (typeWrapping.length && chainWrapping?.length) {
throw new Error(
`Cannot use t.list / nonNull chaining and list() / nonNull() type wrapping the same time (on ${location})`
)
}
const allWrapping = typeWrapping.concat(chainWrapping ?? [])
// Ensure the first item is wrapped, if we're not guarding
if (nonNullDefault && (!allWrapping[0] || allWrapping[0] === 'List')) {
Expand Down
12 changes: 11 additions & 1 deletion src/messages.ts
@@ -1,13 +1,23 @@
export const messages = {
/* istanbul ignore next */
removedResolveType: (location: string) => `\
The .resolveType used in the ${location} has been removed in favor of a more robust approach to handling abstract types.
Visit https://nexusjs.org/docs/guides/abstract-types for an explanation and upgrade info.
Visit https://github.com/graphql-nexus/schema/issues/188 for the original motivation for the change.
`,
/* istanbul ignore next */
removedDeclarativeWrapping: (location: string) => `\
The list/nullable/required object properies used in the ${location} have been removed in favor of better chaining APIs
and the list() / nonNull() type wrapping functions. If you would like to incrementally migrate, or prefer the
existing API, it is now supported via the declarativeWrappingPlugin. Add this to your plugins array in your makeSchema config.
makeSchema({
plugins: [declarativePluginApi, ...]
plugins: [declarativePluginApi(), ...]
})
`,
/* istanbul ignore next */
removedFunctionShorthand: (typeName: string, fieldName: string) =>
`Since v0.18.0 Nexus no longer supports resolver shorthands like:\n\n t.string("${fieldName}", () => ...).\n\nInstead please write:\n\n t.string("${fieldName}", { resolve: () => ... })\n\nIn the next major version of Nexus this will be a runtime error (seen in type ${typeName}).`,
}
3 changes: 3 additions & 0 deletions src/plugins/declarativeWrapping.ts
Expand Up @@ -102,6 +102,9 @@ function maybeWrapType(

let type = field.type
if (field.list === true) {
if (field.nullable === false || field.required === true) {
type = nonNull(type)
}
type = list(type)
} else if (Array.isArray(field.list)) {
for (const isNonNull of field.list) {
Expand Down
13 changes: 0 additions & 13 deletions tests/null-list.spec.ts
Expand Up @@ -437,17 +437,4 @@ describe('edges cases', () => {
}
`)
})

test('cannot use chained API and wrappers at the same time', () => {
expect(() =>
testInputField(list('String'), ['list'], { nonNullDefault: false, useChainingApi: true })
).toThrowErrorMatchingInlineSnapshot(
`"Cannot use t.list / nonNull chaining and list() / nonNull() type wrapping the same time (on Bar.foo)"`
)
expect(() =>
testOutputField(list('String'), ['list'], { nonNullDefault: false, useChainingApi: true })
).toThrowErrorMatchingInlineSnapshot(
`"Cannot use t.list / nonNull chaining and list() / nonNull() type wrapping the same time (on Query.foo)"`
)
})
})
139 changes: 5 additions & 134 deletions tests/plugins/declarativeWrapping.spec.ts
@@ -1,13 +1,6 @@
import { GraphQLInputObjectType, GraphQLSchema } from 'graphql'
import { makeSchema, queryType } from '../../src'
import {
arg,
inputObjectType,
declarativeWrappingPlugin,
list,
SchemaConfig,
stringArg,
} from '../../src/core'
import { arg, inputObjectType, declarativeWrappingPlugin, SchemaConfig } from '../../src/core'

type InputOutputFieldConfig = {
nullable?: boolean
Expand Down Expand Up @@ -172,7 +165,7 @@ describe('output types ; nonNullDefaults = false ;', () => {
nullable: false,
})

expect(field).toMatchInlineSnapshot(`"[String]!"`)
expect(field).toMatchInlineSnapshot(`"[String!]!"`)
})

it.each(OUTPUT_TYPES_NON_NULL_DEFAULTS_FALSE.nonNull)('%s ; nullable = false', (_, field) => {
Expand Down Expand Up @@ -290,7 +283,7 @@ describe('input types ; nonNullDefaults = false ;', () => {
nullable: false,
})

expect(field).toMatchInlineSnapshot(`"[String]!"`)
expect(field).toMatchInlineSnapshot(`"[String!]!"`)
})

it.each(INPUT_TYPES_NON_NULL_DEFAULTS_FALSE.nonNull)('%s ; nullable = false', (_, field) => {
Expand Down Expand Up @@ -365,7 +358,7 @@ describe('input types ; nonNullDefaults = true ;', () => {
}
)

expect(field).toMatchInlineSnapshot(`"[String]!"`)
expect(field).toMatchInlineSnapshot(`"[String!]!"`)
})

it.each(INPUT_TYPES_NON_NULL_DEFAULTS_TRUE.nonNull)('%s ; nullable = false', (_, field) => {
Expand Down Expand Up @@ -425,7 +418,7 @@ describe('arg def ; nonNullDefaults = false ;', () => {
nullable: false,
})

expect(field).toMatchInlineSnapshot(`"[String]!"`)
expect(field).toMatchInlineSnapshot(`"[String!]!"`)
})

it.each(ARG_DEF_NON_NULL_DEFAULTS_FALSE.nonNull)('%s ; nullable = false', (_, field) => {
Expand Down Expand Up @@ -544,125 +537,3 @@ describe('arg def ; nonNullDefaults = true ;', () => {
expect(field).toMatchSnapshot()
})
})

describe('edge-cases', () => {
test('cannot use list: true and a wrapped type at the same time on an output type', () => {
expect(() =>
testField('output', {
type: list('String'),
list: true,
})
).toThrowErrorMatchingInlineSnapshot(
`"[declarativeWrappingPlugin] It looks like you used \`list\` and wrapped the type of the field 'Query.foo'. You should only do one or the other"`
)
})

test('cannot use list: true and t.list at the same time on an output type', () => {
expect(() =>
testField('output', { list: true, useDotListShorthand: true })
).toThrowErrorMatchingInlineSnapshot(
`"Cannot use t.list / nonNull chaining and list() / nonNull() type wrapping the same time (on Query.foo)"`
)
})

test('cannot use nullable and a wrapped type at the same time on an output type', () => {
expect(() =>
testField('output', { type: list('String'), nullable: false })
).toThrowErrorMatchingInlineSnapshot(
`"[declarativeWrappingPlugin] It looks like you used \`nullable\` and wrapped the type of the field 'Query.foo'. You should only do one or the other"`
)
})

test('cannot use list: true and a wrapped type at the same time on an input type', () => {
expect(() =>
testField('input', {
type: list('String'),
list: true,
})
).toThrowErrorMatchingInlineSnapshot(
`"[declarativeWrappingPlugin] It looks like you used \`list\` and wrapped the type of the field 'Foo.foo'. You should only do one or the other"`
)
})

test('cannot use list: true and t.list at the same time on an input type', () => {
expect(() =>
testField('output', {
list: true,
useDotListShorthand: true,
})
).toThrowErrorMatchingInlineSnapshot(
`"Cannot use t.list / nonNull chaining and list() / nonNull() type wrapping the same time (on Query.foo)"`
)
})

test('cannot use nullable and a wrapped type at the same time on an input type', () => {
expect(() =>
testField('input', { type: list('String'), nullable: false })
).toThrowErrorMatchingInlineSnapshot(
`"[declarativeWrappingPlugin] It looks like you used \`nullable\` and wrapped the type of the field 'Foo.foo'. You should only do one or the other"`
)
})

test('cannot use list: true and a wrapped arg type at the same time on an arg', () => {
expect(() =>
testField('arg', {
type: list('String'),
list: true,
})
).toThrowErrorMatchingInlineSnapshot(
`"[declarativeWrappingPlugin] It looks like you used \`list\` and wrapped the type of the arg 'id' of the field 'Query.foo'. You should only do one or the other"`
)
})

test('cannot use nullable and a wrapped arg type at the same time on an arg', () => {
expect(() =>
testField('arg', { type: list('String'), nullable: false })
).toThrowErrorMatchingInlineSnapshot(
`"[declarativeWrappingPlugin] It looks like you used \`nullable\` and wrapped the type of the arg 'id' of the field 'Query.foo'. You should only do one or the other"`
)
})

test('cannot use wrapped arg and list: true at the same time on an arg', () => {
expect(() =>
makeSchema({
outputs: false,
types: [
queryType({
definition(t) {
t.string('foo', {
args: {
id: list(stringArg({ list: true } as any)),
},
})
},
}),
],
plugins: [declarativeWrappingPlugin()],
})
).toThrowErrorMatchingInlineSnapshot(
`"[declarativeWrappingPlugin] It looks like you used \`list\` and wrapped the type of the arg 'id' of the field 'Query.foo'. You should only do one or the other"`
)
})

test('cannot use wrapped arg and nullable: true at the same time on an arg', () => {
expect(() =>
makeSchema({
outputs: false,
types: [
queryType({
definition(t) {
t.string('foo', {
args: {
id: list(stringArg({ nullable: true } as any)),
},
})
},
}),
],
plugins: [declarativeWrappingPlugin()],
})
).toThrowErrorMatchingInlineSnapshot(
`"[declarativeWrappingPlugin] It looks like you used \`nullable\` and wrapped the type of the arg 'id' of the field 'Query.foo'. You should only do one or the other"`
)
})
})
6 changes: 3 additions & 3 deletions tests/wrapping.spec.ts
Expand Up @@ -33,18 +33,18 @@ describe('wrapping', () => {
it('adds nonNull around unguarded elements when nonNullDefaults = true', () => {
const input = ['List', 'List', 'Null', 'List'] as const
const output = ['NonNull', 'List', 'NonNull', 'List', 'List', 'NonNull'] as const
expect(finalizeWrapping('test', true, input)).toEqual(output)
expect(finalizeWrapping(true, input)).toEqual(output)
})
it('does not add nonNull around elements when nonNullDefaults = false', () => {
const input = ['List', 'List', 'List'] as const
const output = ['List', 'List', 'List'] as const
expect(finalizeWrapping('test', false, input)).toEqual(output)
expect(finalizeWrapping(false, input)).toEqual(output)
})

it('does not add nonNull around elements when nonNullDefaults = false', () => {
const input = ['List', 'List', 'NonNull', 'List'] as const
const output = ['List', 'List', 'NonNull', 'List'] as const
expect(finalizeWrapping('test', false, input)).toEqual(output)
expect(finalizeWrapping(false, input)).toEqual(output)
})
})
})

0 comments on commit bff7008

Please sign in to comment.