Skip to content

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Proposal] Advanced Model Field configuration #1620

Closed
3nvi opened this issue Sep 16, 2021 · 1 comment
Closed

[Proposal] Advanced Model Field configuration #1620

3nvi opened this issue Sep 16, 2021 · 1 comment
Labels
binder Related to mapping Go to GraphQL types enhancement New feature or request modelgen

Comments

@3nvi
Copy link

3nvi commented Sep 16, 2021

What happened?

Hey folks, I saw that there's some talk around the need for custom tags and configuration on generated model fields. For example, this PR adds support for additional tags on generated fields.

This is nice, but in my company, we actually needed a more complex configuration to limit the number of manual changes to models when having multiple dataloaders around. Each dataloader addition currently requires "moving away" from autogenerated models in order to support the changes that each "dataloading" field has.

Since gqlgen doesn’t support that we ended up creating a modelgen plugin with a secondary gqlgen-like configuration. In this config, each field has the following options:


  • skip: to skip this field when generating the model
  • include: to include this field when generating the model (opposite of skip and mutually exclusive)
  • fieldType: The type of the generated field
  • fieldName: The name of the generated field (matches gqlgen's fieldName, but we wanted to have all in 1 place)
  • fieldTag: The json tag that the model would get (could be expanded to support any tag, not just json ones)
  • fieldPointer: Whether the field should be a pointer or not (i.e. whether it's nullable)

All of the above mainly serves the purpose of dataloading, where you have to make changes to certain autogenerated model fields (since the autogeneration is not aware that you're going to use a dataloader. Rather than having to do that manually (like the getting started docs suggest), we’ve opted in autogenerating everything. At the same time, this will cover a lot of other needs such as additional tags, field renamings, etc.

I was wondering whether that’s something that we’d want gqlgen to support. This would - more or less - handle all model generation needs that a user might have and would involve expanding the default field configuration, which currently has 2 options: resolver and fieldName.

Examples:

This would add a new field to a model:

MyModel:
  fields:
    myField:
      include: true
      fieldType: string
      fieldName: MyFieldID
      fieldTag: myFieldId

This would change an existing field to match the data loading needs:

MyModel:
  fields:
    myField:
      fieldType: string
      fieldName: MyFieldID
      

This would remove a field from a model:

MyModel:
  fields:
    myField:
      skip: true

What did you expect?

I would expect to limit the amount of changes I have to manually make. Currently, the docs tell you to copy/paste the model, but that's not optimal since it involves a lot of manual maintainance.With the solution above, no manual model maintanance is needed.

I'm personally willing to own this effort, create a PR and take it from there. What I want to check is:

  • Is this something that gqlgen would want to support?
  • Is the work going to be reviewed and ultimately merged in a reasonable time frame. I'm only asking because the PR I linked above has been open for more than a year :/

Minimal graphql.schema and models to reproduce

The schema from the Getting Started docs should do. Ultimately, with my proposal, no copy/pasting would be needed to simply remove or modify a single field.

versions

  • v0.14.0
@tschwann
Copy link

Bump!

@lwc lwc added binder Related to mapping Go to GraphQL types modelgen labels Sep 17, 2021
@lwc lwc added the enhancement New feature or request label Sep 17, 2021
@frederikhors frederikhors converted this issue into discussion #1911 Feb 4, 2022

This issue was moved to a discussion.

You can continue the conversation there. Go to discussion →

Labels
binder Related to mapping Go to GraphQL types enhancement New feature or request modelgen
Projects
None yet
Development

No branches or pull requests

3 participants