Allow type-only imports on ImportEqualsDeclarations

[andrewbranch]

A while back, @gr2m asked me to help take a look at octokit/webhooks.js#245. My summary of the issue is:

1. @octokit/webhooks.js legitimately targets --module=es2015 because it ships ESM to npm
2. Naturally, it also generates type declarations with --declaration
3. It has a CommonJS dependency, "aggregate-error"

The combination of (1) and (3) is a problem—CommonJS imports are not allowed with --module=es2015. The error message says that you have to turn on allowSyntheticDefaultImports or esModuleInterop, which is what the Octokit maintainers did. However, that is a problem for (2), because it just pushes the error onto consumers of the library who don’t have esModuleInterop turned on (although it is suppressible with skipLibCheck).

In places where they used AggregateError without it making its way into the generated type declarations, they were able to turn esModuleInterop off, continue using a default import (since it works at runtime—this kind of interop is in the current ESM draft spec, and is implemented in Node now), and suppress the TypeScript error with // @ts-ignore. However, there was one place where they needed to declare a publicly exposed class that extended AggregateError as part of its definition. They ended up simply copying the relevant parts of the class declaration as they had no good, portable way to import it. (An ImportTypeNode wouldn’t have worked without separately declaring a value side for the constructor, although this might have been slightly less tedious than copying the declaration.) If the syntax import type AggregateError = require("aggregate-error") had been allowed, it would have been a perfect fit—it captures both the type and the value side of the AggregateError class, and would have been allowed in the extends clause because the class declaration was ambient. The CJS-style import can be allowed to exist even in --module=es2015 because it will necessarily be elided due to the type keyword.

All the type-checking (and parsing—in order to give a good parse error) mechanics were already enabled for this, so this PR mostly just removes error messages to allow it. It’s obviously not a full solution to the awkward interop story between ESM and CJS, but it’s an additional tool that library authors can use, at next to no implementation cost. The only downside here is the factory API change for ImportEqualsDeclaration. (I could improve back-compat by putting the isTypeOnly parameter last, but it breaks the pattern of node factory function parameters corresponding to syntax order—i.e., the type keyword comes before the name identifier when written out, so the isTypeOnly parameter comes before the name parameter in the factory function.)

Also fixes #41603

[DanielRosenwasser]

Looks good, but let's chat about it at a design meeting.

[DanielRosenwasser]

Could you also add tests with importsNotUsedAsValues?

[DanielRosenwasser]

Tests for importsNotUsedAsValues

[andrewbranch]

All I want is just once to be able to push a change that will pass CI without having run tests locally, just once

[DanielRosenwasser]

Sounds like this is useful enough for us that we don't need a bot action: #39898

[DanielRosenwasser]

Aw you use your own fork, sad!

[sandersn]

Looks good!