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.

Sign up for GitHub

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

Jump to bottom

isOldIE throws 'window is not defined' in non-browser environments #272

Closed
JoshuaKGoldberg opened this issue Nov 10, 2017 · 7 comments
Closed

isOldIE throws 'window is not defined' in non-browser environments #272

JoshuaKGoldberg opened this issue Nov 10, 2017 · 7 comments

Comments

@JoshuaKGoldberg
Copy link

I'm using React server-side rendering in React.NET and compiling my app with webpack+style-loader+etc. isOldIE assumes window as a variable exists and can be falsy. The correct way to check for its existence would be typeof like in lib/urls.js.

ReferenceError: 'window' is undefined at Anonymous function (Script Document:700:1)
 at Anonymous function (Script Document:695:111)
 at module.exports (Script Document:702:23)
 at srcViewsRootStylesCss (Script Document:984:1)
 at __webpack_require__ (Script Document:50:9)
 ...
@JoshuaKGoldberg JoshuaKGoldberg mentioned this issue Nov 10, 2017
Closed
@indolent-developer
Copy link

Guys any update on this. do we have any PR for this. This should be simple one line change.

@alexander-akait
Copy link
Member

@michael-ciniawsky friendly ping

@michael-ciniawsky
Copy link
Member

michael-ciniawsky commented Nov 17, 2017 •
edited
Loading

@indolent-developer Describe your use case please and provide {Error} logs and your webpack.config.js

@indolent-developer indolent-developer mentioned this issue Nov 18, 2017
Open
@indolent-developer
Copy link

@michael-ciniawsky i have reference this in library where we see issues. And regardless of that the code is not correct. Dont you agreee? The window checks are different in code else where.

@michael-ciniawsky
Copy link
Member

@indolent-developer Then please provide more info about this issues :). Let's say I don't disagree but the checks seem to work and unless there isn't a confirmed bug within the scope of this loader there is less reason to change anything

@JoshuaKGoldberg
Copy link
Author

tl;dr: The check does not work and that doesn't matter.

As per the OP, you can't check for existence of a global variable by seeing if it's truthy (which is what the check does). Just checking if (window) will throw an error. You have to use if (typeof window !== "undefined"). The same goes for if (document) verses if (typeof document !== "undefined").

However, it's irrelevant whether this check is fixed, because style-loader relies on the existence of a document elsewhere in code. See lib/addStyleUrl.js. Merging in the linked PR still wouldn't get style-loader to work with server-side rendering (or more genera, execution in a non-browser environment).

michael-ciniawsky's comments in the PR are why this issue should be closed:

style-loader requires a DOM and doesn't officially support SSR

For SSR specifically:

please use an proper alternative instead e.g isomorphic-style-loader. style-loader solely adds CSS via <style> to the DOM

@chardos
Copy link

chardos commented Dec 7, 2017 •
edited
Loading

Thanks for the replies peeps.

What about the use case where a bundle includes both DOM dependent modules, and non-DOM dependent modules? I have a "component library" that has a bunch of react modules being compiled, but also a config file that needs to be accessed by DefinePlugin in another module. In my case, fixing the check would allow me to require just the single file I need.

Can we implement this fix?

@Pixelatex Pixelatex mentioned this issue Aug 23, 2018
Closed
@fbaiodias fbaiodias mentioned this issue Jun 19, 2019
Open
dbmikus added a commit to supercool-labs/pixel-art-react that referenced this issue Nov 14, 2021
@dbmikus
Don't use style-loader in devel Webpack config
b0fc1f2 
Don't use style-loader in devel Webpack config because it always
references `window` which breaks Node.js pre-rendering of React
components.

webpack-contrib/style-loader#272
dbmikus added a commit to supercool-labs/pixel-art-react that referenced this issue Nov 14, 2021
@dbmikus
Modify Webpack to build and expose package as a library
58243ec 
Modify Webpack to build and expose package as a library. Most
importantly, we specify the Webpack config's `library` and
`libraryTarget` fields to export the code as as UMD library. We also set
Webpack `output.globalObject` to `this` so that the code can run in both
Node.js for pre-rendering and in the browser.

Reconcile some differences between the prod and development Webpack
config. I'm not really sure why these differences were in place, but
some of them were causing problems. Don't use style-loader in devel
Webpack config because it always references `window` which breaks
Node.js pre-rendering of React components. [1]

Allow the babel-loader to run on pixel-art-react for when this is
imported as a library. If we just excluded it from running on any
`node_modules` it would not process the React code inside
`pixel-art-react` when `pixel-art-react` is installed into the
importer's `node_modules` folder.

[1]: webpack-contrib/style-loader#272
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

fix(addStyles): allow window && document to not exist (isOldIE)
5 participants
@indolent-developer @JoshuaKGoldberg @chardos @alexander-akait @michael-ciniawsky