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

Make the engine.io client work in Node.js enviornments that have a window global #147

Closed
3rd-Eden opened this issue Mar 26, 2013 · 7 comments · Fixed by #148
Closed

Make the engine.io client work in Node.js enviornments that have a window global #147

3rd-Eden opened this issue Mar 26, 2013 · 7 comments · Fixed by #148

Comments

@3rd-Eden
Copy link
Contributor

The current node check is really awful as it check is a global window variable is defined or not; https://github.com/LearnBoost/engine.io-client/blob/master/lib/util.js#L204-L226

Which happens to be case in my app. Is there a way we can make this a proper node.js check?

@rauchg
Copy link
Contributor

rauchg commented Mar 26, 2013

What's a proper node.js check?

@rauchg
Copy link
Contributor

rauchg commented Mar 26, 2013

I believe that things like browserify populate a lot of node globals in the browser, that's why not having a window global was the closest thing to "detecting node" in my mind.

@3rd-Eden
Copy link
Contributor Author

I was thinking about a '.node' in require.extensions check.

@3rd-Eden
Copy link
Contributor Author

exports.request = function request (xdomain) {
  if ('undefined' !== typeof require && require.extensions && '.node' in require.extensions) {
    var _XMLHttpRequest = require('xmlhttprequest').XMLHttpRequest;
    return new _XMLHttpRequest();
  }

  if (xdomain && 'undefined' != typeof XDomainRequest && !exports.ua.hasCORS) {
    return new XDomainRequest();
  }

  // XMLHttpRequest can be disabled on IE
  try {
    if ('undefined' != typeof XMLHttpRequest && (!xdomain || exports.ua.hasCORS)) {
      return new XMLHttpRequest();
    }
  } catch (e) { }

  if (!xdomain) {
    try {
      return new ActiveXObject('Microsoft.XMLHTTP');
    } catch(e) { }
  }
};

Would work against edge cases of client module loaders. No mode loader is going to require '.node' files and the module system is set in stone in node so it would be save enough to expect this property.

@3rd-Eden
Copy link
Contributor Author

or we can just forget all the checks.. and add a

try { return require('xmlhttprequest').XMLHttpRequest() }
catch (e) {}

@rauchg
Copy link
Contributor

rauchg commented Mar 26, 2013

I like that most I think. Not sure if that exact impl is gonna work though.

@3rd-Eden
Copy link
Contributor Author

i'll make a pull req with working version :p

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants