herbsjs / herbs2gql

Create a GraphQL endpoint based on Entities and Use Cases
MIT License
14 stars 21 forks source link

Issue with "requestFieldType2gql" function and GraphQL Input types #58

Closed italojs closed 1 year ago

italojs commented 1 year ago

An issue has been identified with the function "requestFieldType2gql". This function takes an entity as a parameter, and if that entity extends BaseEntity, we append the string "Input" to the property type in GraphQL. For example:

mutation createUser(name: String, age: Number, Address: EntityAddressInput)

The relevant line of code can be found here.

However, when attempting to execute this operation, GraphQL fails to find the "Input" type, resulting in the following error:

Error: Unknown type "CustomerSubscriptionInput". Did you mean "CustomerSubscription"?

A preliminary solution could be to create Inputs for entities and also create an Input for request objects within usecases. However, this might not be the best approach, as we might end up creating Inputs that aren't necessary or that don't necessarily fit what's expected in a usecase's "request" field.

usecase('create user'
[...]
request: { 
 name: String
 age: Number,
 address: AddressEntity
}
[...]

For instance, in the usecase create user, where the request includes an entity-based field (e.g., address: AddressEntity), creating an Input for it solves the problem, but this Input would also include an "ID" field. This is a potential issue as we don't want the ID in the address field during user creation.

My proposed solution works for most scenarios, but not all. The suggestion is to proceed with this preliminary solution for the beta release and open up a discussion for a more definitive solution.

dalssoft commented 1 year ago

The current implementation already create the CustomerSubscription but not the CustomerSubscriptionInput, right?

What if we remove the Input at the end of the name, would it work?

italojs commented 1 year ago

to receive complex objects into mutation as params we need to create a Input instead of a type https://graphql.org/graphql-js/mutations-and-input-types/ So as I know, we need the input

dalssoft commented 1 year ago

to receive complex objects into mutation as params we need to create a Input instead of a type https://graphql.org/graphql-js/mutations-and-input-types/ So as I know, we need the input

TL;DR: I don't think we should use Inputs with Herbs at all. (1) It won't work well with the way we extract the request object from the use case. (2) I don't see the benefit of using Inputs at all.

Complete answer:

As I understand, we don't really need Inputs, like a MUST have. It's more like an Apollo-imposed (and debatable) best practice. More on that later.

However, I don't think it will work well with the way use create the metadata for Herbs use cases.

On the doc you informed they recommend to create a Input for a Message object so it can be used to create and update mutations, where on the creation scenario there is no ID:

input MessageInput {
  content: String
  author: String
}

type Message {
  id: ID!
  content: String
  author: String
}

type Query {
  getMessage(id: ID!): Message
}

type Mutation {
  createMessage(input: MessageInput): Message
  updateMessage(id: ID!, input: MessageInput): Message
}

The only difference from Message type and MessageInput input is that MessageInput doesn't have ID.

With Herbs, the way we declare the request of a use case is different.


usecase('Create Message', {
    request: request.from(Message, { ignoreIDs: true }), // { content: String, author: String }
...

usecase('Delete Message', {
    request: { id: String },
...

usecase('Find a Message', {
    request: { id: String },
...

usecase('Update Message', {
    request: request.from(Message), // { id: String, content: String, author: String }

So it won't be possible to generate a Input from the request object given that resquest.from extract the fields and the information about the original entity (Message) is lost.

All of that to say that I don't think we should use Inputs with Herbs at all.

We could use scalar types for simple objects:

type Mutation {
  createMessage(content: String, author: String): Message
  updateMessage(id: ID!, content: String, author: String): Message
}

This is very straight forward for Herbs to work with.

And use gql types for entity objects:

entity('Message', {
    id: String,
    content: String,
    user: User // entity object
})

usecase('Create Message', {
    request: request.from(Message, { ignoreIDs: true }), // { content: String, user: User }
...
type Message {
  id: ID!
  content: String
  user: User
}

type User {
  id: ID!
  name: String
  ...
}

type Mutation {
  createMessage(content: String, user: User): Message
  updateMessage(id: ID!, content: String, user: User): Message
}

This translates well to the way Herbs works. Actully, the first implementation of Herbs GraphQL was like that. But we changed to use Inputs because it was the recommended way by Apollo, but it seems that it doesn't work well.

Also, regardless of Herbs, I have a hard time to see the benefit of using Inputs.

Why this:

input MessageInput {
  content: String
  author: String
}

type Mutation {
  createMessage(input: MessageInput): Message
  updateMessage(id: ID!, input: MessageInput): Message
}

is better than this?

type Mutation {
  createMessage(content: String, author: String): Message
  updateMessage(id: ID!, content: String, author: String): Message
}

Given than in both cases we need to inform content and author to create a message. I doubt that having Input will make the life of the frontend developer easier. It seems to be a bad abstraction to me.

italojs commented 1 year ago

how do you see the DX for large and pretty complex objects? like

{
  "endereço": {
    "rua": "Avenida Principal",
    "número": 123,
    "complemento": "Bloco A",
    "bairro": "Centro",
    "cidade": "Exemploville",
    "estado": "Exemplostate",
    "país": "Exempleland",
    "cep": "12345-678",
    "coordenadas": {
      "latitude": 40.712776,
      "longitude": -74.005974
    },
    "informações_adicionais": {
      "tipo": "Residencial",
      "ano_construção": 2005,
      "proprietário": {
        "nome": "João da Silva",
        "idade": 35,
        "contato": {
          "telefone": "(00) 1234-5678",
          "email": "joao.silva@example.com"
        }
      },
      "areas": {
        "quartos": 3,
        "banheiros": 2,
        "sala_estar": true,
        "sala_jantar": true,
        "cozinha": {
          "tipo": "Americana",
          "eletrodomésticos": ["geladeira", "fogão", "microondas"]
        }
      }
    }
  }
}

My point is, in a scenario like this we will handle everything as scalar? the DX experience sont will be interesting

another thing is, the difference from the request object could be totally different from the final entity, i think you are looking for simple scenarios but the framework will be used in many scenarios that we cant imagine, so base us on request.from to build the Inputs is super important

dalssoft commented 1 year ago

For coordenadas, informações_adicionais, proprietário, contato, areas I guess it would be necessary an entity (Herbs side) and a type (GQL side). If we use Inputs we would have to create a Input for each of those objects. Again, I don't see the benefit of using Inputs here anyway. Please, enlighten me if I'm missing something.

BTW, "Input types can't have fields that are other objects, only basic scalar types, list types, and other input types." here

For the rest, like endereço, rua, número, etc it would be scalar types as there is no other way to represent them.

the difference from the request object could be totally different from the final entity

request.from is just a helper to extract the fields from the entity. If the request is different from the entity, you can just create the request object manually and it should work fine.

italojs commented 1 year ago

I will do a check if grahphql accepts type as mutation param, i didnt find it on internet saying it accepts, but didnt find saying it not accept too

dalssoft commented 1 year ago

I found a stackoverflow answer that tries to argument in favor of inputs: https://stackoverflow.com/questions/41743253/whats-the-point-of-input-type-in-graphql

The main arguments are that Inputs are different than objects used to output. I could agree with that if Herbs2GQL was using all the features for validation and default value.

However I don't see these arguments strong enough since Herbs2GQL generates only simple objects (types and accepting nulls) leaving complex validations and defaults for the entities and use cases. So, it is ok to have just one abstraction for input and output, just like Herbs just have one kind of entity.

I don't see Herbs2GQL generating complex objects and using all the capabilities of GraphQL in the future.

italojs commented 1 year ago

i don't know how to use type as mutation params in graphql, I tried it but i always got errors like The type of Mutation.updatePrice(product:) must be Input Type but got: Product.

i remember I got this error some year ago and because of this we implemented using Inputs

my gqlConverter.js

const { checker } = require('@herbsjs/suma')
const { camelCase, upperFirst } = require('lodash')
const { BaseEntity } = require('@herbsjs/gotu/src/baseEntity')
const stringCase = require('./stringCase')

function requestFieldType2gql(type, presence, input) {
    let name
    if (Array.isArray(type))
        name = `[${requestFieldType2gql(type[0], false, input)}]`
    else if (type === Number)
        name = `Float`
    else if (type.prototype instanceof BaseEntity) 
        name = `${upperFirst(camelCase(type.name))}`
        // name = `${upperFirst(camelCase(type.name))}${input ? 'Input' : ''}`
    else
        name = stringCase.pascalCase(type.name)

    return presence ? `${name}!` : name
}

function usecaseRequest2gql(useCase, presence) {
    const fields = Object.keys(useCase.requestSchema)
    const output = []
    for (const field of fields) {
        const type = useCase.requestSchema[field]
        let name = requestFieldType2gql(type, presence, true)
        output.push(`${field}: ${name}`)
    }
    return output.join(`, `)
}

function usecaseResponse2gql(useCase, presence) {
    let name = requestFieldType2gql(useCase.responseSchema, presence, false)
    return name
}

function schemaOptions(options) {
    return Object.assign({
        presenceOnRequest: false,
        presenceOnResponse: false
    }, options || {})
}

function entityFieldType2gql(type, param) {
    let name
    if (Array.isArray(type)) name = `[${entityFieldType2gql(type[0], param)}]`
    else if (type === Number) name = `Float`
    else if (type.prototype instanceof BaseEntity) {
        name = upperFirst(camelCase(type.name))
        // if(param == 'input') name = `${upperFirst(camelCase(type.name))}Input`
    }
    else name = type.name
    return name
}

function entityField2gql(entity, param) {
    const fields = Object.keys(entity.prototype.meta.schema)    
    let gql = ""
    for (const field of fields) {

        if(typeof entity.prototype.meta.schema[field] === 'function') continue        

        const { type, options } = entity.prototype.meta.schema[field]

        let name = entityFieldType2gql(type, param)

        let typeOptions = fieldOptions2gpq(options)

        gql += `${field}: ${name}${typeOptions}\n`
    }
    return gql
}

function fieldOptions2gpq(options) {
    let optionsGql = ``
    const { validation } = options

    if (validation) validation.presence && (optionsGql += `!`)

    return optionsGql
}

function usecaseFieldToParams(useCase, schema) {
    const hasResponse = !checker.isEmpty(useCase.requestSchema)
    if (hasResponse) return `(${usecaseRequest2gql(useCase, schema.presenceOnRequest)}) `
    return ''
}

module.exports = {
    requestFieldType2gql,
    usecaseRequest2gql,
    usecaseResponse2gql,
    usecaseFieldToParams,
    schemaOptions,
    entityFieldType2gql,
    entityField2gql
}
dalssoft commented 1 year ago

Solved in beta. Planned for the next release