Rule proposal: No assignment to global object

[SamVerschueren]

Recently someone did a PR where he put something on the global object. Something like

global.foo = 'bar';

This doesn't feel like a good thing to do. You might accidentally override something. Also it makes it very hard to track where the global variable was set. I don't believe I actually ever used the global keyword in my code.

It's the same thing for window in browser environments.

Just wanted to bring up the discussion as I might be wrong though. If not, should we forbid the use of global entirely? Or only setting values?

[SamVerschueren]

Related rule https://eslint.org/docs/rules/no-global-assign. But not sure if it blocks my example. On mobile so can't check.

[sindresorhus]

@SamVerschueren Can you check?

[brettz9]

When the env has node, no-global-assign will only report if one attempts to overwrite the whole variable, e.g., global = 'bar';. See https://runkit.com/brettz9/5e3676fecfe2f1001a4f9159 .

The rule won't report for adding to global properties as in the original post. See https://runkit.com/brettz9/5e36777bfa499e001b98ecb4 .

[brettz9]

Particularly in the age of ES modules, this sounds like a great rule.

Even a rule to prevent reading from the global object (besides whitelisted properties) would be great (including for checking imported code, as a kind of privilege checker).

[sindresorhus]

I agree, this could be useful.

It should prevent setting properties on window, global, and globalThis.

[sindresorhus]

Even a rule to prevent reading from the global object (besides whitelisted properties) would be great (including for checking imported code, as a kind of privilege checker).

I don't see how that's feasible in a browser environment. Pretty much everything are globals.

[sindresorhus]

// @fisker Thoughts on this rule?

[fisker]

I agree, this could be useful.

It should prevent setting properties on window, global, and globalThis.

How do we do with setting window properties, location, name, cookie? those setters are sometimes useful, make a list?

Should self forbid too?

[sindresorhus]

We should only disallow setting nonexistent properties.

[sindresorhus]

self too, yes.

[fisker]

By nonexistent properties, what about window.JSON = require('json2') ?

I guess, JSON should considered exist, maybe people could allow it by // eslint-disable

[brettz9]

@sindresorhus : If you mean that users need to access their own variables, besides the case of ESM where users don't need to access globals for such functionality, users can add their globals to /* globals */ (or in their .eslintrc.*).

If you mean that the HTML APIs are globals, yes, but I had been thinking the rule could allow permitting certain globals (e.g., a dom option could enable access to document, HTMLElement, etc., some safe ones like Math could always be permitted, etc.).

While one can disable env browser and node to get no-undef reporting (of document, etc.), one can't easily whitelist groups of certain globals only. (Admittedly a dynamic .eslintrc.js file could be written to do this, but a rule appeals to me personally.)

[sindresorhus]

I guess, JSON should considered exist, maybe people could allow it by // eslint-disable

Yes, we can document that in the rule docs.

[sindresorhus]

If you mean that the HTML APIs are globals

Yes, this is what I meant.

[sindresorhus]

I feel like this would be better as just an option to https://eslint.org/docs/rules/no-implicit-globals to prevent assigning to known globals.

[sindresorhus]

Someone please open an issue on ESLint about this and link it here. If they decline it, we can consider adding it here.