Skip to content

Conversation

N-Olbert
Copy link
Contributor

@N-Olbert N-Olbert commented Aug 7, 2025

Summary of the changes (Less than 80 chars)

  • Considerd descriptor.Name("...") and [GraphQlName("...")] for ID<Type>
  • This is a breaking change

Closes #6256

@N-Olbert
Copy link
Contributor Author

N-Olbert commented Aug 7, 2025

Related: #6264

@michaelstaib @glen-84 : Did I miss something here? I considered fluent type definition naming and the [GraphQLName] attribute by using the name of the referenced ObjectType after it was created. I couldn’t think of anything else at the moment...

@michaelstaib michaelstaib added this to the HC-16.0.0 milestone Aug 28, 2025
@michaelstaib michaelstaib requested a review from glen-84 August 28, 2025 06:36
@michaelstaib michaelstaib marked this pull request as draft August 28, 2025 06:36
@michaelstaib michaelstaib self-requested a review September 1, 2025 13:25
@N-Olbert N-Olbert requested a review from glen-84 September 1, 2025 18:04
@N-Olbert
Copy link
Contributor Author

N-Olbert commented Sep 1, 2025

I applied the changes from the review, thanks 👍

Regarding the docs: Should this change be mentioned in the migration guide from v15 to v16? I could add it in another PR. The generic ID attribute isnt very popular in the current docs, so maybe that could be reworked as well.

@glen-84
Copy link
Collaborator

glen-84 commented Sep 2, 2025

Thanks @N-Olbert

Looks good from my side (just one more indentation issue), but @michaelstaib will do a final check after GraphQLConf.

Regarding the docs: Should this change be mentioned in the migration guide from v15 to v16?

Yes, all breaking changes should be mentioned in the migration guide. A separate PR would be great.

The generic ID attribute isnt very popular in the current docs, so maybe that could be reworked as well.

I'll let Michael comment on this one. 🙂

@glen-84
Copy link
Collaborator

glen-84 commented Sep 3, 2025

@N-Olbert The indentation is still incorrect. 😁 It should be 4 spaces, not 3.

@N-Olbert
Copy link
Contributor Author

N-Olbert commented Sep 3, 2025

@glen-84 meeh, sometimes the simplest things seem to be the hardest 😃

This is what I get if I let my IDE (Rider with the correct .editorconfig) reformat the code:

        // act
        var result =
            await services.ExecuteRequestAsync(
                $$"""
                  mutation {
                     validAnyIdInput1: acceptsAnyId(input: { id: "{{userId}}" }) { int }
                     validAnyIdInput2: acceptsAnyId(input: { id: "{{fooId}}" }) { int }
                     validAnyIdInput3: acceptsAnyId(input: { id: "{{fluentFooId}}" }) { int }
                     validAnyIdInput4: acceptsAnyId(input: { id: "{{singleTypeFluentFooId}}" }) { int }

                     validUserIdInput: acceptsUserId(input: { id: "{{userId}}" }) { int }
                     validFooIdInput: acceptsFooId(input: { id: "{{fooId}}" }) { int }
                     validFluentFooIdInput: acceptsFluentFooId(input: { id: "{{fluentFooId}}" }) { int }
                     validSingleTypeFluentFooIdInput: acceptsSingleTypeFluentFooId(input: { id: "{{singleTypeFluentFooId}}" }) { int }
                  }
                  """);

Following your last review, I moved the entire raw string two spaces to the left to make it align with the start of the $$ (which is different from how the IDE formats it by default). I had missed that you were actually referring to the spacing inside the mutation itself. Hopefully I’ve got it right this time 🙃

@glen-84 glen-84 marked this pull request as ready for review September 19, 2025 12:18
@michaelstaib michaelstaib added the 🍒 cherry-pick Consider cherry-picking these changes into the previous major version. label Sep 28, 2025
@michaelstaib
Copy link
Member

michaelstaib commented Sep 28, 2025

@glen-84 was this one ready?

@glen-84
Copy link
Collaborator

glen-84 commented Sep 28, 2025

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🍒 cherry-pick Consider cherry-picking these changes into the previous major version. 🌶️ hot chocolate
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Use GraphQLName when referencing type with ID<Type>
3 participants