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

TypeScript 4.6 regression #48118

Open
thetutlage opened this issue Mar 4, 2022 · 9 comments
Open

TypeScript 4.6 regression #48118

thetutlage opened this issue Mar 4, 2022 · 9 comments
Assignees
Labels
Docs The issue relates to how you learn TypeScript

Comments

@thetutlage
Copy link

thetutlage commented Mar 4, 2022

Bug Report

I have read the release notes and the code that is breaking for me is not mentioned in the breaking changes section.

🔎 Search Terms

None. As the code works with 4.5

🕗 Version & Regression Information

The typeof var === 'function' call narrows the variable to function vs one of the union.

  • This changed between versions 4.5 and 4.6

⏯ Playground Link

Playground link with relevant code

💻 Code

type Context = Record<string, any>

class Foo<T extends Context> {  
  private contextAccumlator!: () => T | Promise<T>

  constructor(
    context: T | (() => T | Promise<T>)
  ) {
    if (typeof context === 'function') {
      this.contextAccumlator = context
    }
  }
}

🙁 Actual behavior

I expect this.contextAccumlator = context to report zero errors.

🙂 Expected behavior

Instead it says

Type '(() => T | Promise<T>) | (T & Function)' is not assignable to type '() => T | Promise<T>'.
  Type 'T & Function' is not assignable to type '() => T | Promise<T>'.
    Type 'Context & Function' provides no match for the signature '(): T | Promise<T>'.(2322)
@tristan00b
Copy link

@thetutlage I played around with this a bit and reduced your example:

function isFunction<T extends Function>(it: unknown): it is T
{
  return typeof it === 'function'
}

function isFn(it: unknown): it is Function
{
  return typeof it === 'function'
}

function f<T>(arg: T | (() => T))
{
  let x: () => T
 
  if (typeof arg === 'function')
    x = arg // (error) arg: (() => T) | (T & Function)

  if (isFunction(arg))
    x = arg // (works) arg: () => T

  if (isFn(arg))
    x = arg // (works) arg: () => T
}

This can be reproduced as far back as version 3.3.3 (play).

@thetutlage
Copy link
Author

🤔 Did you mean there is no regression?

@jihndai
Copy link
Contributor

jihndai commented Mar 4, 2022

Weirdly enough, it seems those are two different cases. This issue's sample code was indeed working by 4.5.5, broken in 4.6.2, and still broken in 4.7.0-dev.20220304. While the second example was never fixed as far as I can tell.

@tristan00b
Copy link

Maybe a recent change caused the old bug to surface in @thetutlage 's code.

@RyanCavanaugh
Copy link
Member

This is a correct error that was not properly found before:

type Context = Record<string, any>

class Foo<T extends Context> {  
  private contextAccumlator!: () => (T | Promise<T>)

  constructor(context: T | (() => T | Promise<T>)
  ) {
    if (typeof context === 'function') {
      this.contextAccumlator = context
    }
  }

  invoke() {
    this.contextAccumlator();
  }
}

function isPromise(arg: any): arg is Promise<any> {
  return false;
}

const p = new Foo<(n: string) => void>(n => n.substring(0));
// Crashes
p.invoke();

@RyanCavanaugh RyanCavanaugh added the Docs The issue relates to how you learn TypeScript label Mar 4, 2022
@thetutlage
Copy link
Author

thetutlage commented Mar 5, 2022

So shouldn't this be under breaking changes?

@RyanCavanaugh
Copy link
Member

Yes, that's why it's a Docs bug assigned to the blog post author

@thetutlage
Copy link
Author

Okay! Thanks for the clarification :)

@jakebailey
Copy link
Member

jakebailey commented Apr 16, 2024

This was "fixed" in #49119; should it have been?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Docs The issue relates to how you learn TypeScript
Projects
None yet
Development

No branches or pull requests

6 participants