Typescript: Is there a way to use TypeScript to prevent accidental global access?

[bcherny]

I just spent 2 hours tracking down this issue:

• We have a class with a prototype method called focus()
• Our code was calling focus(), but it should have been calling this.focus()
• The code compiled fine, because window.focus() shares the same signature as our focus() method

Is there a way to throw a compile time error when implicitly accessing global methods (on window, global, etc.)?

If not, a compiler flag would be extremely helpful. I would happily be more explicit about commonly used globals (window.setTimeout, window.document, ...) if it meant I could more easily catch insidious bugs like this one.

Full commit here: https://github.com/coatue/SlickGrid/commit/0f8bab3fa9968173d749ccdf535c88f3a526ca8b.

[DanielRosenwasser]

The number of times I use name and location as variables tends to make this pretty frustrating.

The unfortunate problem is that you can end up writing accidentally wrong code if we omit the global declarations (because you might accidentally redeclare global variables) so we'd really need to be able to recognize that there are undesirable properties declared that you don't want to access.

[niieani]

How about adding a tsconfig.json option such as:

explicitGlobals: true

With it on, if you'd write name, location or focus() without being explicit, i.e. window.name, while at the same time, if those names aren't declared in the local scope, it would mark those references as errors. At the same time there could be a quick fix to either reference the local scope, in case of a similar problem to @bcherny (missing this) or to reference the global scope explicitly, (i.e. name => window.name).

This would require being able to mark variables such as window, self and global as globals, instead of redefining each of their properties globally (as is currently done).

This would also mean that given the option is true, and one declared a $ global which is typeof jQuery, any usage would trigger the above error, unless you'd use it as window.$ or using import * as $ from 'jQuery'.

[blakeembrey]

That's a good idea and typical of JavaScript linting (e.g. the /* global $ */ header). If it were explicitGlobals you could set an array for the same effect (e.g. whitelisted globals). However, I wonder if this would be better as a TSLint rule instead. For instance, a rule like http://eslint.org/docs/user-guide/configuring#specifying-globals could be implemented. I saw https://github.com/palantir/tslint/issues/922, but no discussion of a "globals" option whitelist like JavaScript linters tend to have.

[bcherny]

The unfortunate problem is that you can end up writing accidentally wrong code if we omit the global declarations (because you might accidentally redeclare global variables)

@DanielRosenwasser AFAIK TS gives an error when declaring a variable without a var/let/const keyword, similar to how use strict enforces this at runtime. Eg. foo = 1 fails, but var foo = 1 is fine. How else could you accidentally overwrite a global? Would you mind helping me understand with a concrete example?

explicitGlobals: true... At the same time there could be a quick fix to either reference the local scope

@niieani That would be a very clean UX, I like these two ideas a lot.

This would require being able to mark variables such as window, self and global as globals, instead of redefining each of their properties globally (as is currently done).

Can you help me understand why? Shouldn't the fact that the declaration is ambient (as opposed to exported from a module) be enough to hint to TSC that a name is global?

However, I wonder if this would be better as a TSLint rule instead.

@blakeembrey That's an interesting idea - I wonder what makes a behavior belong in linterland vs. compilerland, semantically speaking. Perhaps since this sort of mistake causes incorrect behavior, it belongs in TSC rather than TSlint.

[Jessidhia]

Another name that is tricky, especially when converting old code, is self. Almost got bitten by that a few weeks ago...

[zpdDG4gta8XKpMCd]

workaround is to use "noLibs": true and then add necessary *.d.ts by hands from https://github.com/Microsoft/TypeScript/tree/master/src/lib

once done, you can delete unwanted declarations from say dom.d.ts by hands

downside though is that you will have to manually update theses files as typescript advances

[zpdDG4gta8XKpMCd]

@DanielRosenwasser it might worth considering adding an option to mute certain ambient declarations, something like:

undeclare let name: string;

[lefb766]

1351 is related.

