microsoft / TypeScript

TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
https://www.typescriptlang.org
Apache License 2.0
101.12k stars 12.5k forks source link

[isolatedDeclarations][5.5] Type annotations needed for literals containing enum values #58394

Open MichaelMitchell-at opened 6 months ago

MichaelMitchell-at commented 6 months ago

πŸ” Search Terms

enum, isolated declarations

βœ… Viability Checklist

⭐ Suggestion

Enum values seem trivially inferrable if the enum is defined in the lexical scope, so isolated declarations shouldn't report them as errors.

πŸ“ƒ Motivating Example

export enum Foo {
  A = 1,
  B = 2,
}

export const x = Foo.A as const;
             ~
export const y = [Foo.A] as const;
                  ~~~~~

Playground: https://tsplay.dev/w68Prm

πŸ’» Use Cases

Self-apparent

RyanCavanaugh commented 6 months ago

I'm not 100% convinced this is safe. @dragomirtitian , thoughts?

dragomirtitian commented 6 months ago

We generally don't look up any symbols beyond the current expression. So Foo.A could really be anything not necessarily an enum. Looking up the symbol in the same file is feasible, but what would only allow you to write this if the enum is in the same file. And looking at symbols across files goes against one of the design goals of isolated declarations (allowing single file .d.ts emit)

So we could do it in some cases. Our though process is that generally we should be consistent. It would be a bit strange for this to work only within a file.

I think there is a case to be made that we could allow more of these in isolated declarations if there is demand for it. But I think this limitation should stay for v1 of this feature.

MichaelMitchell-at commented 6 months ago

That's fair. An idea I haven't put much thought into is to emit something like

export enum Foo {
  A = 1,
  B = 2,
}

export declare const x: typeof Foo.A;
export declare const y: readonly [typeof Foo.A];

when the subject of typeof is a valid expression. At least that would allow deferring any potential errors until later, e.g. with "skipLibCheck: false".

I think a hard case might be something like:

export enum Foo {
  A = 1,
  B = 2,
}

export const x = {
  [Foo.A]: 1,
  [Foo.B]: 2,
} as const;

since you'd have to maybe emit something like

export enum Foo {
  A = 1,
  B = 2,
}

export declare const x: {
  readonly [K in typeof Foo.A]: 1;
} & {
  readonly [K in typeof Foo.B]: 2;
}

which I'd be ok with, but perhaps beyond the level of complexity you'd want to implement in tsc

dragomirtitian commented 6 months ago

For computed properties we already allow this and error in TS if the type is not a late bindable computed property (ie it's not a property that can be a computed key and would have t be turned into a string index)

For other positions, we did debate doing typeof Identifier. It's not totally off the table as a possible future improvement, but it is not always 100% accurate so there would have to be places where const x = y is an error because const x: typeof y is not actually the correct type.

enum E { A = 1, }

let x = E.A;
const y = E.A // equivalent to typeof E.A

const s = Symbol();
const z = s // not equivalent to typeof s

Playground Link

This is a class of inferences we called optimistic inferences. We did however rule them out for the time being as being too confusing to explain why this would fail sometimes.

MichaelMitchell-at commented 6 months ago

Subtle, didn't realize unique symbols had such unique behavior πŸ™‚

MichaelMitchell-at commented 6 months ago

@dragomirtitian I think I found a false negative, a case that should error but doesn't. mapping should report an error here. Interestingly, the ts.transpileDeclaration API does detect the error. There may be some other false negatives I found as well. Will create a separate issue once I've condensed them down into repro cases.

// @filename: a.ts
export enum Foo {
  A = 'a',
  B = 'b',
}

// @filename: b.ts
import {Foo} from './a';

export const mapping = {
  [Foo.A]: 1,
  [Foo.B]: 2,
}

Playground