Nexus: allow an objectType as second parameter to field()

[capaj]

we were just on a call with @schickling discussing nexus vs decorator based libs and someone mentioned that it might be hard to navigate between the files using the current tooling.

I think this is easily solvable by allowing this:

const CommentType = objectType("Comment", (t) => {
  commonFields(t);
  t.description("A comment about an entry, submitted by a user");
  t.directive("cacheControl", { maxAge: 240 });
  t.float("createdAt", {
    property: "created_at",
    description: "A timestamp of when the comment was posted",
  });
  t.string("content", {
    description: "The text of the comment",
  });
  t.string("repoName", {
    description: "The repository which this comment is about",
  });
});

exports.Mutation = objectType("Mutation", (t) => {
  t.field("submitComment", CommentType, { // use the type directly here instead of just it's name
    args: {
      repoFullName: RepoNameArg,
      commentContent: stringArg({
        required: true,
        description: "The text content for the new comment",
      }),
    },
  });
});

sorry for such a long example showing a simple function overload.
It would be beneficial to keep the string as an option, because it might be needed when running into circular dependencies.

What do you think @tgriesser? Would this be reasonable? It would certainly make an IDE navigation a breeze in VSCode.

[tgriesser]

Yeah dealing with the circular dependency/import-hell that happens when you're passing the concrete objects is why I had held off on adding this as a supported API.

I'm curious how much of a problem it is in-practice, since there's usually only ever going to be a single type definition (maybe a few extendType definitions: https://github.com/graphql-nexus/nexus/issues/4). So a quick search for "objectType('___" should get you there pretty quickly.

I haven't found jumping to the type definition to be something I've wanted to do much in-practice. What you will want though is the jump-to-definition for is the resolver arguments:

t.boolean("isBooked", {
  resolve(launch, _, { dataSources }) {
    return dataSources.userAPI.isBookedOnLaunch({ launchId: `${launch.id}` });
  },
});

Since the types are generated dynamically using TS conditionals, the best approach I've found here to jump to launch, for instance is to add a goToTypeDefinition binding in my keybindings.json

{
    "key": "cmd+'",
    "command": "editor.action.goToTypeDefinition"
}

Another issue is jumping to the return value of the resolver, which I think based on https://github.com/Microsoft/TypeScript/issues/14148 requires some internal changes in TS if anyone wants to take a shot at it!

Anyway, going to hold off on adding this API just yet, but interested to hear what others think.

[Weakky]

I agree with both points here. The whole purpose of using strings is to avoid circular dependencies and passing references straight will cause more harm than good.
However, while I agree this isn't causing too much trouble with object types, I feel like it's way more problematic with input types, that can potentially be reused across the codebase.

Unless we find an alternative to the two proposed solutions, we might be stuck to solve one problem or another (circular dependencies vs navigable code)

[tgriesser]

However, while I agree this isn't causing too much trouble with object types, I feel like it's way more problematic with input types, that can potentially be reused across the codebase.

That's a great point! Need to think about that case a little more.

[capaj]

Yeah dealing with the circular dependency/import-hell that happens when you're passing the concrete objects is why I had held off on adding this as a supported API.

When circular dependency occurs, it can be detected easily-whenever an undefined is passed as an argument rather than an object or a string we can be certain it occurred.

[tgriesser]

Actually I'm going to look into this a little further.

Realizing it would be possible to support this without breaking anything about how type inference works on the resolvers. Also, as long as you're not using default exports of the types and trying to circularly reference those, I think the fact that the fields are all defined in the closure of the second argument of "objectType" block, which isn't evaluated until the schema is built, means that this might work just fine. Will investigate & report back.