Add missing self variable if not defined and when running under node environment II

[simonThiele]

Hi again,

PR #7747 had an issue. You need to check if node is running separately and this is done by this PR. The reasons are the same. Sry for that. Please don't merge this atm because I'm checking how this will work with webworks to get clean with it.

[simonThiele]

So, the trick is to use an own scope variable to store the valid scope. In the browser, it's window, on node it's global and in webworker it is self. I validated it in the browser, in the raytracer webworker sandbox example and on node runtime and all passed the test and are working.

BufferGeometryLoader is using scope as well as it get's merge after the scope definition in the final build file.

But please review it with care and maybe try it out. It's always a mess to handle all these different environments.

[bhouston]

I would prefer to have comments in the code and a clear organization so that we can maintain this code easily. Something like:

var scope = null;
// if in a browser window
if ....
   scope = window;
// if in a node.js module
else if ...
   scope = global
// if in a web worker
else if ...
   scope = self
else 
   // and then error out if none of these work
   throw new Error( "can not identify global scope." );

The code as it is right now isn't that clear or well commented. It took me a while to figure it out.

Also you missed some usages of "self" -- at least in Clock and in the Font stuff.

[mrdoob]

Agree with @bhouston.

[simonThiele]

Thx for annotations! I added a few comments on that, I hope thinks are a little clearer now.

Also you missed some usages of "self" -- at least in Clock and in the Font stuff.

True, but these classes are not part of the build file so the scope is not defined. I'm afraid that users have to define the scope by their own.

[mrdoob]

I think calling the variable scope may create even more problems. Maybe this?

if ( typeof module !== 'undefined' && module.exports ) {

    // the code is running under node environment

    if ( typeof self === 'undefined' ) {

        // if self wasn't defined before, set to nodes global scope
        self = global;

    }

} else if ( typeof self !== 'undefined' ) {

    // if the code is running under other environments e.g. webworker, self is defined

} else {

    // the code is running in the browser, so window is defined as the global scope
    self = window;

}

[bhouston]

If you want to be a bit cleaner, one could create a function or variable on THREE rather than setting a global. It is less likely to get clubbered by other code. THREE.getSelf() or THREE.self or something like that. Just an idea.

[mrdoob]

It's just that the variable scope is used in many other places. Using the same variable is very prone to confusion and bugs.

[simonThiele]

@mrdoob Yes, calling it "self" works too (on node, browser and ww). Will refactor it.

[mrdoob]

How about we turn https://github.com/mrdoob/three.js/blob/dev/src/Three.js#L9-L17 into this instead?

if ( typeof define === 'function' && define.amd ) {

    define( 'three', THREE );

} else if ( 'undefined' !== typeof exports && 'undefined' !== typeof module ) {

    module.exports = THREE;
    self = global;

} else {

    self = window;

}

[simonThiele]

Doesn't work on node, since line https://github.com/mrdoob/three.js/blob/dev/src/Three.js#L9 is either true in node and L14/15 need to be called too so there must be at least an else if -> if. Furthermore, it seems you need to check for self because 1. I could be defined earlier and 2. Browser webpack test won't work https://github.com/brianchirls/webpack-test/blob/three.js/index.html (see #7783)

Only thing you can shorten is:

if ( typeof define === 'function' && define.amd ) {

    define( 'three', THREE );

}

if ( typeof module !== 'undefined' && module.exports ) {
    // the code is running under node environment

    module.exports = THREE;

  if ( typeof self === 'undefined' ) {

      self = global;

  }

} else if ( typeof self !== 'undefined' ) {

    // if the code is running under other environments e.g. webworker, self is defined

} else {

    // the code is running in the browser, so window is defined as the global scope
    self = window;

}

[bripkens]

Any update on this?

[mrdoob]

Here's another solution. How about we remove all the self instances from core?

We have mainly 3:

1. if ( self.requestAnimationFrame === undefined || self.cancelAnimationFrame === undefined ) ...
2. if ( self.performance === undefined ) ...
3. var typedArray = new self[ attribute.type ]( attribute.array );

As far as I know, 1 and 2 will still work when removing self.. For 3 we can just do an object.

[mrdoob]

Actually, I think it's time to remove the requestAnimationFrame and performance shims...

https://developer.mozilla.org/en-US/docs/Web/JavaScript/Typed_arrays#Browser_compatibility
https://developer.mozilla.org/en-US/docs/Web/API/Performance/now#Browser_compatibility

[mrdoob]

Ok, done. We shouldn't have to worry about self anymore.

[bripkens]

thanks @mrdoob!