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

'exports.clearTimeout' shadows global/window 'clearTimeout' #127

Closed
ntilwalli opened this issue Oct 6, 2016 · 3 comments
Closed

'exports.clearTimeout' shadows global/window 'clearTimeout' #127

ntilwalli opened this issue Oct 6, 2016 · 3 comments

Comments

@ntilwalli
Copy link
Contributor

ntilwalli commented Oct 6, 2016

Minimal reproduction repo: https://github.com/ntilwalli/rollupSuperagentIssue

I'm trying to bundle Superagent using rollup and it is not working. I had originally thought the issue had to do with this being overwritten to undefined but in actuality, the cause of the failure is the naming of one of the exports on the request-base.js module. That module exports a bunch of functions using exports.[name-of-function] syntax. One of the functions exported is exports.clearTimeout. That function itself calls the global clearTimeout, but this call to the global method gets shadowed upon transformation and during run-time the local clearTimeout function which tries to call the global clearTimeout ends up calling itself instead causing an error.

The offending function is here: https://github.com/visionmedia/superagent/blob/master/lib/request-base.js#L15

A non-ideal workaround does exist and has been posted (but this is arguably more of rollup-plugin-commonjs issue): ladjs/superagent#1076

I'm looking to start a discussion on how to handle this. I'd be willing to submit a PR if given guidance.

cc: @pornel

@Rich-Harris
Copy link
Contributor

Sorry for the slow response. I'm unable to run the repro because of this line. Could you update it please so that there's no TypeScript noise in the repro (unless TS is part of the problem)?

@kornelski
Copy link

BTW, we've released superagent 3 with a workaround for this. I think the problem still exists in rollup, but we've changed our inheritance implementation to dodge it.

Rich-Harris added a commit that referenced this issue Dec 22, 2016
Rich-Harris added a commit that referenced this issue Dec 22, 2016
don't overwrite globals
@Rich-Harris
Copy link
Contributor

Fixed in 6.0.1

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

No branches or pull requests

3 participants