graphql-nexus / nexus

Code-First, Type-Safe, GraphQL Schema Construction
https://nexusjs.org
MIT License
3.4k stars 274 forks source link

Add reference directive to typegen output & disable rule for linters #485

Open Akxe opened 4 years ago

Akxe commented 4 years ago

Perceived Problem

The very usual: TS2304: Cannot find name 'NexusPrisma'.

Ideas / Proposed Solution(s)

A very easy fix was to add /// <reference types="nexus-prisma-typegen" /> to the file. This way the user doesn't need to figure out that they need to add something tsconfig.json.

Also when at it, adding disable rule for linters would be awsome... as that file will most never pass user-defined styles...

// eslint-disable
// tslint:disable
/// <reference types="nexus-prisma-typegen" />
jasonkuhrt commented 4 years ago

Hey @akxe, how have you hit that issue before? Was it by altering the types field of tsconfig?

A very easy fix was to add /// to the file

Which file?

Akxe commented 4 years ago

@jasonkuhrt I was trying to get Prisma and nexus working in NWRL setup. I generated an Express server, which for me a tsconfig too. The fact that you can't use types definitely is not expected.

My suggestion was to add the triple-slash directive to the generated typegen file.

makeSchema({
    types: [
        Query, Mutation,
    ],
    plugins: [nexusPrismaPlugin()],
    outputs: {
        schema: __dirname + '/schema.graphql',
        typegen: __dirname + '/nexus.ts',
    },
    typegenAutoConfig: {
        contextType: 'Context.Context',
        sources: [
            {
                source: '@prisma/client',
                alias: 'prisma',
            },
            {
                source: require.resolve('./context'),
                alias: 'Context',
            },
        ],
    },
});
jasonkuhrt commented 4 years ago

NWRL setup

This: https://nrwl.io ?

My suggestion was to add the triple-slash directive to the generated typegen file.

It looks like you're use Nexus Schema standalone, correct?

In that case, you should just configure the [nexus schema] prisma plugin https://www.nexusjs.org/#/components/schema/plugins/prisma?id=configuration to generate the types where you think it makes sense.

Akxe commented 4 years ago

NWRL setup

This: https://nrwl.io ?

Yes.

My suggestion was to add the triple-slash directive to the generated typegen file.

It looks like you're use Nexus Schema standalone, correct?

In that case, you should just configure the [nexus schema] prisma plugin https://www.nexusjs.org/#/components/schema/plugins/prisma?id=configuration to generate the types where you think it makes sense.

The problem is not where they get created, but that if the user has defined compilerOptions.types in tsconfig.json the compilation that references typegen file will crash with the Cannot find name 'NexusPrisma' error.


I was trying to address two issues with the generated typegen file.

1) Using the compilerOptions.types option in tsconfig.json will break compilation unless users add nexus-prisma-typegen type.

{
  "compilerOptions": {
    "types": ["node", ... , "nexus-prisma-typegen" ]
  }
}

2) Running TSLint (or compatible esLint) on the project after generate will always result in some errors, as generated code does not respect rules set by the user, logically, yet inconvenient.

These comments at start of typegen file would fix both.

// eslint-disable
// tslint:disable
/// <reference types="nexus-prisma-typegen" />

...
jasonkuhrt commented 4 years ago

but that if the user has defined compilerOptions.types in tsconfig.json the compilation that references typegen file will crash with the Cannot find name 'NexusPrisma' error.

Why can't the user just import the generated type modules into their app?

Akxe commented 4 years ago

Why can't the user just import the generated type modules into their app?

