marmelab/react-admin

ra-data-graphql: faulty deprecation warning regarding `override`

Closed this issue · 5 comments

What you were expecting:
I can invoke buildGraphQLProvider(myProviderOptions) without seeing a warning that I shouldn't use the override option (which I'm not using as far as I understand).

What happened instead:
I see the warning: "The override option is deprecated. You should instead wrap the buildQuery function provided by the dataProvider you use.

Steps to reproduce:
Invoke buildGraphQLProvider with some provider options without the override property.

Related code:

buildGraphqlProvider({
    buildQuery: () =>
      (
        operationType: string, 
        resourceName: string,
        params: unknown, 
      ) => {
        switch (operationType) {
          case 'GET_LIST':
            return ...;
          case 'GET_MANY':
            return ...;
          case 'GET_ONE':
            return ...;
          case 'GET_MANY_REFERENCE':
            return ...;
          case 'CREATE':
            return ...;
          case 'UPDATE':
            return ...;
          case 'UPDATE_MANY':
            return ...;
          case 'DELETE':
            return ...;
          case 'DELETE_MANY':
            return ...;
          default:
            throw new Error(`Operation ${operationType} is not supported`);
        }
      },
  };
)

Other information:
Since I had no idea what the warning was refering to, I searched the code and found this line. My impression is that the warning is logged b/c override is defaulted to {} a couple of lines above and therefore is not undefined in the check.

Environment

  • React-admin version: 5.0.4
  • Last version that did not exhibit the issue (if applicable): I think I didn't see it in 4.x
  • React version: 8.3.1
  • Browser: Firefox, but it happens in any browser and even logged at the server-side

Thanks for the report. I confirm there seems to be a bug at the line you pointed.

Would you like to open a PR to replace the override check by something like checking Object.keys(override).length?

Thanks

Thanks for the report. I confirm there seems to be a bug at the line you pointed.

Would you like to open a PR to replace the override check by something like checking Object.keys(override).length?

Thanks

Sure, I can give it a shot the next days!

As I can see the solution have been merged. So the issue needs to be closed right? Or does it needs further enhancement?
If yes, I am willing to contribute to this issue

As I can see the solution have been merged. So the issue needs to be closed right? Or does it needs further enhancement? If yes, I am willing to contribute to this issue

Oh, you're right! I should close it then. Yes, it's merged but I think not released yet.

Indeed I forgot to close the related issue, thanks guys! ❤️
I'll release it in version 5.1.2 today!