Jss: Change browser check

[AlastairTaft]

At the moment you do a browser check using the is-browser. However when I compile the code with webpack I'm forced to pick either the server or the client when it builds the bundle so running the code isomorphically makes the check useless. Unless I'm mistaken?

Could we do a browser check like the following instead?

if (typeof document !== 'undefined')
  console.log('in browser')
else
  console.log('not in browser')

[kof]

What exactly is the problem you are trying to solve?

[kof]

I don't see why you are forced to pick anything and by whom. Webpack knows what to take automatically.

[AlastairTaft]

I'm getting this error on the server side

~/css-vendor/lib/prefix.js:29
  var style = document.createElement('p').style;
^
ReferenceError: document is not defined

UPDATE: Just realised this is to do with the vendor prefix in the default preset. About to look into if that package checks if it's in the browser or not, if it does, then the below question still stands, if it doesn't then that's my problem and I'll have to guard against it.

So my webpack server bundle compiles down to this.

var _isBrowser = __webpack_require__(916);
...
/* 916 */
/***/ function(module, exports) {
  module.exports = true;
/***/ }

Which makes it always think it's in a browser environment and throws when it tries to add a style to the document. In my webpack config I have target: 'node' but it seems to have no effect on the isBrowser check.

My project where I'm compiling my webpack bundle depends on my custom components library and it's this library that depends on jss. Perhaps I've misconfigured webpack.

Or would changing the browser check to a runtime check for document be an option?

[kof]

The question here is why your webpack using the browser environment if you are building on the server?

[AlastairTaft]

I'm not sure, as I thought target: 'node' should solve that. Will look further into it.

[kof]

Actually you shouldn't need to do anything special, thats how is-browser works with browserify/webpack.

[AlastairTaft]

I suspect it's something to do with configuring externals so that the is-browser package doesn't get bundled, but I'm using a mono-repo setup so adding it as an external causes it to not exist. Will keep looking into it but I suspect this probably isn't a jss specific issue. Thanks for your help.

[kof]

Yeah looks like target: node should solve that https://webpack.github.io/docs/configuration.html#target.

But actually why do you use webpack for serverside? You can pre-build everything into cjs and then just use regular node's require.

[AlastairTaft]

Solved, came back at it fresh this morning. It was to do with externals. So the important thing is is to tell webpack not to bundle the 'is-browser' package, which means 'is-browser' has to be in externals.

What I did was exclude all the packages in my server config like so.

var nodeModules = {};
fs.readdirSync('node_modules')
  .forEach(function(mod) {
    if (mod.startsWith('@')){
      // Scoped package need to look inside it
      fs.readdirSync(`node_modules/${mod}`).forEach(function(innerMod){
        nodeModules[`${mod}/${innerMod}`] = `commonjs ${mod}/${innerMod}`
      })
    } else {
      nodeModules[mod] = 'commonjs ' + mod;
    }
  });

// Webpack server config
Object.assign(config, {
  ...
  target: 'node',
  externals: nodeModules,
  ...
})

And good point, maybe I don't need webpack server side. I guess the only grey area I'm unsure about is what happens when you're using loaders to load images and fonts, common js would fall over?

[kof]

Thats a lot of stuff one needs to do, how can we fix this easier? There should be some simple way.

[AlastairTaft]

I hit this issue again because my node_modules catching code didn't catch a module I was using that was a dependency inside another package.

Only thing I can suggest is a runtime check like my first comment.

But this is more a failing of the difficulty of webpack than your package, possibly it shouldn't fall on you to solve.

[rsaccon]

Now I am running into this issue as well. While everything works fine on a normal app, actually running in a browser (react, hot-module-reloaded from a webpack based devserver), things break when I run my code in Electron in the renderer process (which is a browser environment, exposes window):

there require('is-browser') returns false

My webpack target for the devserver which provides the hot-reloading js bundle:

target: 'electron-renderer',

a runtime check with my own isBrowser version, which runs for other purposes at many places at my app:

const isBrowser = (new Function("try {return this===window;}catch(e){ return false;}"))()

returns true.

I dont use jss directly, it is a dependency of material-ui (_next_ branch), so it will be a mess to hacky-patch jss and get the working version propagated to my app ...

so my request here is to use a runtime check as the one in the code snippet above, otherwise there might probably come up even more exotic use cases where is-browser fails

[rsaccon]

My current dirty ugly webpack devserver hack to get jss running in the electron app:

externals: {
  'is-browser': '__isBrowser__'
}  

so require('is-browser') resolves to a global variable, and before loading the js bundle, I simply set:

window.__isBrowser__ = true;

[kof]

Hmm year, thats unfortunate ...

[BenjaminVerble]

Having the same problem in electron. checking for window or document as described at the top of the thread seems reasonable to me. happy to submit a pr

[kof]

Ok lets switch to https://github.com/tuxsudo/is-in-browser

Needs to be done in all repositories which use currently is-browser package.

[kof]

Its done, will be released soon

[kof]

released

[v-trof]

I am new to jss and I might be doing something wrong, but require('jss') in node 8.2.1 throws and error.

myfile.js
require('jss') //yes removed everything else to test this

node myfile.js

node_modules\jss\lib\renderers\DomRenderer.js:119
  var style = document.createElement('style');
              ^

ReferenceError: document is not defined
    at C:\Projects\_learn\apmath-test\node_modules\jss\lib\renderers\DomRenderer.js:119:15
    at Object.<anonymous> (C:\Projects\_learn\apmath-test\node_modules\jss\lib\renderers\DomRenderer.js:153:2)
    at Module._compile (module.js:569:30)
    at Object.Module._extensions..js (module.js:580:10)
    at Module.load (module.js:503:32)
    at tryModuleLoad (module.js:466:12)
    at Function.Module._load (module.js:458:3)
    at Module.require (module.js:513:17)
    at require (internal/module.js:11:18)
    at Object.<anonymous> (C:\Projects\_learn\apmath-test\node_modules\jss\lib\utils\findRenderer.js:12:20)

[kof]

Thats strange, please submit a separate bug report

[kof]

Along with the JSS version you are using.

[kof]

Ok found it and fixed in v9.0.0-pre.3 please reinstall

[v-trof]

It helped.
Thanks a lot!
(global.document = {createElement: () => 0, getElementsByTagName: () => 0} felt way to hacky)