When compilerOptions.types are specified, typescript will stop looking into node_modules/@types/**. When specified it will only look for types specified in the provided array so for "types": ["node", "express"] it will look into node_modules/@types/node/ and node_modules/@types/express/. I chose those two, as it is very common, to specify them both for nodeJS servers.

The typegen file should include the reference to the nexus-prisma-typegen itself, either by importing import type { NexusPrisma } from 'nexus-prisma-typegen'; or by /// <reference types="nexus-prisma-typegen" />. But I am not sure if importing from nexus-prisma-typegen is even possible.

jasonkuhrt commented 4 years ago

When compilerOptions.types are specified, typescript will stop looking into

I know. And my question is why isn't importing the typegen files into your app directly good enough.

Akxe commented 4 years ago

I know. And my question is why isn't importing the typegen files into your app directly good enough.

If the typegen file does not have the reference to nexus-prisma-typegen, then crud and model of ObjectDefinitionBlock will be of type any:

const name = objectType({
    name: 'name', // Applies to any object, query or mutation
    definition(t) {
        t.model; // `t.crud` is of type `any`
        t.crud; // `t.crud` is of type `any`
    },
});

Basically, because the whole typegen file is just ignored as it is not extending existing any interfaces.

jasonkuhrt commented 4 years ago

If your app imports both typegen files things should work. They will find each other. They are sharing global state. Their order does not matter.

Akxe commented 4 years ago

If your app imports both typegen files things should work. They will find each other. They are sharing global state. Their order does not matter.

Indeed, but if you specify compilerOptions.types in tsconfig this will break. Only the generated typegen will be found. That is why I would like the triple slash reference. The typegen file should reference all its required components so that it does not break in any circumstances.

jasonkuhrt commented 4 years ago

this will break

Why would it break if you import them in your app?

import "./path/to/typegen/nexus.ts"
import "./path/to/typegen/nexus-prisma.ts"
Akxe commented 4 years ago

Here are examples from prisma. If you would add "types": ["node"] to theirs tsconfig.json you would see the TS2304: Cannot find name 'NexusPrisma'. error in typegen file.

https://github.com/prisma/prisma-examples/tree/master/typescript/subscriptions-pubsub https://github.com/prisma/prisma-examples/tree/master/typescript/graphql-apollo-server https://github.com/prisma/prisma-examples/tree/master/typescript/

When I was implementing nexus-prisma I was referring to the examples provided and to the documentation, but since I had "types" defined in tsconfig I was getting errors.


From examples above I was using this import { nexusPrismaPlugin } from 'nexus-prisma'.

import "./path/to/typegen/nexus-prisma.ts"

But I am not sure what file this is supposed to be... If it is @typers/nexus-prisma-typegen" then I would think that it should be part of the file that requires it not part of a step users must make.

import "./path/to/typegen/nexus.ts"
jasonkuhrt commented 4 years ago

But I am not sure what file this is supposed to be...

https://www.nexusjs.org/#/components/schema/plugins/prisma?id=configuration

outputs.typegen

Akxe commented 4 years ago

But I am not sure what file this is supposed to be...

https://www.nexusjs.org/#/components/schema/plugins/prisma?id=configuration

outputs.typegen

That is what I always referred to as typegen as it has user-defined name 😀.


The generated file will be missing the reference to the nexus-prisma-typegen.

See this repo I made for you https://github.com/Akxe/nexus-error-example

jasonkuhrt commented 4 years ago

You aren't setting outputs.typegen in your repro repo: https://github.com/Akxe/nexus-error-example/blob/master/src/schema.ts#L106

then I would think that it should be part of the file that requires it

Not sure what this means. Nexus Schema Prisma plugin generates types just like Nexus Schema does.

not part of a step users must make

If you're looking for something with less boilerplate then you might want to consider Nexus Framework.

Akxe commented 4 years ago

You aren't setting outputs.typegen in your repro repo: https://github.com/Akxe/nexus-error-example/blob/master/src/schema.ts#L106

https://github.com/Akxe/nexus-error-example/blob/master/src/schema.ts#L109 Defines output or am I missing something?

then I would think that it should be part of the file that requires it

Not sure what this means. Nexus Schema Prisma plugin generates types just like Nexus Schema does.

The TS file nexus-prisma-typegen provides is the static part of every typegen file (generated/nexus.ts). No matter what I put in schema.ts I will get same result in nexus-prisma-typegen. Or this is how I understood them...

not part of a step users must make

If you're looking for something with less boilerplate then you might want to consider Nexus Framework.

I would love to try it but npx nexus fails to run... I don't even get an error...

jasonkuhrt commented 4 years ago

Defines output or am I missing something?

What you're missing is that the typegen output config for Nexus Schema has nothing to do with typegen output for Nexus Schema plugins. You need to configure the typegen output for both.

The link I keep sending you https://nexusjs.org/#/components/schema/plugins/prisma?id=configuration is documentation for Nexus Schema Prisma plugin, NOT Nexus Schema.

In code, this is my point:

export const schema = makeSchema({
  types: [Query, Mutation, Post, User],
  plugins: [nexusPrismaPlugin(
    typegen: __dirname + '/generated/nexus-prisma.ts',
  )],
  outputs: {
    schema: __dirname + '/../schema.graphql',
    typegen: __dirname + '/generated/nexus.ts',
  },
  typegenAutoConfig: {
    contextType: 'Context.Context',
    sources: [
      {
        source: '@prisma/client',
        alias: 'prisma',
      },
      {
        source: require.resolve('./context'),
        alias: 'Context',
      },
    ],
  },
})

I would love to try it but npx nexus fails to run... I don't even get an error...

Feel free to open an issue about that!

Akxe commented 4 years ago

@jasonkuhrt I have just tested it and found that the __dirname + '/generated/nexus-prisma.ts' file is the same as node_modules/@types/nexus-prisma-typegen/index.ts.

Conclusion:

If nexusPrismaPlugin should generate output to user folder

If the nexusPrismaPlugin's typegen is to be outputted to the user-defined folder, then this should be also in all of Prisma examples. Also, I would think that a "sane default" would be using the name of nexus's typegen.

ex: /generated/nexus.ts => /generated/nexus.prisma.ts or /generated/nexus-prisma.ts

Shouldn't the nexusPrismaPlugin.options.outputs.typegen be required?

And also, if this is the right way, then adding tslint & eslint would be nice...

If nexusPrismaPlugin should generate output into node_modules/@types

If the nexusPrismaPlugin's typegen is to be outputted to the node_modules/@types/nexus-prisma-typegen/index.ts file, then the tripple-slash directive would suffice.

If both are to be supported

I would consider adding the triple last conditionally...

jasonkuhrt commented 4 years ago

have just tested it and found that the __dirname + '/generated/nexus-prisma.ts' file is the same as node_modules/@types/nexus-prisma-typegen/index.ts.

Yes, that's because they are the same. node_modules/@types/nexus-prisma-typegen/index.ts is the default path for nexusPrismaPlugin.options.outputs.typegen.

I would consider adding the triple last conditionally...

I still don't understand the benefit. Either users stick to the defaults and don't play around with their types config in tsconfig or they want to get more advanced and deal with all the configurations manually.

I don't think we're going to start adding new logic/conditional behaviour here––at least I haven't seen a strong case for it yet.

I do think the docs could be improved, but that's almost always the case, and PRs are welcome for that.

Akxe commented 4 years ago

To me, it looks weird, that the nexus file does not import its direct dependencies.

jasonkuhrt commented 4 years ago

To me, it looks weird, that the nexus file does not import its direct dependencies.

There's currently no [clean] way for Nexus Schema plugins to modify the Nexus Schema typegen file.

Nexus Schema typegen does not depend on Nexus Schema Prisma plugin typegen. It is the inverse.

Since the dependency is managed through global state it is arguably weird to have an import/file directive between them. The only true requirement is that all typegen files be part of the TypeScript type checking process.

Today that is achieved by either 1) relying on the defaults (including not touching types config of tsconfig) or 2) make explicit imports of all the typegen files into your app.

Akxe commented 4 years ago

I am so sorry, I finally got the whole picture... Neither I do not know how to approach this...

Plugins probably need some pre & post hooks to alter the file...

jasonkuhrt commented 4 years ago

All good, you are interacting with a part of the system that few actually understand. It is interesting to have to explain it. It is not as simple as it could be probably.