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

tslib is polluting window object #32

Open
MasterCassim opened this issue Jun 9, 2017 · 13 comments · Fixed by #42 · May be fixed by #52
Open

tslib is polluting window object #32

MasterCassim opened this issue Jun 9, 2017 · 13 comments · Fixed by #42 · May be fixed by #52

Comments

@MasterCassim
Copy link

MasterCassim commented Jun 9, 2017

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
Copy link
Member

@rbuckton

@MasterCassim
Copy link
Author

Any answer would be appreciated.

@rbuckton
Copy link
Member

This was to support the following scenario:

  • You have both module and non-module (script) files in your TypeScript project
  • You specify both --importHelpers and --noEmitHelpers (or just --noEmitHelpers)

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
Copy link
Member

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
Copy link

jwmerrill commented Sep 14, 2017

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
Copy link

timocov commented Oct 11, 2017

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

@timocov
Copy link

timocov commented Nov 1, 2017

@rbuckton any updates on this?

@dgoldstein0
Copy link

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
Copy link
Member

rbuckton commented Nov 8, 2017

#42 does not entirely fix this issue.

@dasa
Copy link

dasa commented Dec 4, 2017

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
Copy link

dasa commented May 11, 2018

Any status update on this issue?

@rbuckton rbuckton linked a pull request May 13, 2018 that will close this issue
@slavafomin
Copy link

Any news on this?

@Kinrany
Copy link

Kinrany commented Mar 14, 2020

Similar to getsentry/sentry-javascript#2215, this causes a (somewhat minor) issue for test frameworks.

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

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