Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

4.3: as const looking for the identifier const in a generic class #44292

Closed
orta opened this issue May 27, 2021 · 6 comments · Fixed by #44311
Closed

4.3: as const looking for the identifier const in a generic class #44292

orta opened this issue May 27, 2021 · 6 comments · Fixed by #44311
Assignees
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue

Comments

@orta
Copy link
Contributor

orta commented May 27, 2021

Bug Report

We got a report that as const stopped being special case syntax from NordicSemiconductor/cloud-e2e-bdd-test-runner-js#188

🔎 Search Terms

as const 4.3 Cannot find name 'const'

🕗 Version & Regression Information

  • This changed between versions 4.2 and 4.3

⏯ Playground Link

type Store = { a: 123 }
export type Cleaner = <W extends Store>(runner: FeatureRunner<W>) => Promise<any>

export class FeatureRunner<W extends Store> {
	private readonly cleaners: Cleaner[] = []

	async runFeature(): Promise<any> {
		const objectWhichShouldBeConst = {
			flags: {},
			settings: {},
		} as const

//  ERR: Cannot find name 'const'.
	
		return objectWhichShouldBeConst
	}

	async run(): Promise<any> {
		const result = {}
		// Removing this line gets rid of the error above
		this.cleaners.forEach(c => c(this))
		return result
	}
}

Workbench Repro

🙁 Actual behavior

Compiler error from bad lookup for const

🙂 Expected behavior

No error

@ahejlsberg ahejlsberg self-assigned this May 27, 2021
@ahejlsberg ahejlsberg added the Bug A bug in TypeScript label May 27, 2021
@ahejlsberg ahejlsberg added this to the TypeScript 4.3.2 milestone May 27, 2021
@ahejlsberg
Copy link
Member

ahejlsberg commented May 27, 2021

What a strange issue. It's caused by a local AST walk we do in isTypeParameterPossiblyReferenced. During that walk we resolve every type node to see if it references a particular type parameter, but we're not excluding const type references in as type assertions. So we try to look up const and we fail. The issue existed before 4.3, it just so happens no one has triggered it yet.

@ahejlsberg
Copy link
Member

Following #43696 we now call isTypeParameterPossiblyReferenced on method function types where we didn't before, and that's what's causing the issue to show up. But the actual problem is that we don't properly exclude as const in the AST walk.

@ahejlsberg
Copy link
Member

Actually, it does appear that #43696 is the direct cause of this issue. For methods that have a type annotation, the changes in that PR cause isTypeParameterPossiblyReferenced to walk over the entire AST of the method, including the body, when really it should just examine the type parameters, parameters, and return type node. That in turn then triggers the as const issue because the logic is only designed to walk over types and not expressions and statements.

Not too hard to fix, I'll put up a PR.

@ahejlsberg
Copy link
Member

ahejlsberg commented May 27, 2021

BTW, here's a smaller repro:

export class C<T> {
    f(): void {
        let one = 1 as const;  // Error, but shouldn't be
    }
}
new C<string>().f();

@forivall
Copy link

i also ran into this issue in my codebase, and was able to produce this repro case:

export class A<T> {
  public myMethod(): void {
    const a = 'str' as const;
  }
}
const a = new A();

export class B<T> extends A<
  T
> {
  public static staticMethod<T>(
  ): B<T> {
    return new B();
  }
}

@DanielRosenwasser
Copy link
Member

This can't be scheduled for TypeScript 4.3.2 because that was already shipped last week.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Fix Available A PR has been opened for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants