remove custom logger

[TkDodo]

context

In v3, we had a global logger that you could set via setLogger to your own logging mechanism. This was a side effect which we tried to get rid of, so in v4, you can now pass a logger prop to the QueryClient.

The logger mainly does one thing: It logs failed queries to the console. This is fine for development, but it was confusing to many that it also showed up in production.

That's why we've removed all logging in production in v4, and we've also started to use the logger more to show development specific warnings, for example:

• when you pass a queryKey that isn't an Array.
• when you return undefined from your queryFn.
• when you set multiple, conflicting query defaults.

Those logs are meant to help developers find potential issues in their code. They are not meant to be shown in production. I'd see them on the same level as the warning you get from React in development mode, for example:

• Each child in a list should have a unique "key" prop.

There is also no way to show these warnings in production. This also helps with bundle size because all logging happens behind an env check, so it is stripped from the final bundle. This means we can be as verbose as we want with those messages.

---

All of this means that the custom logger is kind of unnecessary. Why would you want to pass a custom logger only to log differently in development mode? The logger prop itself cannot be tree-shaken, so it will always be included in the final bundle even though we actually never use it.

proposal

• remove the logger prop from QueryClient
• do not log failed queries to the console anymore, as failed queries are not a programmer "error" that we need to point people towards. The network tab shows failed requests just fine.

We should be able to just use the console for those development logs as it exists in all environments.

[saurabhan]

You'd like to remove the logger prop from the QueryClient, but keep the defaultLogger functionality for logging programmer errors, right? just a quick question would returning defaultLogger from getLogger() function suffice or should I refactor the call everywhere?

[TkDodo]

I would replace the logger completely and just use console.log instead. All logging should happen behind env checks.

[saurabhan]

Got it.

[TkDodo]

do you want to work on this @saurabhan ?

[saurabhan]

Yes, I'd like to help with this.

[bjesuiter]

How does the logging env check work? for example, when I'm running a npm module in deno the required envs might not be there as expected.

Or another Szenario: running on deno deploy and imported via esm.sh

[TkDodo]

we already have env checks around each log statement, e.g.:

https://github.com/TanStack/query/blob/48612006306fd9006aacc7cd4b9cf97b68dcbcf5/packages/query-core/src/query.ts#L455-L459

Is that troublesome for the environments you've mentioned?

[LauSam09]

We've just upgraded to v4, and were originally using the v3 setLogger API to suppress console errors for during our tests.

Ironically, doing the equivalent in v4 achieves the same, but generates errors about the API being deprecated instead. We'd rather not suppress all console errors as they're often useful, but if we're explicitly testing failure cases, then we don't want the expected console errors to pollute our test runs.

Should I raise a separate issue for this, or is there a workaround? Thanks.

[redtogin]

Do I assume correctly that the production check with process.env will not work in a React app built with Vite, because Vite uses import.meta.env? Or has this maybe been addressed already? Apologies if it has been.

If it hasn't, perhaps QueryClient could have an isProduction option param, to override the default and accommodate different build tools?

(My use-case behind the question: I need to avoid logging valid errors to the console because that is picked up by Sentry, and I see now that the logger is deprecated.)

[TkDodo]

We've always had checks against process.env.NODE_ENV. If they wouldn't work for a specific bundler, I had hoped that someone would have reported that by now.

[jcready]

It probably doesn't outweigh the benefits, but I'd like to point out one benefit of the old logger: I could force unit tests to fail which had query issues by passing:

{
    error: (e) => {
        if (e instanceof Error) {
            throw e;
        }
    }
}

Now I will need to mock console.error() to throw instead which affects more than just the query issues. Several libraries use console.error() to warn about deprecations (looking at you @tanstack/query).

[TkDodo]

I think its good practice to fail every test when there's a console.error. In v5, we're only using console.error if there is something that you should address immediately, like a wrong query key or undefined being returned from your queryFn, which is not allowed.

React does the same thing by warning you about missing keys on list items, or when duplicate keys are encountered. Both are real issues in your code, so its fine to console.error and thus fine to fail your test.

So if you have that setup, you can just as easily do:

const testQueryClient = new QueryClient({
  queryCache: new QueryCache() {
    onError: (e) => {
      console.error(e)
    }
  }
})

so the console statement would make your test fail.

Several libraries use console.error() to warn about deprecations

good call about that, I think this violates what I just said, and we should rather use console.warn in the future for that.

[jcready]

Right, my point is that many libraries (not just this one) use console.error() to warn about deprecations (e.g. things that explicitly don't need to fail my tests).