-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Split large unions over multiple lines #10219
base: master
Are you sure you want to change the base?
Conversation
|
Hi @benjie , would there be a scenario where the existing behaviour (i.e. the equivalent of |
@eddeee888 Thanks for asking! I would 100% be in support of adding this without a config option. I don't know why anyone would want it all on one line, I was just trying to minimize codegen diff for people who didn't opt in. |
It's going to be a little while before I can make that change - feel free to take over, or leave it for me in a few weeks time 👍 |
Any change needs to be discussed before proceeding. Failure to do so may result in the rejection of the pull request.
✔️ Acknowledged. Think of this as the issue, to open the discussion via demonstration.
Please ignore whitespace changes when reviewing this PR.
Description
To make
git
management of changes easier, it's nice if lines aren't too long. Some of the lines emitted by graphql-code-generator can be excessively large which results in hard to review diffs. This PR aims to introduce newline splits in a few more places to make changes more surgical.Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Tests have been updated.
Test Environment:
master
branchChecklist:
Notes:
TypeScript GraphQL Request tests › works without variables
andTypeScript GraphQL Request tests › returns first 3 entries
tests fail locally for me onmaster
. I have not investigated.Further comments
I've currently gated this behind the
printFieldsOnNewLines
setting which seems related; I don't know if you'd want separate settings for each change, or a more generaluseNewlinesForUnions
setting, or...?This is also not an exhaustive change that adds newlines to all unions.