[isolatedDeclarations][5.5] Disagreement between transpileDeclaration API and typechecker on Symbol.iterator as computed property

[MichaelMitchell-at]

[isolatedDeclarations][5.5] Disagreement between transpileDeclaration API and typechecker on Symbol.iterator as computed property

[weswigham]

To be even more explicit, this

// @filename: defines.d.ts
export const mySymbol: unique symbol;
// @filename: usage.ts
import {mySymbol} from "./defines.js"
export class Foo {
  [mySymbol]() {}
}

is something you want to allow because it's how symbols are meant to be used, but

// @filename: defines.d.ts
export const mySymbol: string;
// @filename: usage.ts
import {mySymbol} from "./defines.js"
export class Foo {
  [mySymbol]() {}
}

is something you need to forbid because you don't know if mySymbol conflicts with anything. And when you only look at usage.ts, these two cases are identical. So you're either conservative and forbid basically all computed names (very bad for usability), incredibly loose and allow most computed names (which is a big footgun for anyone relying on the output computed name actually working/existing when transformed into a type), or you slice it in some weird way to sometimes work and sometimes not based on some common usecase heuristics.

[MichaelMitchell-at]

@dragomirtitian @weswigham here's a fun one for y'all 🙃

[dragomirtitian]

For computed properties we allow them to be used in isolated declarations even if we can't determine the type is valid locally. This was due to the fact that computed properties with unique symbols would otherwise be completely unusable with isolated declarations.

In transpileDecalarations the checker should only be used when needed. In this case the checker will not actually find a type for Symbol.iterator so it will complain. I think since transpileDecalarations uses noChecck it should probably fall back on the isolated declaration test and just emit the entity name even if it can't check the type is correct. @weswigham thought?

[weswigham]

it should probably fall back on the isolated declaration test and just emit the entity name even if it can't check the type is correct

It is. The issue is that the check for the ID error (in visitDeclarationSubtree in declarations.ts) relies on elements of the Symbol global resolving, in this case. Specifically, that resolver.isEntityNameVisible check in the condition. If the checker isn't running and you have no other files in the program, the name isn't gonna resolve for a missing global. Yeah, that would be a checker error if the checker was running in the same noResove-ish single-file mode, but if you're not even capturing those... ??? The error behavior here is checker dependent because of the entity name lookup - hence the difference in command-line (where the global resolves) and API (where it doesn't) behavior.

Symbols are a big hole in isolatedDeclarations usability that we don't have great answers for right now. For builtin symbols, I could easily remove the error by passing a more complete lib into the transpileDeclarations program, so Symbol.iterator is recognized as a valid late-bound property, but that's basically a stop-gap. Custom symbols like

const sym = Symbol();
export class Foo {
  public [sym]() {
    return {
      next: () => ({ value: 42, done: false }),
    };
  }
}

I've currently patched into emitting "correctly", but still issuing the isolatedDeclarations error - I imagine this case should be similar. The ID error should appear in all cases (even when the name fails to resolve), but the emit should be reasonably sane.

[MichaelMitchell-at]

still issuing the isolatedDeclarations error

Would you happen to know of a clean and concise way to annotate code following this pattern to satisfy isolatedDeclarations? IIRC they can't be suppressed by @ts-expect-error (on my phone so can't validate quickly)

[weswigham]

The issue with the symbol computed names is that there isn't one - to write it you'd still need a type with a symbol computed name, which then has the same issue. Call it a flaw with the whole idea that you can do declaration emit without expression checking, since computed names mean even types can rely on (nonlocal!) expression evaluation - minimally you need to make breaking assumptions that expressions in computed names probably resolve to valid property names that probably don't conflict with other property names.

Or you give up and do some limited checking-like analysis - looking for references to the global Symbol and members thereof. This is probably the direction we'll go - just finding the right way to slice the analysis is weird. It's unclear how much of the lib we should bring in.

[MichaelMitchell-at]

Hm. Wonder if it would be too special casey to look for expressions that match the pattern Symbol.iterator satisfies unique symbol and emit without error in those cases