Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ScrollLock dist missing window undefined check #3802

Closed
swensorm opened this issue Oct 4, 2019 · 15 comments · Fixed by #4423
Closed

ScrollLock dist missing window undefined check #3802

swensorm opened this issue Oct 4, 2019 · 15 comments · Fixed by #4423
Labels
issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet issue/reviewed Issue has recently been reviewed (mid-2020)

Comments

@swensorm
Copy link

swensorm commented Oct 4, 2019

While attempting to upgrade react-select from version 2.4.4 to the latest 3.0.8 I'm running into issues with server side rendering and the following error:

ErrorDetails : ReferenceError: window is not defined
    at ./node_modules/react-select/dist/base/dist/react-select-cac0a5ae.browser.esm.js (server.min.js:117809:17) -> var canUseDOM = !!(window.document && window.document.createElement);

We use this within a .Net CMS by way of ReactJS.Net so the window object is not available while it is being rendered on the server. The strange thing is that looking at the dist output from the npm package, the check for window being undefined isn't there:
image
However, looking at the src it is:
image

The check did exist in the dist output of the 2.4.4 version so something with the build process may have been changed with version 3.0.0 that could have caused this? Thank you for looking at the issue!

@krzysztof4M
Copy link

@swensorm Have you found any solution?

@swensorm
Copy link
Author

swensorm commented Oct 8, 2019

I have a workaround in place for the time being, but it's really not ideal as it renders nothing server side so there's a delay to it appearing in the browser. The exception would be thrown just from the static import, since the offending code isn't part of the component, so I have to dynamically require it.

if (typeof window === 'undefined') {
  return null;
}
const { default: Select, components } = require('react-select'); // eslint-disable-line global-require

@mgrzyb
Copy link

mgrzyb commented Feb 21, 2020

The bundle you use on the server should be built with webpacks target: 'node' to resolve modules from 'main' or 'module' fields and not using client-side specific aliases.

Since you're not running it under nodejs (ReactJS.NET does not provide full nodejs environment) you will get "require is not defined" when trying to use bundle built like this but there is away.

In the server build, keep target: 'web' (default when unspecified) but change resolve.aliasFields to an empty array, like this:

module.export = {
 ...
 resolve: {
    aliasFields: []
  }
}

This way webpack will not use browser specific aliases when resolving react-select module... Ufff... :)

@alverdal
Copy link

I was able to solve a similar issue with "window is not defined" when using SSR with reactjs.net by using your fix @mgrzyb .
Issue was described in ticket marcelltoth#8.

@swensorm
Copy link
Author

While I can confirm that adding aliasFields: [] to the webpack config did indeed fix the drop-down rendering server side, it had the unfortunate side effect of causing over 1 MB of locale data from the Intl package to be included as well.

@bladey bladey added the issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet label Jun 3, 2020
@rafalschmidt97
Copy link

While I can confirm that adding aliasFields: [] to the webpack config did indeed fix the drop-down rendering server side, it had the unfortunate side effect of causing over 1 MB of locale data from the Intl package to be included as well.

Same :)

@devstarman
Copy link

Why can't you check if windows is defined on canUseDOM() function?
I'd like to use some of hacks mentioned above but:

  1. I can't conditionally render this component because my SSR config is still going through all the code and encounters this canUseDOM method even in case when it's not rendered.
  2. I can't modify my webpack config due to project configuration (it's create-react-app based template and ejecting would cause much damage).

@ianks
Copy link

ianks commented Oct 19, 2020

In the code, it looks like there is a check for typeof window !== 'undefined'

However, in the compiled version I see:

// node_modules/react-select/dist/Select-9fdb8cd0.browser.esm.js:430s

var canUseDOM = !!(window.document && window.document.createElement);

Manually updating that code fixes the issue. Why does the code in the git repo differ from the built version on NPM?

Potential solution

