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.