microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.23k stars 12.52k forks source link

Is there a way to use TypeScript to prevent accidental global access? #14306

Open bcherny opened 7 years ago

bcherny commented 7 years ago

I just spent 2 hours tracking down this issue:

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.

zardoy commented 10 months ago

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? image

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

erquhart commented 6 months ago

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 commented 6 months ago

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

erquhart commented 6 months ago

@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 commented 5 months ago

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 commented 5 months ago

@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 commented 5 months ago

@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 commented 5 months ago

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;