@bcherny Check out this issue for an example of the redeclaration problem (https://github.com/Microsoft/TypeScript/issues/1351#issuecomment-106573083).

[Knagis]

We ended up editing the lib.d.ts file and removing most of the globals to avoid using parent, name, top etc. by mistake.

Perhaps these globals could be separated in their own lib file that can then be selected by adding, for example, DOM.Globals to the lib setting in tsconfig.json? The default DOM would include only very few selected variables, such as window and document.

[ericmuyser]

Keep running into this with name. Runtime errors :|

[ericmuyser]

Another suggestion is anything inside a defined module doesn't inherit globals (that's the first thing I tried). I gather that may reduce the barrier to doing this so it won't be an all or nothing thing. Maybe combine with undeclare or omit or visa versa (opt into globals). eg

module MyModule {
  global name
  global focus

  export class Topic {
    constructor() {
      focus() // refers to global focus
    }
  }
}

I recall some language doing it like this.. maybe ActionScript or C.

[evmar]

We have also been bitten by the 'name' thing at Google, and have also been considering patching it out of our lib.d.ts. I think the fix that behaves how TypeScript does is to split the standard library up further so that a project can opt in or out from the global declarations.

[ccorcos]

I just had an issue where we refactored a function that called parent.close() and parent wasn't in scope anymore. It causes electron to freeze with no errors anywhere. What a pain!

It would be nice if we could whitelist globals inside TSConfig. I don't want accessing anything on the window object without explicitly calling window.property.

Another option would be to require importing any globals. Not sure where it would import it from, but something like import window from "ts/lib/dom" would solve this issue as well.

[ianks]

i just got hosed by name. it would awesome if there was away to limit access to these to the window object.

[zpdDG4gta8XKpMCd]

as of now the only way is to take ownership of lib.d.ts by fixing it, ask me how

[a7madgamal]

how about a new setting or lib to allow these globals ONLY through window.xxx

[poke]

What about creating a separate lib.dom-strict.d.ts that only declares the global variables window and document? So any use to e.g. name would have to go through window.name?

So users could choose to reference that strict version of the dom library to avoid these global names from being used within their applications.

[mac2000]

Looking for a way to prevent usage (not declaration) of global variables to have proper IoC and DI in place in Angular.

E.g. wish to prevent usage of localStorage and force its injection via constructor

[ccorcos]

For now, I just have a post-install script that rewrites type definition files for malicious files.

rewrite("node_modules/@types/jest/index.d.ts", str => {
    return str
        .replace(/declare const/g, "export const")
        .replace(/declare var/g, "export var")
        .replace(/declare function/g, "export function")
})

[csicky]

The autocomplete list has nothing to do with my work, I don't know why is there, I don't know what the focus function is and why is on top. All the items are like random words and I have no idea where are coming from. I have disabled auto import but still what I see in this list is disturbing. I try to imagine why are all these items there but can't find a reason. It's just random garbage.

I don't know why it's so complicated, really. What is on window and what is in my code. Nothing more, nothing less. My variables to be on top, those from window to have less priority.

[RyanCavanaugh]

I don't know why is there, I don't know what the focus function is

I don't know why it's so complicated, really. What is on window and what is in my code. Nothing more nothing less

focus comes from window. TypeScript is literally doing what you said it should do, minus the sorting part.

[csicky]

Yes, you are right, the screenshot has most of the things from window. I hate this autocomplete because of what it did in the past, I've had so many exotic things in it. It seems that disabling the auto import feature works well indeed.

I don't know how it does it, but Visual Studio was giving me exactly what I needed at the right time. I use Visual Studio Code since many years now, but I never have a good experience with what it suggests. If Visual Studio would work well with Vue.js I wouldn't be here writing about fixing Code.

Sorry for my tone, this is just real feedback, without the feelings it would be just half the feedback.

[cazgp]

Was just also bitten by the name problem.

[KurtPreston]

There is a new TSLint rule that prevents access to certain global vars:

https://palantir.github.io/tslint/rules/no-restricted-globals/

[cazgp]

Still seems like it's something the compiler should be able to handle.

[trotyl]

undefined is also a normal global variable in JavaScript (some people may not even aware of that), and every access to [[global]].undefined is not accidental, but deliberate.

From the thread most people only want to forbid HTML-extension of global, rather than JavaScript global itself.

[csicky]

It's not that important what is there, it's what is selected. In Visual Studio is almost like it's reading my mind. With VS Code is like it is only now learning what it should be.

