I think we can avoid this playground link as we already have examples below and we are working on typescript syntax support in ESLint playground.

we are working on typescript syntax support in ESLint playground.

Neat!

Still, I think the TypeScript playground does add context. The code example alone doesn’t show why it’s ok (better even) to use var instead of let inside a declare global block. I’ll gladly remove it if you’re still not convinced.

In that case, instead of the playground link, how about if we add a code block in the docs page to explain why we need var in TypeScript with appropriate code comments in block?

you can see this page for reference.

I think it's fine to link to the TypeScript playground for demonstration purposes. Our own playground won't have the type checking so not as solid of an example.

On the rule logic - I was thinking this should also apply to top-level definitions in (non-module) declaration files, right? Been a while since I've worked with that particular setup. But something like this?

// foo.d.ts
declare var globalVar: string;

Here's a little minimal repository demonstrating this behavior for reference: https://github.com/kirkwaiblinger/tsglobalvardemo

Come to think of it, it actually doesn't matter whether it's a .d.ts file or not. The same behavior applies with declared variables in ordinary .ts files.

// foo.ts

// setup
declare var globalVar: string;
declare let globalLet: string;
declare const globalConst: string;

// read unqualified
console.log(globalConst); // works
console.log(globalLet); // works
console.log(globalVar); // works

// write unqualified
globalConst = 'doesnt work';
globalLet = 'works';
globalVar = 'works';

// read on globalThis
console.log(globalThis.globalConst); // doesnt work
console.log(globalThis.globalLet); // doesnt work
console.log(globalThis.globalVar); // works

// write on globalThis
globalThis.globalConst = 'doesnt work';
globalThis.globalLet = 'doesnt work';
globalThis.globalVar = 'works';

// read on self
console.log(self.globalConst); // doesnt work
console.log(self.globalLet); // doesnt work
console.log(self.globalVar); // works

// write on self
self.globalConst = 'doesnt work';
self.globalLet = 'doesnt work';
self.globalVar = 'works';

What matters is whether TypeScript considers file to be a script or a module. If a module contains imports or exports, it’s a module. Otherwise it’s a script. In a script everything you declare is global. In a module you can declare globals with declare global.

So this module:

declare global {
  var x: number;
}

export {}

is equivalent to this script:

declare var x: number;

However, TypeScript also has the option moduleDetection. If this is set to force, everything source file considered a module. This makes it ambiguous whether the latter is a local variable declaration or a global. We could assume default options. The moduleDetection option doesn’t apply to declaration files.

In typescript-eslint/typescript-eslint#7941, the proposal is to require var for global ambient declarations rather than just to permit it. Is the discrepancy in behavior here, where non-vars are simply ignored, intentional?

That’s #2594, though there is discussion about it in #7941. I have been fiddling a bit to implement #2594. I don’t think it should have been closed as a duplicate. They’re two different things.

Hmm, I see what you're saying...

However, I think it's not such a good idea to have two separate rules, especially in two separate plugins, handle overlapping and possibly-conflicting logic. It's like if return-await were actually two separate rules for when return await is required and when it's prohibited, and those two rules were in separate plugins, and one of them didn't even have access to the TS API required for correctness in some cases.

What do you think of the following proposal instead? What if we draw the line such that this rule, no-var, ignores all TS ambient declarations, even those that shouldn't be vars, and then the typescript-eslint rule (maybe called something like declare-var) would have the job of properly enforcing or prohibiting vars in all cases for declared variables. This way detailed TS knowledge stays out of no-var and the dividing line between what's handled by typescript-eslint and what's handled by no-var is much more clear.

Concretely, that would mean I propose that all of the following, currently documented as incorrect, would be permitted by no-var...

declare var x: number

declare namespace ns {
	var x: number
}

declare module 'module' {
	var x: number
}

and instead would be banned by a new typescript-eslint rule.

To be clear, I most prefer having all the logic handled in this no-var rule, including enforcing declare var where required, at the possible expense of a niche edge case as mentioned in #19714 (comment).

But if we're not going to enforce declare var where required in this rule, I think we should draw the line at no-var ignoring declare var completely.

I think the most correct implementation is as you suggested in #19714 (comment). no-var should not be requiring var anywhere, it should be allowing var where another option doesn't make sense.

Actually this even applies to regular code, not just declarations. For example, the following file is also a global script as far as TypeScript is concerned. It doesn’t even matter if it’s JavaScript or TypeScript syntax.

var a = 1

How far should the no-var exceptions go? Also how many users will actually run into this? I think most code is written as a module today, or at least that’s often the intention.

There are only 2 cases where the user should definitely use var:

1. Within declare global, which is handled by this PR.
2. Within a TypeScript declaration file that doesn’t contain ESM syntax. This can be detected based on the file name (*.d.ts / *.d.cts / *.d.mts / *.d.*.ts). Should I add support for these?

The original intent of this rule is to ban the use of var completely. The goal of this PR is to mark as an exception those places in TypeScript where changing from var to something else either isn't possible or changes the behavior in an undesirable way.

2. Within a TypeScript declaration file that doesn’t contain ESM syntax. This can be detected based on the file name (*.d.ts / *.d.cts / *.d.mts / *.d.*.ts). Should I add support for these?

At this point I'd say no. This feels like a step too far. A better approach for a user is to disable this rule just in those files.