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

Returned assert functions need needlessly explicitly typed #56695

Closed
jas7457 opened this issue Dec 6, 2023 · 6 comments
Closed

Returned assert functions need needlessly explicitly typed #56695

jas7457 opened this issue Dec 6, 2023 · 6 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@jas7457
Copy link

jas7457 commented Dec 6, 2023

🔎 Search Terms

"assert function", "react hook", "Assertions require every name in the call target to be declared with an explicit type annotation", "2775"

🕗 Version & Regression Information

  • This is the behavior in every version I tried

⏯ Playground Link

Link

💻 Code

import React from "react";

function invariantFn(condition: unknown, message: string): asserts condition {
  if (!condition) {
    throw new Error(message);
  }
}

function useInvariant() {
  return invariantFn;
}

type Filter = {
  title: string;
} | null;

function Test1({ filter }: { filter: Filter }) {
  // using the hook version doesn't work, even if I return exactly the same function
  const invariant = useInvariant();
  invariant(filter, "Filter necessary");
  console.log(filter.title);

  // explicitly typing the hook version works? WAT
  const invariant2: ReturnType<typeof useInvariant> = useInvariant();
  invariant2(filter, "Filter necessary");
  console.log(filter.title);

  return <div>{filter.title}</div>;
}

function Test2({ filter }: { filter: Filter }) {
  // calling the funciton directly works fine
  invariantFn(filter, "Filter necessary");
  console.log(filter.title);

  return <div>{filter.title}</div>;
}

🙁 Actual behavior

I need to manually type the function returned from my hook, even though it is the same exact function used without needing a type.

🙂 Expected behavior

TypeScript should allow me to return an assert function from a hook or other function and not need to manually type it at the callee location.

Additional information about the issue

I'm trying to create an invariant function that will act similar to the npm invariant package. I'd like to put it into a React hook so that I can use another hook to grab our logger, and then return a function that will call the invariant function and our logger.

Even with this slimmed down example that I provided, without any of the logging logic, TS is unhappy and forces me to provide some type annotation for every consumer of the hook.

@RyanCavanaugh
Copy link
Member

This is an intentional trade-off since the performance characteristics of making the CFA graph include any possible function call are quite bad (in other words, this error message isn't in the code by accident).

@RyanCavanaugh RyanCavanaugh added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Dec 6, 2023
@jas7457
Copy link
Author

jas7457 commented Dec 6, 2023

This is an intentional trade-off since the performance characteristics of making the CFA graph include any possible function call are quite bad (in other words, this error message isn't in the code by accident).

Sorry, I'm not familiar with what a CFA graph is. But what's the difference here if I manually type it? Shouldn't both be of type Invariant, so I'd think both should act exactly the same?

@MartinJohns
Copy link
Contributor

MartinJohns commented Dec 6, 2023

I'm not familiar with what a CFA graph is.

CFA stands for control flow analysis.

But what's the difference here if I manually type it?

It's a design decision to only analyze assertion calls that are explicitly typed (for performance reasons). That's why the one version works, the other not. This is intentional. See also the implementing PR: #32695

@fatcerberus
Copy link

It’s interesting to me that the second version works, since useInvariant doesn’t have an explicit return type either.

@MartinJohns
Copy link
Contributor

since useInvariant doesn’t have an explicit return type either.

It doesn't need to. The identifiers of your expression need to refer to explicitly typed values. When calling invariant2(..) it checks if invariant2 is explicitly typed, and it is explicitly typed as (condition: unknown, message: string) => asserts condition. The asserting call is not calling useInvariant.

@fatcerberus
Copy link

Huh, I guess return type inference happens before CFA then (since otherwise the compiler wouldn't be able to resolve ReturnType<typeof useInvariant> as an assertion functon type at that time)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

4 participants