-
Notifications
You must be signed in to change notification settings - Fork 28
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
feat(enhancement): Upgrades generated graphql nexus file with more CRUD examples #174
Conversation
…D examples and proper Input Types
9e79d63
to
52dbd69
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looking great. Just a few minor questions
// export const UserRole = enumType({ | ||
// name: 'Role', | ||
// members: Object.values(Role), | ||
// }); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
it feels like we might delete this most of the time. I definitely see the value in outlining the Object.values
example, but I did want to ask if you think this falls into the "you'll need this often" category.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Agreed.
It is not an everyday thing, probably something better suited for Docs over a generated file.
Will opt to remove here.
// Database Specific Fields | ||
t.nonNull.id('id'); | ||
t.nonNull.date('createdAt'); | ||
t.nonNull.date('updatedAt'); | ||
// Model Specific Fields | ||
t.nonNull.string('name'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the idea of trying to organize some of this. I worry that doing it this way may actually confuse things though. It requires you to ask yourself "is this a database field or a model field". Additionally, the name
field that you have here doesn't have a custom resolver so it looks like it would be coming from the database.
Perhaps it would be better not to force the organization and just show an example of a custom resolver for a field?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup, those comments are a bit misleading.
name
is in our generated example.
I'll remove the comments and leave it as is.
t.list.field('<%= plural.toLowerCase() %>', { | ||
type: '<%= camelized %>', | ||
t.field('<%= plural.toLowerCase() %>', { | ||
type: list('<%= camelized %>'), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
correct me if I'm wrong but I think both of these are equivalent. I feel t.list.field
is slightly more straightforward due to the intellisense autocompletion and not requiring an additional import of the list function. thoughts?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We made this change when troubleshooting the other day.
It is the same...
I'll revert and save an import. 👍🏼
@@ -25,21 +33,32 @@ export const <%= camelized %>Queries = extendType({ | |||
type: 'Query', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
while you are here, thoughts on moving this to queryField
instead of extendType
? Not required, but it does make it a bit more concise.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Going to leave this one for another ticket.
We'll want to refactor to use query-field
and mutation-field
throughout in our user
module as well to stay consistent.
I'll need to look at that syntax a bit, it looks like each piece would become its own exported queryType
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome!
Description
Upgrades generated graphql nexus file with more CRUD examples and proper Input Types
Changes
GraphQL Nexus template was lacking, had bogus "SOMETHING" and User definitions.
Now generates:
Base Module
Query
thing
w/ user logged in guardQuery
list('thing')
w/ user logged in guardMutation
createThing
w/isAdmin
guardMutation
updateThing
w/IsAdmin
guardThingWhereInput
ThingWhereUniqueInput
ThingCreateInput
ThingUpdateInput
ThingOrderByInput
Generating a new app works
closes #173