graphql-nexus / nexus

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

Enable typegen by default in development #657

Open jasonkuhrt opened 3 years ago

jasonkuhrt commented 3 years ago

I think the Nexus defaults for typegen should be more helpful. I propose the following:

  1. Enabled by default when NOT production
  2. Output to node_modules/@types/typegen-nexus/index.d.ts

We learnt from Nexus Framework that these are great defaults.

jasonkuhrt commented 3 years ago

WDTY @tgriesser @Weakky

Nayni commented 3 years ago

Personally I'm not fan of having the generated code being hidden away in some node_module that doesn't exist.

Even though the types are generated, the result of that typegen is because of something a user did in userland and I think it belongs there. I would prefer a default that looks something like path.join(process.cwd(), "nexus.d.ts")

jasonkuhrt commented 3 years ago

@Nayni I think you're a good profile for someone that would want to customize it. Would your proposed default be reliably picked up by the TS language server? It would require a an additional gitignore step by the user. New users would probably ask if they should commit it to version control.

Nayni commented 3 years ago

@Nayni I think you're a good profile for someone that would want to customize it.

That's true, but I also think it's important for any user (experienced or new) that you understand why and how Nexus gets its type information. Hiding that file away means you have to explain it really well in documentation.

Would your proposed default be reliably picked up by the TS language server?

In many of my projects we have it land right under src/ since in a regular tsconfig.json the includes property is usually set to ["src/**/*"] this should get auto picked up rather easily.

It would require a an additional gitignore step by the user. New users would probably ask if they should commit it to version control.

I guess this would be the only additional step you would advice people to do. On the other hand, we actually started with commiting this file but slowly came to the conclusion it added a lot of clutter in a PR. So while I agree that it is a possible best practice, I'm debating if it's good or not that people find this out for themselves.

jasonkuhrt commented 3 years ago

In many of my projects we have it land right under src/

src is not a standard that Nexus can default to

Nayni commented 3 years ago

Maybe I am overthinking this from my own perspective.

I thought about it and it would indeed be neat that Nexus just works out of the box.

But I still believe typegen should be documented and highlighted more. So maybe defaulting to a location like the one you suggested can be good to get it working out of the box reliably, and for users that run into issues or want more fine-grained control there can be a section in the documentation that highlights some of the:

I think especially when you start working with backing types (or models) it is really advisable that this artifact lands in user land code instead of being tucked away into node_modules.

tgriesser commented 3 years ago

But I still believe typegen should be documented and highlighted more.

100% agree on this front. It's the most powerful part of Nexus, and IMO the least documented.

src is not a standard that Nexus can default to

Could we maybe check whether the current project is a TS project path.join(cwd(), tsconfig.json) and if it is, check the outDir? If both of those are set, we could output to the root of that folder, maybe adding something to the header to learn more about why the file showed up?

I don't think this needs to be solved for 0.19 though, would rather get final things wrapped up (#663, #666) and shipped since the current docs are incorrect w/ latest release.

jasonkuhrt commented 3 years ago

I don't think this needs to be solved for 0.19 though

Yeah

Could we maybe check whether the current project is a TS project path.join(cwd(), tsconfig.json) and if it is, check the outDir? If both of those are set, we could output to the root of that folder, maybe adding something to the header to learn more about why the file showed up?

Possible but wouldn't cover JS projects.

calirvine commented 3 years ago

Disclaimer: This could be entirely due to misconfiguration on my side, I find the makeSchema config object a bit confusing.

I've run into some trouble when leaving my typegen output out of source control, and that is that I lose portability of the project. When I clone my repo to another environment, I run npm install and prisma generate which generates my prisma types fine, but when I start my project I get a lot of typescript type errors, usually on objectTypes that use custom scalars.

i.e

TSError: ⨯ Unable to compile TypeScript:
src/schema/story.ts:20:7 - error TS2339: Property 'json' does not exist on type 'ObjectDefinitionBlock<"Story">'.

20     t.json('body')

If I comment out the field, allow the project to run successfully, the scalar is added to my generated types and I can then uncomment the field and it works as expected.

I'm assuming this could be fixed by keeping generated types in source control (currently I keep them in the recommended node_modules/@types/typegen-nexus/index.d.ts) but I could be wrong, and this might be unrelated to this discussion.

jasonkuhrt commented 3 years ago

You should run ts-node-dev with transpile only flag

calirvine commented 3 years ago

Thank you @jasonkuhrt. I did hope it was something I could fix easily on my end, but thought I would ask just in case. (I'm a new convert to TS 😅 and still have a hard time with the workflow and configuration).

In general I agree with this issue's premise, btw. My 2 cents would be that the automatic type generation is a huge part of the reason why I am using nexus/schema, and I'm sure the same is true for a lot of your users, so to have it 'just work' out of the box would be a big benefit in my opinion. The flip side though is if you're less experienced (like me) and the thing that should just work doesn't seem to be working as intended, and you haven't configured anything yourself, and the 'magic' is all happening inside your node_modules outside of view, it can add to the frustration and confusion.

Thanks for all the work you and the team are doing, nexus schema has really reignited my love for GraphQL, it's a lot of fun to work with.

jasonkuhrt commented 3 years ago

The flip side though is if you're less experienced (like me) and the thing that should just work doesn't seem to be working as intended, and you haven't configured anything yourself, and the 'magic' is all happening inside your node_modules outside of view, it can add to the frustration and confusion.

It can't be any worse than it is right now, which is it doesn't work at all out of the box and requires configuration.

We could have the default undocumented and only show docs about how to configure it and do that in examples too.

The default is not guaranteed to work. For example it would be broken by skipLibCheck TS compiler option.

tgriesser commented 3 years ago

I've run into some trouble when leaving my typegen output out of source control, and that is that I lose portability of the project. When I clone my repo to another environment, I run npm install and prisma generate which generates my prisma types fine, but when I start my project I get a lot of typescript type errors, usually on objectTypes that use custom scalars.

Planning on adding a simple script that helps with this step of the process prior to 1.0 - it's a huge pain point.

It'll be a wrapper around ts-node & chokidar, and require that you have these modules installed as peer-deps, but as a runtime enforcement rather than a hard peer dependency, similar to what we do with prettier.

calirvine commented 3 years ago

It can't be any worse than it is right now, which is it doesn't work at all out of the box and requires configuration.

We could have the default undocumented and only show docs about how to configure it and do that in examples too.

The default is not guaranteed to work. For example it would be broken by skipLibCheck TS compiler option.

Totally get it, I think that defaulting to it working properly unless overwritten by configuration is probably the right course to take. I started in nexus framework and migrated over to schema before it had been officially deprecated (for all the reasons it ended up being deprecated), so I missed out on the migration advice given to framework users, and also didn't go through the schema onboarding tutorial at that time. I just skimmed the onboarding tutorial now and I think if I had started from nothing at this time, I would have had much less trouble.

Planning on adding a simple script that helps with this step of the process prior to 1.0 - it's a huge pain point.

It'll be a wrapper around ts-node & chokidar, and require that you have these modules installed as peer-deps, but as a runtime enforcement rather than a hard peer dependency, similar to what we do with prettier.

I think this would be incredibly helpful to people like me!