graphql-code-generator-community icon indicating copy to clipboard operation
graphql-code-generator-community copied to clipboard

fix: add empty object in place of variables for more consistent keys

Open rliljest opened this issue 1 year ago • 1 comments

Description

Addresses potential issues stemming from mismatched generated keys.

Related https://github.com/dotansimha/graphql-code-generator-community/issues/866

Type of change

Please delete options that are not relevant.

  • [X] Bug fix (non-breaking change which fixes an issue)

NOTE: Does have a fairly minor breaking change in case clients were hard-coding keys (not using generated helpers) and specifying "exact" react-query key matching

How Has This Been Tested?

Automated tests have been updated to accommodate this change

Test Environment:

  • OS: macOS
  • @graphql-codegen/typescript-react-query: 6.1.0

Checklist:

  • [X] I have followed the CONTRIBUTING doc and the style guidelines of this project
  • [X] I have performed a self-review of my own code
  • [X] I have commented my code, particularly in hard-to-understand areas
  • [X] I have made corresponding changes to the documentation
  • [X] My changes generate no new warnings
  • [x] I have added tests that prove my fix is effective or that my feature works
  • [X] New and existing unit tests pass locally with my changes
  • [x] Any dependent changes have been merged and published in downstream modules

rliljest avatar Nov 12 '24 19:11 rliljest

🦋 Changeset detected

Latest commit: 761b305b27179773c189184f7b5909107d182ddc

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
@graphql-codegen/typescript-react-query Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Nov 12 '24 19:11 changeset-bot[bot]

Is there anything that I'm missing to get this PR approved and merged? The issue that this resolves persists

rliljest avatar Sep 15 '25 12:09 rliljest

@TkDodo thoughts?

saihaj avatar Sep 15 '25 12:09 saihaj

looking at the linked issue, if you want to treat

useDummyQuery()

and

useDummyQuery({})

the same, I guess this change is what you want. A real-life example would be helpful though. Like, why isn't the argument for useDummyQuery required so that there is only one syntax to achieve the same thing?

This may be a breaking change if users were invalidating based on exact query key matches without using the .getKey helper, although that seems unlikely

agree with that sentiment, too.

TkDodo avatar Sep 16 '25 10:09 TkDodo

... why isn't the argument for useDummyQuery required so that there is only one syntax to achieve the same thing?

That would be another valid solution to the underlying issue. The main reason why I took the approach I did in this MR was to avoid breaking changes by forcing users to supply an empty object when they did not previously.

Another thing to consider is that currently in the codegen there is no differentiation between a query that takes no arguments vs. a query that takes no required arguments. So that approach would require users to provide an empty object on queries that do not take any arguments at all, which seems like an unnecessary breaking change

rliljest avatar Sep 22 '25 13:09 rliljest