microsoft / tslib

Runtime library for TypeScript helpers.
BSD Zero Clause License
1.25k stars 126 forks source link

tslib is polluting window object #32

Open MasterCassim opened 7 years ago

MasterCassim commented 7 years ago

Is there a reason why tslib is pushing the helper functions to the window object when module.exports is available?

-> https://github.com/Microsoft/tslib/blob/master/tslib.js#L38

DanielRosenwasser commented 7 years ago

@rbuckton

MasterCassim commented 7 years ago

Any answer would be appreciated.

rbuckton commented 7 years ago

This was to support the following scenario:

Since tslib can't know how it is going to be used, it tries to ensure it supports both module and non-module scenarios.

rbuckton commented 7 years ago

If the issue is compelling enough, we could add a tslib.global.js variant (similar to what we do for tslib.es6.js), and restrict tslib.js. However; that would be a breaking change if anyone is depending on this functionality today.

jwmerrill commented 7 years ago

I ran into this issue while developing a library that needs to avoid adding/modifying properties on window. As a workaround, I am wrapping tslib with the following prelude/postlude:

tslib-prelude

;(function() {
  var global = {};

tslib-postlude

})();

And building a wrapped copy of tslib using

cat tslib-prelude tslib.js tslib-postlude > tslib-wrapped.js

This works because tslib first checks for a variable named global and if it finds such a variable with type 'object', it will attach the library functions to that object. The wrapper makes global a disposable, locally-scoped object so that attaching functions to it will not interfere with outer scopes. You can then still use amd or the other module systems tslib supports to import the tslib functions.

This is brittle and could easily stop working if the structure of the tslib export code is changed, but I thought I would share it anyway in case it's useful to someone else.

timocov commented 6 years ago

It is very strange to have window.__esModule === true now.

timocov commented 6 years ago

@rbuckton any updates on this?

dgoldstein0 commented 6 years ago

Hey, so we just ran into this at Dropbox while trying to upgrade to Typescript 2.6, since that requires us to also upgrade tslib. We use alameda (an alternative requirejs implementation) and load AMD & UMD modules. So we expect to have few if not no global variables.

When upgrading, we discovered that this global leakage (which we were unaware of before) now completely breaks our video previews: we use videojs 6.2.8, which appears to be a webpack or browserify bundle which internally, has a module that exports window. It also sprinkles logic like this around:

function _interopRequireDefault(obj) {
    return obj && obj.__esModule ? obj : { 'default': obj };
}

to transform non-es6 modules into things that look like es6 modules. When loading that window module with the new tslib already loaded, videojs then wrongly decides that it's window module is actually an es6 module, and returns window instead of {"default": window}, and crashes a few lines later, trying to get _window2["default"].navigator. This is particularly bad because the crash is when evaluating the module, so it stops any script that depends on videojs from loading. So in particular, the leakage of __esModule means we cannot ship a vanilla copy of tslib.

Moreover, we also use requirejs contexts so that we can transparently load different versions of the same module - which means, in theory, we could attempt to load two different copies of tslib on a page, so any code using the globals is not guaranteed to get the right version. Thankfully I see nothing that depends on the globals, but I would sleep better if we didn't leak them in the first place.

rbuckton commented 6 years ago

42 does not entirely fix this issue.

dasa commented 6 years ago

If the issue is compelling enough, we could add a tslib.global.js variant (similar to what we do for tslib.es6.js), and restrict tslib.js.

This sounds good.

dasa commented 6 years ago

Any status update on this issue?

slavafomin commented 5 years ago

Any news on this?

Kinrany commented 4 years ago

Similar to https://github.com/getsentry/sentry-javascript/issues/2215, this causes a (somewhat minor) issue for test frameworks.

In my case leak detection was enabled by default in @hapi/lab.