add eslint-plugin-no-typeof-window-undefined

[nirtamir2]

This PR adds eslint-plugin-no-typeof-window-undefined eslint plugin to prevent typeof window === "undefined" because Deno supports window (so this won't check well if the code is in browser / server-side) and replace it with typeof document === "undefined"

I run pnpm test:eslint --fix and it auto-fix some places the code uses type window === "undefined", so make sure the conversion in those places works for you.

I'm following this tread https://twitter.com/TkDodo/status/1598227778435948545

Notice

I run test and one of the tests (should not use window to get the context when contextSharing is true and window does not exist) failed, so I fix it by simulating undefined document (like another place in code does).
I found 2 more test cases with the same situation but they pass. (look for // Mock a non web browser environment in code) I'm not sure if it's better to use the same approach with deleting the document from globalThis and return it in the end of the test. Currently I did not touch them.

[nirtamir2]

Not sure if it's better to specify this plugin config (with a single rule) or specify it in plugins and enable this rule specifically.

[codesandbox-ci]

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit 43d54a1:

Sandbox	Source
@tanstack/query-example-react-basic-typescript	Configuration
@tanstack/query-example-solid-basic-typescript	Configuration
@tanstack/query-example-vue-basic	Configuration

[nirtamir2]

This test failed after my changes with

 TypeError: Cannot read properties of undefined (reading 'ReactQueryClientContext')

here

query/packages/react-query/src/QueryClientProvider.tsx

Line 32 in 5ebc68a

if (!window.ReactQueryClientContext) {

So I simulate undefined document like here

query/packages/query-core/src/tests/focusManager.test.tsx

Line 59 in 5ebc68a

delete globalThis.document

I could do it in another 2 places (but the test passed). Search for // Mock a non web browser environment in the code

[TkDodo]

I don't think the changes made by this eslint rule are correct here. Why should we check for the existence of document if we later access things on window., for example:

if (typeof document !== 'undefined') {
  return window.matchMedia(query).matches
}

reading that code makes no sense to me 😅

[nirtamir2]

Hi,
Thanks for the quick response.
I created this eslint plugin because I thought we could not depend on window existence to check if we are in a browser environment.
It seems like it's a thing we have to get used to, although less intuitive.

From a TypeScript perspective, the check for typeof window !== "undefined" narrow the window type, if it could be undefined.
But window's type is Window & globalThis and cannot be undefined in this scope regardless of the check.

Why should we check for the existence of document if we later access things on window.

Because I believe the intent is to check if we are in a browser environment. It won't narrow window type in this case. If so, checking typeof document === "undefined" would be more correct (in environments like Deno).

What do you think is the best approach to handle this case? I can:

• Revert the code changes and run the lint rule in specific places (with eslint's overrides)
• Revert the code changes and write // eslint-diable-next-line eslint-plugin-no-typeof-window-undefined comments
• Use isServer from @tanstack/query-core or a similar function/variable that just encapsulates the typeof document !== "undefined"

What do you think?

[TkDodo]

I think for code in the devtools, we can simply remove the window check. devtools don't render on the server, and the code in useEffect is not executed on the server at all.

However, we are getting some bug reports that intervals are not running in react-native since we merged this. Are you sure that typeof document returns true in react native environments?

see:

• refetchInterval not working #4584

[nirtamir2]

For react native:
typeof document === "undefined" return true
typeof window === "undefined" return false

[TkDodo]

What would be a good way to check if you're in a react native browser environment other than checkout for window? Server environments like deno have window defined, but not document...

[TkDodo]

gonna close this because of the react-native issue:

• fix(query-core): revert isServer check to check for window #4592

I don't think checking for document is a good idea anymore :)