Improve dx around type checking/autocomplete of objectType etc.

[jasonkuhrt]

Originally raised here prisma-labs/graphql-framework-experiment#77 (comment).

Possible Problems

1. name needs to be given before being able to work on definition with backing types active. This is not encoded by the API.

2. Asymmetrical API between object and field level

   objectType({ name: "foo", ... })
   t.field("foo", { ... })

Solution

Allow user to pass name as first argument to function.

1. Forces user to supply name before attempting work on definition block, required anyways.

2. Harmonizes API across type and fields

   objectType("foo", { ... })
   t.field("foo", { ... })

Notes

Assuming we think this is a good API, I think we might want to see it as an addition, meaning we preserve the object style API too. Then, would also propose that t.field grow to accept object style too e.g. t.field({ name: 'foo', ... }). Full symmetry, backwards compatible. One con I can imagine is that autocomplete becomes potentially less ergonomic. But actually it seems alright:

[tgriesser]

We may want to preserve object passing to support machines (easier for them to build up and pass objects).

Can you give an example of what this looks like?

My only comment here is that there's a lot of history/thought put into why the API is the way it is, so before we try to make any changes, it'd be worth confirming that it will in-fact work well with the limitations of TypeScripts' type-system, generics, etc. But I definitely agree there's a lot of room for improvements to be made.

[jasonkuhrt]

@tgriesser Just that we preserve the current API too. Issue description revised for clarity.

[tgriesser]

Interestingly enough, this was the initial API:

https://github.com/prisma-labs/nexus/blob/86e7e5c26a47b5b0e2733e400fc6f114cfc5c1ce/src/definitions.ts#L30-L54

I am trying to remember the reason for the change. I honestly think it may have been due to the way Prettier formats w/ longer type names:

export const SomeDescriptiveObjectTypeName = objectType(
  "SomeDescriptiveObjectTypeName",
  {
     definition(t) {
        t.int('id')
     }
  }
)

I can't recall whether there were also type implications. I suppose we could overload it, though I don't personally see it as a huge issue, and I'm always wary of overloading APIs unnecessarily.

[jasonkuhrt]

I'm always wary of overloading APIs unnecessarily.

@Weakky was too I think. I'll let him speak for himself 😄 but wanted to mention he had an observation that, if one overload is intended for machines, then simply expose it somehow differently.

Of all dx issues its probably safe to say this one is not top 5 or even 10. But it was real feedback we got and I personally find it burdensome to go between the different name passing styles. So I captured an issue to formalize it. I guess we'll see where it goes, or maybe receive more community feedback.

[Ericnr]

This change is welcoming! Sometimes I do have to wonder where I should declare the name.

[Weakky]

honestly think it may have been due to the way Prettier formats w/ longer type names

@tgriesser I remember we spent a while discussing that part of the API but I couldn't remember why we went for the current one. I remember the prettier issue now, and even though I'm sure there were more, it's already enough for me to discard the proposal again.

I see @Ericnr likes it, let's keep it open for a while and see if more people find it better

[jasonkuhrt]

Yeah this sucks

export const _ = objectType(
  "something",
  {
	// ...

This would be ok:

export const _ = objectType("something", {
	// ...

But yeah... not happening

[Ericnr]

FWIW I think going the contrary route might be even better. Make it so mutationField and queryField take only one object argument, and a name property to it.

const CreateWorker = mutationField({
  name: 'createWorker',
  type: CreateWorkerResponse,
  args: {
    input: arg({
      type: createWorkerInput,
      required: true,
    }),
  },
  resolve: () => {}
})

Now we don't have to guess the number of arguments and no bad prettier formatting. It's a simple and clear convention. Some people will even push for all functions only taking one argument, which is a valid opinion imo. > See swyx's tweet

[jasonkuhrt]

e.g.:

objectType({ name: "foo", ... })
t.field({ name: "foo", ... })

objectType({
	name: "foo",
	defintion(t) { 
		t.field({
			name: "foo",
		})
	}
})

[Ericnr]

I guess the problem now is that you will see different api's when defining a type

const User = objectType({
  name: "User",
  definition(t) {
     t.int('id');
     t.field({ name: "Role", type: "Role", resolver: () => {} })
  }
})

For scalars the name is just a string argument, for fields it's inside the object. Might be an ok tradeoff though.

[jasonkuhrt]

Overall, I am currently in favour of the how the Nexus API is. fields often fit on one line where objects would never. It's a case where prettier behaviour is a real shame, and interesting how much influence it has over the api design. imo

[jasonkuhrt]

One reason the current API fares OK is that field level functions are likely to break out into a resolve function quickly, a natural new line break.

And thanks to type inference, the resolve function signature should stay short all the time, pretty much.