graphql-nexus / nexus

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

Split outputs config into "typegen" and "sdl" #647

Open jasonkuhrt opened 3 years ago

jasonkuhrt commented 3 years ago

Current typegen related options:

outputs.typgen
shouldGenerateArtifacts
prettierConfig
formatTypegen
typegenConfig

Suggestion:

{
sdl: {
  enabled: boolean // shouldGenerateArtifacts
  path: string
}
typegen: {
  enabled: boolean // shouldGenerateArtifacts
  path: string // outputs.typegen
  format: prettierConfig | formatTypegen
  createInfo: ... // typegenConfig
  headers: ... // typegenAutoConfig.headers
}
jasonkuhrt commented 3 years ago

WDTY @tgriesser @Weakky

jasonkuhrt commented 3 years ago

I think typings.backing could be renamed typings.models.

Weakky commented 3 years ago

I like it a lot overall. The part I'm not sure about is sdl. SDL is not a newcomer-friendly name. The problem is that I don't see schema being a better name, as it feels like it's configuring some stuff related to the whole library. That's probably the reason you've chosen sdl in the first place.

As we discussed, I also prefer models over backing as it's a much more familiar naming that we use other domains for similar concepts (such as orms). I never heard of the term "backing type" before working on @nexus/schema.

jasonkuhrt commented 3 years ago

That's probably the reason you've chosen sdl in the first place.

Yep

The part I'm not sure about is sdl. SDL is not a newcomer-friendly name.

GraphQLSchemaDefinitionLanguageFile is a verbose version, better?

This option is enabled by default now so it might not matter as much as before to be a newcomer friendly term.

Also there is always jsdoc fwiw.

tgriesser commented 3 years ago

Going to need to think on this one for a bit

jasonkuhrt commented 3 years ago

IMO I don't think this can be done after 1.0

jasonkuhrt commented 3 years ago

Updated to use "source" terminology

jasonkuhrt commented 3 years ago

Via @Weakky related to https://github.com/graphql-nexus/schema/issues/583#issuecomment-740046565 wanting to sync local type def sourceTyping with naming we have globally.

Updated to remove typings property namespace.

tgriesser commented 3 years ago

sourceTyping or sourceTypings for the schema config do you think?

jasonkuhrt commented 3 years ago

@tgriesser oh yeah good point! Updated

jasonkuhrt commented 3 years ago

Hm, if user provides typegen.createInfo does that mean that contextType and sourceTypes are completely ignored? Just wondering if there is a way the config can make this relationship more intuitive.

jasonkuhrt commented 3 years ago

Following more discussion with @Weakky changed my mind that "type" is better than "typings"

jasonkuhrt commented 3 years ago

@Weakky and I agreed these are good names for our source type strategies:

GraphQL Schema Strategy (default)
Exact Matching Strategy
Pattern Matching Strategy
Manual Mapping Strategy

With this in mind I have updated the config above.

sourceTypes: { // ...typegenAutoConfig - contextType
  exactMatch: ...
  patternMatch:... // typegenAutoConfig.backingTypeMap
  skip: ...
  debug: boolean
}

Note that I don't think we need the ing suffix...

Also I don't think we need the Strategy suffix... I realize this deviates from abstract types configuration but I think in this case it feels better because skip and debug are peer properties and because two of the strategies aren't listed here (GraphQL Schema Strategy and Manual Mapping Strategy).

jasonkuhrt commented 3 years ago

After [more] discussion with @Weakky I realized typegen auto config is a whole topic unto itself and I have reduced the footprint of this issue to be just about the outputs configuration.