Fix froken isomorphic support

[t1mmen]

Sorry :(

Current release errors out when running on the server:

/srv/app/web/node_modules/smoothscroll/smoothscroll.js:24
})(window, function(){
   ^
ReferenceError: window is not defined
    at Object.<anonymous> (/srv/app/web/node_modules/smoothscroll/smoothscroll.js:24:4)
    at Module._compile (module.js:446:26)
    at Module._extensions..js (module.js:464:10)
    at Object.require.extensions.(anonymous function) [as .js] (/srv/app/web/node_modules/babel/lib/babel/api/register/node.js:130:7)
    at Module.load (module.js:341:32)
    at Function.Module._load (module.js:296:12)
    at Module.require (module.js:351:17)
    at require (module.js:370:17)
    at Object.<anonymous> (/srv/app/web/app/mixins/smoothScrolling.js:3:20)
    at Module._compile (module.js:446:26)

Changes made to https://github.com/alicelieutier/smoothScroll/pull/12 via https://github.com/alicelieutier/smoothScroll/pull/13 broke isomorphic support.

I had a look Todd Motto's article that @ahoym linked, particularly the Non-browser global environments section, and it seems just passing in this instead of window in the IIFE is the correct approach.

This approach works well for our usage, tested both client and server-side.

[t1mmen]

@ahoym's suggestion looks the best to me, I'll update the PR. Feel free to voice your preference, @alicelieutier, and I'll update accordingly.

This is new territory for me, sorry about my confusion.

[ahoym]

Hm, the more I think about the logic though, maybe the conditional should be in the library instead of the wrapper. The reason being:

1. I'm mostly worried about false positives during wrapper execution. What if the window doesn't exist, but that's what's expected, and we wanted to use one of the module definitions?
2. The actual operations on the window and document are in the library, and it kinda makes sense to return out if there's nothing to operate on.
3. It will be in the same place as another blocking conditional for unsupported browsers here: https://github.com/alicelieutier/smoothScroll/blob/master/smoothscroll.js#L27-L29

If my suspicion is correct, for your server side, the logic is leaking through this conditional

  // CommonJS
  } else if (typeof exports === 'object' && typeof module === 'object') {
    module.exports = smoothScroll();

since both exports and module exist in node.js. This is why it never returns from the else in the existing UMD wrapper.

Either way, @t1mmen's original solution in the PR and my alternative both work in my situations.

[alicelieutier]

@ahoym - It makes sense now. I think you are right then, in saying the checks should go inside the library, closer to where we use them. Then the wrapper deals with attaching the library to the right object, and the library makes the necessary checks for its own logic.

What about something like this:

(function (root, smoothScroll) {
  'use strict';

  // Support RequireJS and CommonJS/NodeJS module formats.
  // Attach smoothScroll to the `window` when executed as a <script>.

  // RequireJS
  if (typeof define === 'function' && define.amd) {
    define(smoothScroll);

  // CommonJS
  } else if (typeof exports === 'object' && typeof module === 'object') {
    module.exports = smoothScroll();

  } else {
    root.smoothScroll = smoothScroll();
  }

})(this, function(){
'use strict';

// Do not initialize smoothScroll when running server side, handle it in client:
if (typeof window !== 'object') return;

...

[t1mmen]

I appreciate you walking me through your thought process on this!

I've made the changes as per your last comment @alicelieutier – hope it's to your liking now :)

[alicelieutier]

Thanks!