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: coatue-oss/slickgrid2@0f8bab3.

[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 palantir/tslint#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 (#1351 (comment)).

[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.

[highruned]

Keep running into this with name. Runtime errors :|

[highruned]

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 :)

[ghost]

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.

No ES runtime starts with "global variables," these are all properties of the global object, therefore this is just a dom lib problem, +1 for dom-strict.

[alamothe]

How do we patch lib.dom.d.ts? Where is it located?

[david-fong]

How do we patch lib.dom.d.ts? Where is it located?

If you've installed typescript as a dependency of a project, it will be in node_modules/typescript/lib/lib.dom.d.ts. Some IDE's will ship with a recent version of TypeScript, which probably isn't a good idea to touch since your changes will be lost every time the IDE updates the version of TypeScript it ships with.

[david-fong]

Picking up from a discussion in 18433, quoting @thw0rted:

I'm not an expert but I think if you fork lib.dom, or split it such that some people are using "bad parts" of the global namespace while others aren't, you open yourself up to the same issues. Let's say I depend on some-lib and it includes a call to doStuff(name), intentionally referring to window.name using shorthand. They built their lib using an older version of TS, or they consciously included lib.dom-strict.d, but your consuming project avoids including the "strict" lib. Without skipLibCheck, the typechecker is going to descend into your dependency and flag the reference to name as an error.

I'm not an expert either, so I'd appreciate someone jumping in to confirm: I think the lib check is just for .d.ts files and not on source code files, and a project would need to set checkJs for that to happen. So I don't think the scenario you mention above will happen, since type definition files typically need a triple-slash lib reference to get dom types (where the author can choose lib.dom or lib.dom-strict), unless the author includes the tsconfig in their published package, which I think is probably a mistake.

This can be seen in DefinitelyTyped, where packages that triple-slash for the dom lib are: auth0-js, bent, fetch-headers, interface-datastore, make-fetch-happen, node-red__editor-client, pdfjs-dist, pdfmake, react-map-gl, requestidlecallback, socket.io, superagent, and xmldom.

To avoid duplicate definitions, I think lib.dom.d.ts can be set to triple-slash type reference lib.dom-strict.d.ts, and then just add the globals.

[crfrolik]

This is a pretty big footgun. We had a bug released to the wild due to this.
I know that I'm suffering from recency bias, but I think this should be near the top of the TypeScript team's priority list.

[Aaron-Pool]

Adding another real world example to this. My team accidentally had a window.stop call released into production. It was in a cleanup function, when a particular DOM node was destroyed. The function that was intended to be called had been renamed from stop to stopPolling. It was insanely hard to track down why requests were randomly getting cancelled. I would love for there to be some rule to force explicit usage of window.* functions, rather than accidentally calling them as global methods.

[muhamedkarajic]

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.

Seriously the sorting is the most important thing. When I program I would like to not have to go trough the hole list of suggestions and pick up where my variable is - ironically it's making me even slower. Just finally sort it.

[thw0rted]

The sorting part (of Intellisense) is configurable in your user / workspace preferences. I do think that the best solution for this ticket is to use the linter rule discussed above, but you might want to file a ticket with VSCode or the TS language service to allow users to have an ignore list that removes specific symbols from autocomplete.

[envatic]

extend the "eslint:recommended" in your eslintrc
add it to extend
extends: ["eslint:recommended", "plugin:vue/vue3-recommended", "prettier"],

[fo-fo]

Never was a fan of globals, and issues like this don't make me any more of a fan. Would be nice to see some kind of a solution for this. My preferred solution would be to have to explicitly import all globals, although I guess such explicit imports which would be omitted from the generated JS code might not align with TypeScript's goals.

An additional problem is that auto-import in VSCode doesn't offer any import suggestions if the symbol's name happens to match some global. One such example is the Text component in React Native (there's also a Text interface/constructor in DOM.)

[zardoy]

#14306 (comment)

Speaking of autocomplete, I'm using this (maybe someone else also would find it handy):

On the right side it also can be lib.es5 or lib.es2020 depending from which lib it comes

#51936

[alamothe]

TypeScript now supports custom DOM library: https://devblogs.microsoft.com/typescript/announcing-typescript-4-5/#lib-node-modules

So someone only needs to publish the library without globals.

The other way is to take lib.dom.d.ts, edit it to remove globals and save it with your project. Then remove a reference to the DOM library from your tsconfig.json. This approach also works great.

[Aaron-Pool]

Adding another real world example to this. My team accidentally had a window.stop call released into production. It was in a cleanup function, when a particular DOM node was destroyed. The function that was intended to be called had been renamed from stop to stopPolling. It was insanely hard to track down why requests were randomly getting cancelled. I would love for there to be some rule to force explicit usage of window.* functions, rather than accidentally calling them as global methods.

Two years later, and my team just got bit by the same issue, but with window.close() this time, would still love to see a stricter lib.dom.d.ts version published that remove window.* functions form the global namespace.

[smirea]

+1 here, the number of times I accidentally autocompleted onchange() from global lib/dom instead of the local onChange handler are too many to recall

If you use pnpm as the package manager, you can use the pnpm patch command to edit typescript/lib/lib.dom.d.ts and comment most of the declare var and declare function declarations towards the end of the file.

[danvk]

Another option would be to mark globals like focus as @deprecated like Daniel did for name in microsoft/TypeScript-DOM-lib-generator#883. That way they'd at least be visibly struck though in your editor, a pretty good indication that something weird is going on!

[zardoy]

Another option would be to mark globals like focus as @deprecated like Daniel did for name in microsoft/TypeScript-DOM-lib-generator#883. That way they'd at least be visibly struck though in your editor, a pretty good indication that something weird is going on!

But isn't completion info is enough for this?

and also you can just easily mark them as deprecated as you said and use eslint no-deprecated rule

[erquhart]

eslint is probably the ideal tool for this, the list of globals shared above works great. If someone made an eslint plugin/preset for this that could be kept up to date (and accept optional exclusions), I'd use it.

[shelbyspeegle]

@erquhart Try https://eslint.org/docs/latest/rules/no-restricted-globals - just solved my problem, hope it solves yours!

[erquhart]

@shelbyspeegle that's what the recommendation I was referencing does (see here), but it includes a long list of globals. The list itself is really helpful, and could be bundled into a plugin that configures no-restricted-globals and keeps the list up to date (and maybe configurable for specific environments, exclusions, etc).

[SystemParadox]

It's insane that this issue has been left unfixed for so long. Whoever wrote all those globals into lib.dom in the first place should be shot - they should just never have been added to the declaration, there is no reason for most of these to ever be used, let alone the fact that many of them have dangerously short names.

Excluding this with no-restricted-globals is utter madness, we shouldn't even be contemplating having to list them all again to block them. Ideally lib.dom should be fixed, but if anyone is thinking of an eslint fix for this there should be a way to whitelist acceptable globals, not have to blacklist all of these.

[david-fong]

@SystemParadox Good grief. Those are actual globals in the DOM API. Even if they aren't put in lib.dom.ts, they still exist as a footgun. If you're going to complain, complain about the actual DOM API (and good luck changing that).

[SystemParadox]

@david-fong your comment makes absolutely no sense. This is exactly the kind of footgun that typescript should be saving us from. It should not be happily declaring them all so we can shoot ourselves. Of course the DOM API isn't going to change, but it doesn't matter if they're still there in the runtime, typescript should pretend that they don't exist so if you use them by mistake it's flagged as an error.

I wouldn't mind if there was a sane way to get the DOM types so I could set this up myself, but short of copying the whole of lib.dom.ts it appears to be impossible to get the DOM types without also getting these dangerous globals. This is my primary complaint - there is no choice. If you want all these globals then that's up to you, but at least give the rest of us a way to keep a sane environment.

[arv]

I wouldn't mind if there was a sane way to get the DOM types so I could set this up myself,

I would love to be able to do that:

import type {Window} from "@types/web/module";  // Strawman
declare var window: Window;