For those looking for a potential fix, here are there solutions:

  1. Use the older version (v2.4.4)
  2. Add this to your webpack config (⚠️ I do not know all of the potential side effects of this):
  {
    resolve: {
      alias: {
        "react-select": require.resolve(
          "react-select/dist/react-select.cjs.prod.js"
        ),
      },
    },
  }

I think we are going to go with option 1...

UPDATE!

I found the culprit. Looks like preconstruct js is replacing things quietly:

https://github.com/preconstruct/preconstruct/blob/master/packages/cli/src/build/rollup.ts#L137

@AdnanTheExcellent
Copy link

AdnanTheExcellent commented Jan 12, 2021

Any headway on this? the alias and aliasFields method did not work for me when trying to upgrade to react-select 3.1.1. Still getting the

ERROR in SERVER PRERENDERING
Encountered error: "ReferenceError: window is not defined"

when trying to SSR

@ebonow
Copy link
Collaborator

ebonow commented Jan 28, 2021

Good news everyone! Looks like this behavior in preconstruct was fixed in version 2.0.2 which was released within the last 24 hours.
https://github.com/preconstruct/preconstruct/releases/tag/%40preconstruct%2Fcli%402.0.2

I will inquire about getting its dependency updated from 1.0 to 2.0.2 to resolve this as well as any other changes that might be required.

@Methuselah96
Copy link
Collaborator

Methuselah96 commented Feb 5, 2021

@mgrzyb's comment hits at the root issue and provides the best solution at the moment. See reactjs/React.NET#1017 for more context.

I don't believe that the fix to Preconstruct affects this issue because that fix is specific for UMD builds, and this issue is specific to non-UMD builds. Preconstruct is intentionally and correctly stripping typeof window checks in the browser build since it's assumed that window will be defined in a browser context.

I'm going to close this for now. I don't believe that there's anything else that react-select can do to improve the situation. This is more of a ReactJS.NET/Webpack problem than a react-select problem since we are correctly assuming that window is defined in a web/browser context. Feel free to comment if you think there's something that we can do to improve the situation and we will reopen the issue.

@Methuselah96
Copy link
Collaborator

Methuselah96 commented Feb 5, 2021

I'll leave this open until a permanent solution is found for purposes of visibility.

@swensorm
Copy link
Author

swensorm commented Feb 5, 2021

I do not agree with the assessment that the runtime environment not supporting a window facade is the root cause of the issue. I do, however, agree that the root cause is not directly react-select, but would say is in fact the preconstruct build tool. If preconstruct is "correctly stripping typeof window checks" then shouldn't it be stripping window.document checks as well since that will also always exist in a browser context? It is unfortunate when tools try to be too 'smart' for their own good. I would accept this issue being closed on this basis though.

It has been some time since I was in these codebases, and we're on a pretty old version, so perhaps just need to take another look at the build and split ssr from client or, as @ianks suggested, see if the non-browser cjs version works.

@Methuselah96
Copy link
Collaborator

Methuselah96 commented Feb 6, 2021

@swensorm I am in general agreement with what you're saying although I would have to do some further investigation into the issue to form a stronger opinion. I don't think that the root issue is Precontruct, I think it has more to do with not having a way to express which file can/should be used for SSR. I've made a PR to remove the browser alias fields since they provide no real benefit in our case.

@Methuselah96
Copy link
Collaborator

I've done some more research. Even if we remove the browser alias fields for react-select it still breaks with Emotion 11. See emotion-js/emotion#1246, emotion-js/emotion#1407, and emotion-js/emotion#1728 for more details. This is a fundamental issue with ReactJS.NET and other SSR libraries that configure their Webpack config to target web.

If we are able to do anything on our end that would fix the problem, we would do it, but at this point Emotion would need to change first (which doesn't seem likely given the above issues).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
issue/bug-unconfirmed Issues that describe a bug that hasn't been confirmed by a maintainer yet issue/reviewed Issue has recently been reviewed (mid-2020)
Projects
None yet
Development

Successfully merging a pull request may close this issue.