Suggestion: no-restricted-globals for types

[OliverJAsh]

The ESLint rule no-restricted-globals is great, but it only applies to global values. I want the same thing, but for global types.

Example usage: I want to disallow usage of the global TypeScript Omit type because it's not strict enough, but I still want to allow imported types of that same name to be used.

// error
type F = Omit<{}, 'foo'>

import { Omit } from './helpers';

// no error
type F = Omit<{}, 'foo'>

This is similar to the existing ban-types rule, but different because ban-types bans the type both globally and locally.

[bradzacher]

I would think a better solution is to rename your helpers so that they don't clash, so that there is no weirdness, and a rule that catches non-imported vs imported isn't needed.

Having developers get confused that they're using "the wrong Omit" is a pretty clear smell IMO:

• your editor can't suggest that it be auto-imported, because it's already a thing in the global namespace
• the compiler won't catch it for the same reason
• you can't auto-fix it because eslint rules shouldn't be adding imports as well as changing other code

[OliverJAsh]

I agree, but sometimes we don't get to decide how to name things—libraries do it for us.

Examples:

• the popular history module has a Location type, but there's also a global Location
• many libraries (fp-ts, Funfix) provide an Option type, but there's also a global Option (modelling something entirely different)

In that case, we still want to catch mistakes where the global one is being used when it shouldn't.

[OliverJAsh]

• your editor can't suggest that it be auto-imported, because it's already a thing in the global namespace

Side note: I really want this TS / VS Code feature to help in this case: microsoft/TypeScript#31199.

[vladimiry]

The type provided by the library can be renamed and exported to global scope, one example.

[OliverJAsh]

Hmm, that's only reasonable if you're happy polluting the global namespace. Seems a shame to me to have to sacrifice modules/imports.

Also, sometimes you're dealing with not just a type but also a value, e.g. the Option example I gave is a class.

There are different ways of managing this problem. This lint rule suggestion would enable one of them, which some people might prefer.

[OliverJAsh]

Regardless of how you avoid the name conflict, we still have the core problem of "how to prevent accidental usage of the global type". Even if I name my custom Omit something else like StrictOmit, I still want to disallow accidental usage of the global Omit. We can use ban-types for that, but that's overly specific—we only need to ban the global type.

[OliverJAsh]

All of the reasons you might want this for values also apply to types. So if ESLint can offer it for values, shouldn't this plugin provide parity by offering it for TS types?

[OliverJAsh]

Just came across this again.

• I can ban the global value location using no-restricted-globals but still use that identifier for non-global values (✅).
• However, if I ban the global type Location using ban-types it will also ban that identifier for non-global values (❌).

If I wanted to write this lint rule, how can I read a list of "global types" in the lint rule? no-restricted-globals uses scope.through—is there a type equivalent of this?

https://github.com/eslint/eslint/blob/9dfc8501fb1956c90dc11e6377b4cb38a6bea65d/lib/rules/no-restricted-globals.js#L113

[bradzacher]

It depends on what you mean by "global types".

There are two types of global types:

1. typescript's core lib types (provided in lib.**.d.ts, specified by the lib compiler option)
2. user-land global types:
   a) types defined in @types/**
   b) types defined in misc packages in node_modules
   c) project-local types.

For (1), it's super simple to detect them, see this example from the no-throw-literal rule, which detects if the given type is the Error type defined in a TS core lib.

typescript-eslint/packages/eslint-plugin/src/rules/no-throw-literal.ts

Lines 31 to 40 in 6aa6bc7

	const symbol = type.getSymbol();
	if (symbol?.getName() === 'Error') {
	const declarations = symbol.getDeclarations() ?? [];
	for (const declaration of declarations) {
	const sourceFile = declaration.getSourceFile();
	if (program.isSourceFileDefaultLibrary(sourceFile)) {
	return true;
	}
	}
	}

For (2), it gets a bit difficult. For example, people are trying to amend the unbound-method rule so it doesn't error on console.* methods. But the console is a core lib type for dom, but not for nodejs. Related comment:
#1085 (comment)

In this case, you have to default to tracing the symbol to figure out where it originated. i.e. if it was defined locally in any way (imported, type def, generic, etc), then it's not a global type.
This also has a bit of weirdness due to interface/namespace declaration merging.

[OliverJAsh]

Thank you @bradzacher. In my specific case I only care about (1), so that's very useful 😄

[lizozom]

Hey @OliverJAsh @bradzacher,

I'm working on implementing ApiExtractor in Kibana.
We ran into a complicated issue last week with the build process:

@octogonz kindly helped us determine that the build process failed, because we used a type from React in a .ts file. In that file, React wasn't imported explicitly, so the fallback was to use the global React, causing the ApiExtractor failure.

We would like to add a typescript-eslint rule to our project, preventing any developer from using React types, without importing React. Since there is no no-restricted-globals for types, do you see any possible workarounds for this situation? Would you be open to work together on such a rule?

[bradzacher]

There's no real solution right now.
It would be relatively trivial to write an extension rule for no-restricted-globals which analyses type references to check if:

1. the name is imported or not.
2. the name is not "banned", if it's not imported.

There's already example code in the plugin for doing check (1):

typescript-eslint/packages/eslint-plugin/src/rules/unbound-method.ts

Lines 97 to 111 in 133f622

	const isMemberNotImported = (
	symbol: ts.Symbol,
	currentSourceFile: ts.SourceFile | undefined,
	): boolean => {
	const { valueDeclaration } = symbol;
	if (!valueDeclaration) {
	// working around https://github.com/microsoft/TypeScript/issues/31294
	return false;
	}
	return (
	!!currentSourceFile &&
	currentSourceFile !== valueDeclaration.getSourceFile()
	);
	};

Happy to accept a PR to add this in!

[lizozom]

@bradzacher any examples of extension rules for reference?

[bradzacher]

Many!
https://github.com/typescript-eslint/typescript-eslint/tree/master/packages/eslint-plugin#extension-rules

One of the simple examples is the no-dupe-class-members rule

[runspired]

Seems like this was added recently since bumping to latest caused typed returns from functions that are returning values gotten from native functions (that are returning native promises) to error.

[OliverJAsh]

I needed this again (this time to ban the global React type) so I would like to try and submit a PR.

I think this makes more sense as an option in ban-types rather than an extension of no-restricted-globals, because no-restricted-globals is currently only concerned with values whereas ban-types is concerned with types. I have a WIP PR to add an option called banGlobalOnly to the ban-types rule: #4441

How does that sound @bradzacher? If that sounds good, could I get some help in the PR? I'm a bit stuck #4441 (comment)

[bradzacher]

When you first posted this issue back in 2019, we didn't have our own scope manager.
So types weren't properly handled as "variables" in regards to ESLint.
However - now we do.

So no-restricted-globals now works for this usecase of banning global types!
We don't need a new rule or new options to have this work!

[OliverJAsh]

Amazing! I guess we can close this then?