Skip to content
This repository has been archived by the owner on Aug 4, 2021. It is now read-only.

rollup superagent breaks tests #56

Closed
leeyeh opened this issue Mar 21, 2016 · 6 comments
Closed

rollup superagent breaks tests #56

leeyeh opened this issue Mar 21, 2016 · 6 comments

Comments

@leeyeh
Copy link

leeyeh commented Mar 21, 2016

Upgrading superagent to version 1.8 broke the tests of my app.

Uncaught TypeError: Cannot set property '_timeout' of undefined

related change:

exports.clearTimeout = function _clearTimeout(){
  this._timeout = 0;
  clearTimeout(this._timer);
  return this;
};

https://github.com/visionmedia/superagent/blob/93b060790e78b6ca088aec57cb3168ee5e248007/lib/request-base.js#L12-L16

which was rolled up to:

var requestBase = __commonjs(function (module, exports) {
//...
exports.clearTimeout = function _clearTimeout(){                                                                                                    
    this._timeout = 0;
    clearTimeout$1(this._timer); // expected: clearTimeout(this._timer);
    return this;
};
//...
});
var clearTimeout$1 = requestBase.clearTimeout;
@eventualbuddha
Copy link
Contributor

Can you provide either a repository that exhibits this behavior or an example on http://rollupjs.org?

@leeyeh
Copy link
Author

leeyeh commented Mar 22, 2016

@eventualbuddha
Copy link
Contributor

Okay, it took me a while to see the problem, but it seems that this plugin is clobbering globals. Your request-base.js file is being rewritten by this plugin to this:

var requestBase = __commonjs(function (module, exports) {
// https://github.com/visionmedia/superagent/blob/93b060790e78b6ca088aec57cb3168ee5e248007/lib/request-base.js

exports.clearTimeout = function _clearTimeout(){
  this._timeout = 0;
  clearTimeout(this._timer);
  return this;
};
});

export default (requestBase && typeof requestBase === 'object' && 'default' in requestBase ? requestBase['default'] : requestBase);
export var clearTimeout = requestBase.clearTimeout;

That's not correct since it is creating a local binding for clearTimeout where there previously was none. That wouldn't be a problem if there wasn't already a reference to a global clearTimeout. Therefore, I think this should be the actual transformed code when an exported name conflicts with an implied global:

var requestBase = __commonjs(function (module, exports) {
// https://github.com/visionmedia/superagent/blob/93b060790e78b6ca088aec57cb3168ee5e248007/lib/request-base.js

exports.clearTimeout = function _clearTimeout(){
  this._timeout = 0;
  clearTimeout(this._timer);
  return this;
};
});

export default (requestBase && typeof requestBase === 'object' && 'default' in requestBase ? requestBase['default'] : requestBase);
var clearTimeout$1 = requestBase.clearTimeout;
export { clearTimeout$1 as clearTimeout };

@TrySound?

@dtothefp
Copy link

@eventualbuddha having the same issue here....

@leeyeh are you using the node-resolve plugin with {browser: true} options? I had a different compilation issue if I didn't use this option, but now we have the same clearTimeout global clobbering issue. Would be great to find a fix!!

@bethcodes

@leeyeh
Copy link
Author

leeyeh commented Mar 25, 2016

@dtothefp Yes.
FYI, superagent@1.7 works fine for me.

@Rich-Harris
Copy link
Contributor

Hi, sorry for the long, long wait to get this resolved. Fairly certain it was the same bug as #84, which was fixed by #107. If you update this plugin it should work fine – will close this but reopen if it isn't fixed

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

No branches or pull requests

4 participants