useIsomorphicLayoutEffect flag set wrongly

[patricksmms]

What is the current behavior?

useIsomorphicLayoutEffect is set to true when running on a Node.js instance, where the window object is defined.

This is a common scenario when you need to emulate a subset of a web browser for tests or DOM manipulation, then you define the object perhaps with JSDom.

It's resulting in console warnings and issues when hydrating the SSR code.

What is the expected behavior?

useIsomorphicLayoutEffect flag is set to false when running in Node.js, always, not relying only on the window object being undefined.

Ref: https://github.com/reduxjs/react-redux/blob/master/src/utils/useIsomorphicLayoutEffect.js#L12

Used versions

[email protected]
[email protected]

This used to work without issues, in [email protected].

[markerikson]

Can you clarify when you are seeing this under Node? Is this specifically in a Jest test environment or similar?

[patricksmms]

I'm emulating the browser env in Node to be able to run DOMPurify and other libs in SSR:

  const { JSDOM } = require('jsdom');

  const DOM = new JSDOM('', { pretendToBeVisual: true });

  global.document = DOM.window.document;
  global.window = DOM.window;

What I did on the linked commit removes false positives for cases like the exemplified above.

[markerikson]

It's sad just how specific we're having to feature-detect this.

Yeah, can you file a PR to add that change here?

[patricksmms]

I made it less specific now, just adding the check for the property innerWidth. I'm assuming that this property is always set on real browsers, but not in emulated environments.

[timdorr]

If you're defining window as a global, that's going to potentially affect a large number of libs, not just React Redux. I don't think what you're doing is the "correct" way of creating a virtual DOM for your libraries.

Over time, JSDOM is going to get closer and closer to the DOM spec, so things like the innerWidth value you've chosen will be implemented and fail as a check for our library. My biggest concern here is getting in an "arms race" over JSDOM's spec implementation and our detection code. That is a losing battle for us, long term.

Is it reasonable to make your code not rely on window being global? Can you create a window instance in a module that will ponyfill where it's needed? That seems more sensible for your codebase anyways, and less likely to cause problems elsewhere.

[patricksmms]

@timdorr I agree with you and thanks for the call out! I'm already doing some refactoring on the app.

[patricksmms]

Closing the issue

[sanbornhilland]

If you're defining window as a global, that's going to potentially affect a large number of libs, not just React Redux. I don't think what you're doing is the "correct" way of creating a virtual DOM for your libraries.

Over time, JSDOM is going to get closer and closer to the DOM spec, so things like the innerWidth value you've chosen will be implemented and fail as a check for our library. My biggest concern here is getting in an "arms race" over JSDOM's spec implementation and our detection code. That is a losing battle for us, long term.

Is it reasonable to make your code not rely on window being global? Can you create a window instance in a module that will ponyfill where it's needed? That seems more sensible for your codebase anyways, and less likely to cause problems elsewhere.

I arrived here with the same question as @patricksmms. The above post is a partially reasonable response in my view but I'm confused habout what the intended "correct" way of creating a virtual DOM is. Jest sets up a JSDOM environment with a window object by default. How are we proposed to get around these warnings in these kinds of test environments? Taking away the window object doesn't seem like the correct path. To be clear I fully recognize that this is not a problem originating in react-redux so the answer could very well be 🤷, not our problem. But given that it's a widely used library and there has been some discussion here I figured I'd follow up here first.