Support GraphQLID

[omarchehab98]

Closes: #187

Summary:

Adds support to specifying a field being of type GraphQLID. Add format: id or format: uuid.

Test Plan:

1. Change directory to packages/openapi-to-graphql
2. Run npm run start_dev
3. Browse to localhost:3000/graphql
4. Click on "Docs"
5. Click on "Query"
6. Observe the type of properties whose names end with "id"

Before:

company(id: String!): Company
office(accept: Accept, id: Int!): Office
productWithId(productId: String!, productTag: String!): ProductWithId
productsReviews(id: String!, limit: Int, productTag: String): [ProductsReviewsListItem]

After:

company(id: ID!): Company
office(accept: Accept, id: ID!): Office
productWithId(productId: ID!, productTag: String!): ProductWithId
productsReviews(id: ID!, limit: Int, productTag: String): [ProductsReviewsListItem]

7. Click on "Company"
8. Observe the type of "id"

Before:

id: String
The identifier of a company

After:

id: ID
The identifier of a company

9. Click "Back" twice
10. Click on "Mutation"
11. Click on "ProductWithIdInput"
12. Observe the type of "productId"

Before:

productId: String
The id of the product

After:

productId: ID
The id of the product

[ErikWittern]

@omarchehab98 Thank you so much for this PR, very interesting proposal! We are not perfectly familiar with the way that id / uuid formatting works in OpenAPI / JSON schema. Maybe you can help us?

For one, you make a check for format being uuid, without also checking for the type. E.g., your implementation allows to combine a number type with a format of uuid. See for example here in the commit. Is it even possible that a number-typed value has a uuid format?

Furthermore, we couldn't find reference to the id format present in your implementation. What constraints does this format imply, and for what types can it be defined? Maybe you have a pointer to more information?

[omarchehab98]

@ErikWittern format id and uuid are undefined by the openapi spec. format uuid was given as an example of undefined formats that may be used.

Since we know uuid follows a strict format, I also wanted to a more generic format. Thus I chose another undefined format called id. It follows no format, we can let the user define it.

I agree with your concern about format uuid, I can amend that.

Finally, regarding format id, I don't plan on adding any constraints. It seems like the tests didn't like it being a number but I recommend we explore that further. Perhaps it's good that we enforce it being a string because of number precision variation in different languages and computers and given that it is generally not helpful to do arithmetic on an id with the exception of hashing.

[omarchehab98]

Please feel free to take the lead from here, I don't mind if you jump in, make changes, and merge it

[Alan-Cha]

@omarchehab98 Thanks for the quick response! And thanks again for filing the issue and the very detailed PR, all highly appreciated.

I think we need to enforce that the ID field should be a string. Your logic regarding number precision variation makes a lot of sense. Also, the ID scalar in GraphQL expects the JSON value to be a string.

However, I do not think we should add our own custom id format. Instead, I believe we should create a new option, that allows the user to describe format types that should be interpreted as ID types. Otherwise we would embrace a convention that may not be shared by all our users.

---

I can fix the tests and help amend the commit as soon as I am free.

[omarchehab98]

Is this PR on your radar to merge?

[Alan-Cha]

Yes! I think we should! I made some changes and added test cases since the last time we spoke. We are still missing the option to declare other ID formats (and exposing it in the CLI) though.

[ErikWittern]

@omarchehab98 @Alan-Cha Great work, thank you!