For example, in VS if I write something.whatever = other.whatever as soon as I type o , other is highlighted and with that o typed I then type . the other is completed, and, here is the magic, whatever is already selected on other.
This is just one example of many IntelliSense is capable off. Being in the same Microsoft, why VS Code team don't ask for a little help from the VS team?

We had good IntelliSense in VS in 2005 already.

Sorry for my English :)

[NN---]

@KurtPreston Since TSLint is deprecated, there is a same rule in and even npm package with list of confusing globals such as name.

[AlexGalays]

This is SO valuable and has been a source of bugs, confusion and generally lost time on most projects.

Examples of globals with totally stupid names that are easily used instead of a local version: find (only a problem in JS project however), name, close, history, length, open, parent, scroll, self, stop, event, ALL event listener attributes, etc.

[inca]

+1 for dom-strict, +1 for globals: ['window', 'document'] tsconfig option — whichever one is more feasible. But we definitely need _something_.

[jahooma]

We had two bugs this week because of this. Why doesn't typescript have a solution out of the box? Seems like an obvious addition for strict mode.

[jahooma]

Ok, while typescript is working on this, here's a solution.

If you use eslint, you can just copy the below rule into your .eslintrc.

I left out some of the more commonly used variables like document, setTimeout, and addEventListener, but otherwise it is most of them.

"no-restricted-globals": ["error","postMessage","blur","focus","close","frames","self","parent","opener","top","length","closed","location","origin","name","locationbar","menubar","personalbar","scrollbars","statusbar","toolbar","status","frameElement","navigator","customElements","external","screen","innerWidth","innerHeight","scrollX","pageXOffset","scrollY","pageYOffset","screenX","screenY","outerWidth","outerHeight","devicePixelRatio","clientInformation","screenLeft","screenTop","defaultStatus","defaultstatus","styleMedia","onanimationend","onanimationiteration","onanimationstart","onsearch","ontransitionend","onwebkitanimationend","onwebkitanimationiteration","onwebkitanimationstart","onwebkittransitionend","isSecureContext","onabort","onblur","oncancel","oncanplay","oncanplaythrough","onchange","onclick","onclose","oncontextmenu","oncuechange","ondblclick","ondrag","ondragend","ondragenter","ondragleave","ondragover","ondragstart","ondrop","ondurationchange","onemptied","onended","onerror","onfocus","oninput","oninvalid","onkeydown","onkeypress","onkeyup","onload","onloadeddata","onloadedmetadata","onloadstart","onmousedown","onmouseenter","onmouseleave","onmousemove","onmouseout","onmouseover","onmouseup","onmousewheel","onpause","onplay","onplaying","onprogress","onratechange","onreset","onresize","onscroll","onseeked","onseeking","onselect","onstalled","onsubmit","onsuspend","ontimeupdate","ontoggle","onvolumechange","onwaiting","onwheel","onauxclick","ongotpointercapture","onlostpointercapture","onpointerdown","onpointermove","onpointerup","onpointercancel","onpointerover","onpointerout","onpointerenter","onpointerleave","onafterprint","onbeforeprint","onbeforeunload","onhashchange","onlanguagechange","onmessage","onmessageerror","onoffline","ononline","onpagehide","onpageshow","onpopstate","onrejectionhandled","onstorage","onunhandledrejection","onunload","performance","stop","open","print","captureEvents","releaseEvents","getComputedStyle","matchMedia","moveTo","moveBy","resizeTo","resizeBy","getSelection","find","createImageBitmap","scroll","scrollTo","scrollBy","onappinstalled","onbeforeinstallprompt","crypto","ondevicemotion","ondeviceorientation","ondeviceorientationabsolute","indexedDB","webkitStorageInfo","chrome","visualViewport","speechSynthesis","webkitRequestFileSystem","webkitResolveLocalFileSystemURL", "openDatabase"],

[kingworm]

:(

[jgehrcke]

+1 for dom-strict, +1 for globals: ['window', 'document'] tsconfig option — whichever one is more feasible. But we definitely need something.

Yes, we need something. In tsc. We got bitten by this kind of problem more than once. It's quite disappointing to see TSC not catching this, currently.

Ok, while typescript is working on this, here's a solution.

Thanks for that @jahooma.