Update requestBase to remove implied global scope reference by 'this'

[ntilwalli]

This PR is submitted for discussion and possible merge. This PR updates requestBase so the this reference doe not imply the global scope which is aligned with ES6 module semantics and is required for proper bundling with rollup.

Rollup, upon seeing a this reference in the code whose parent scope is global will redefine this to undefined since that is conformant with ES6 module semantics. (rollup/rollup#759) In this case, since the methods in requestBase are mixed into other prototypes during run-time, in practice this does not end up referring to the global scope, so the redefinition by rollup causes errors. By enclosing the methods in a parent object the this no longer seems to imply the global scope and rollup does not modify the this references and this module gets bundled without issue.

[kornelski]

This is a bit weird. Rollup's requirement doesn't make sense in CommonJS world. Could such transformation be done by Rollup instead?

[ntilwalli]

I agree, a bit weird to workaround ES6 module semantics in a CommonJS package... ideally this would be handled with some sort of exception in rollup or some smarter parsing, but it was such a simple change I submitted the PR to have discussion/get your perspective. The issue I posted in rollup is here: rollup/rollup#1007

[ntilwalli]

I discovered this PR fixes the rollup-bundling issue but not because this was being overrwritten to undefined as I first thought. The clearTimeout function in the request-base.js module ends up shadowing the global/window clearTimeout due to the way rollup transforms commonjs modules to es6... I've submitted an issue to rollup-plugin-commonjs to better understand how to approach. rollup/rollup-plugin-commonjs#127

[kornelski]

OK, that makes more sense. Unfortunately exports.clearTimeout is a public interface in superagent, so I can't rename it.