-
Notifications
You must be signed in to change notification settings - Fork 336
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
chore(SDL): refactor _legacy.graphql into individual typeDefs #10019
Conversation
Signed-off-by: Matt Krick <[email protected]>
This PR exceeds the recommended size of 1000 lines. Please make sure you are NOT addressing multiple issues with one PR. Note this PR will be delayed and might be rejected due to its size. |
@@ -0,0 +1,2018 @@ | |||
type Mutation { |
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 liked the previous way it was split into multiple files better: put each mutation in its own file and add the related return and input types there too. The second part of this might be a bit harder to automate, but then you have all related types in one file and don't need to jump between them too often.
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 was definitely nice grouping them together & having fewer files!
The question I kept asking myself is, "why are we treating the Mutation object differently from other objects?"
If the mutation returned a User
object, we wouldn't put the User
definition in there, we'd give it its own file because it's shared across definitions.
Using that same logic, UpdateMeetingPromptSuccess
type is shared with the Subscription
object, so why does that get grouped with in the same file as the mutation field?
Moving forward, I'm down to keep using out extend
pattern! TBH I don't really care where the defs are stored, I just use search anyways. I just wanted to get rid of the stale definitions in _legacy.graphql
that were holding us back.
How should we proceed? Could we merge this now, or should we refactor manually?
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's just a preference. I don't see a reason why we shouldn't merge it as is. Better than manually moving types out of _legacy.graphql
.
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.
Love this!
Signed-off-by: Matt Krick <[email protected]>
Description
Refactoring into the SDL pattern has been a pain because we need to remove the typeDef from the _legacy.graphql file & put it into its own typeDef. We do this because it's 8000 LOC & ripe for merge conflicts.
I wrote some code that did this for us for every type.
Now refactoring to the SDL pattern just requires transforming the resolve statements, no need to touch .graphql files 🎉
How is it broken out?
Mutation.graphql