-
Notifications
You must be signed in to change notification settings - Fork 60
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
feature: Add suffix to schema type filenames #580
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for eclectic-pie-88a2ba ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
✅ Docs preview readyThe preview is ready to be viewed. View the preview File Changes 0 new, 1 changed, 0 removed
Build ID: 9d04abe4e703ef5acab42d0d URL: https://www.apollographql.com/docs/deploy-preview/9d04abe4e703ef5acab42d0d |
✅ Deploy Preview for apollo-ios-docc canceled.
|
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.
Just one issue to be resolved with the way the editable generated files are handled.
We also need to add this new option to the documentation (not just the generated docs).
@@ -0,0 +1,10 @@ | |||
// @generated |
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.
The old versions of these scalar files need to be deleted. But this actually surfaces a bigger issue here.
If a user had this new config false
and then sets it to true, previously generated custom scalars (and any other files that are marked to not be overwritten) will not be handled properly. We need to preserve the changes to the code the user may have made. So we actually need to detect the old file name and rename the existing file, rather than just writing a new file.
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.
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 think we need tests that validates that when this is set to true
that is reads it and writes it to the config properly.
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.
Done in 34fea8b.
This PR already updated the codegen configuration documentation - Here it is in the docs preview for this PR. Is there another place in the docs to update? I'll take a look at the other two review comments now.. |
Fixes apollographql/apollo-ios#2598.
Users now have the codegen configuration option (
appendSchemaTypeFilenameSuffix
) to have a suffix added to their schema type filenames. This will help resolve problems in Xcode where the name of a schema type and operation type are the same causing a build error.This new config option has a default value of
false
which means this should have no effect on users without the filename conflict error.