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

Inconsistent type validation: Different result depending on inline defined and executed function and declared and executed function #59560

Closed
medikoo opened this issue Aug 8, 2024 · 17 comments
Labels
Question An issue which isn't directly actionable in code

Comments

@medikoo
Copy link

medikoo commented Aug 8, 2024

🔎 Search Terms

"inline function", "type validation"

🕗 Version & Regression Information

  • This changed between versions ______ and _______
  • This changed in commit or PR _______
  • This is the behavior in every version I tried, and I reviewed the FAQ for entries about _________
  • I was unable to test this on prior versions because _______

⏯ Playground Link

https://www.typescriptlang.org/play/?#code/MYewdgzgLgBFCm0YF4YAoBuBDANgV3gC4ZoAnASzAHMBtAXQEpiMRyATFAPhgG8BfANwAoIQHpRMAOohSAawgwA8rKEJoaDQy4waAIjVRddGFgWhIUBmgYNhYiQDEs5HArQB3ABYv4MclAByBXgARzxybBx4MFgoEBgAM3JSaAYhcyRSRBAcDHgAQVIqBVRrbT0DIxMzcGhhAzQsiBy8wuLrWyEgA

💻 Code

const test = (value: string[]): void => {};

// Works Ok
test((() => ["test"] as const)());

// Fails (while it's equivalent to first)
const resolveArgs = () => ["test"] as const;
test(resolveArgs());

🙁 Actual behavior

The first invocation is accepted as one without issues. The second which is equivalent, is rejected with the:

Argument of type 'readonly ["test"]' is not assignable to parameter of type 'string[]'.
  The type 'readonly ["test"]' is 'readonly' and cannot be assigned to the mutable type 'string[]'.(2345)

🙂 Expected behavior

Both should be accepted

Additional information about the issue

No response

@nmain
Copy link

nmain commented Aug 8, 2024

Different inferences in the face of different contextual types doesn't seem like a bug.

Could you fill out the issue template completely -- is this a regression? And if so, from when?

@medikoo
Copy link
Author

medikoo commented Aug 8, 2024

Different inferences in the face of different contextual types doesn't seem like a bug.

I think it all depends on what are our expectations from this library

I believe we all agree, both executions are equivalent (in sense that in both cases test function receives input that's exactly same typewise), yet they are recognized differently. Why?

It feels as either logical flaw or logical limitation in Typescript.

Could you fill out the issue template completely -- is this a regression? And if so, from when?

All I know is that it happens in v5.5.4. I'm new to the TypeScript so it's hard for me say, if it's something new, or by design for a long time

@MartinJohns
Copy link
Contributor

In the first example you pass the function directly to test, so the compiler can infer based on the context that the value ["test"] as const should be typed as string[].

In your second example the function is on its own. The compiler does not know that it's supposed to be used as a string[] later, so it's using the narrow type readonly ["test"] (because of the as const).

This is neither a logical flaw, nor a logical limitation, but working as intended.

@medikoo
Copy link
Author

medikoo commented Aug 8, 2024

so the compiler can infer based on the context that the value ["test"] as const should be typed as string[].

Why can't the compiler infer the same information when the function is passed on its own?

The implementation of this function is just as accessible as of one passed directly and the compiler can read through it in the same way.

It's clear that both versions work exactly the same, and nothing different can happen in either case. If the compiler is unable to deduce what is evident in these few lines of code, it seems like a limitation on the compiler's side.

@MartinJohns
Copy link
Contributor

MartinJohns commented Aug 8, 2024

Because resolveArgs can be used further down in the code, so it can't use the contextual type information on one specific call site.

This could be somewhere else in the code, and if the type of resolveArgs is inferred based on the usage in test, then it wouldn't be compatible with foo anymore. Since the compiler can't know where the function will be used, it can't use a more specific type and uses the information that is available (the as const).

function foo(arg: readonly ["test"]) {}
foo(resolveArgs())

It's clear that both versions work exactly the same, and nothing different can happen in either case

The second version introduces a new variable, the first one not. It's a major difference.

@medikoo
Copy link
Author

medikoo commented Aug 8, 2024

The compiler can clearly see that this function is not used anywhere else. However, let's put that aside. I understand that there might be an agreement not to make the compiler work in a way where newly introduced code (in this case, additional function usage) changes how previous code is validated.

Now, taking it further, I still don't understand how ["test"] as const can be interpreted differently in both cases. Isn't it equivalent to readonly ["test"] in all cases?

If I understand correctly, you are stating that in the case of test((() => ["test"] as const)());, the ["test"] as const is not inferred as equivalent to readonly ["test"]. Is that correct?

Why is ["test"] as const considered a valid input in the first usage but no longer considered as such in the second one? This value is not subject to represent anything else under any circumstances

@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Aug 8, 2024
@RyanCavanaugh
Copy link
Member

It feels as either logical flaw or logical limitation in Typescript.

I think there are four-ish logical endpoints for this kind of analysis:

  1. No-helping mode, where even obviously-okay code is still rejected if it would also be rejected under indirection
  2. Single-order, the kind that you see today where immediately-observed expressions can get contextual treatment, but not indirectly-referenced ones
  3. A hazy middle ground full of inconsistencies where things work at e.g. four levels of indirection and not five, which is hard to tease out because the differences only start appearing in large samples
  4. Infinite analysis that can detect every possible indirection/non-indirection

Option 3 is not something that's tractable to do with reasonable performance. Option 0 also seems bad; people would complain a lot if obviously-fine code errored for no good reason. So you're asking why we don't do option 2 vs option 1, and I think the description of option 2 explains why

@medikoo
Copy link
Author

medikoo commented Aug 8, 2024

Thanks, @RyanCavanaugh, for this insight. It sounds very reasonable. I can imagine that aiming for option 2 would be like opening Pandora's box.

Agreeing that we're on option 1, I still have trouble understanding why the above fails. Let's group some facts:

  • resolveArgs unconditionally returns ["test"] as const. No matter where or how many times it's used, it's the outcome, and the compiler can easily infer that (no need to go beyond "1")
  • test declares that it will accept any array of strings.

["test"] as const is a valid member of the set of all possible values that will satisfy the string[] declaration. Why, then, is this match reported as invalid?

Why does introducing a second function that takes ["test"] as const, but which declares readonly ["test"]) as the only valid input, changes our validation approach to a function which accepts string[]

@RyanCavanaugh
Copy link
Member

The bare inference of ["test"] as const is readonly ["test"]. Note that it's readonly, and you're trying to pass it to a function that takes a mutable array. That's not legal.

When an as const'd array literal appears in a place where a mutable array is expected, it's allowed to subtype back to the mutable version of the array, since it can be seen that no one else has a reference to the array that would become unsound after the array is mutated.

@medikoo
Copy link
Author

medikoo commented Aug 8, 2024

Note that it's readonly, and you're trying to pass it to a function that takes a mutable array. That's not legal.

Indeed, I missed that in the above reasoning. Technically test assumes rights to mutate the input, therefore readonly input should be rejected.

it's allowed to subtype back to the mutable version of the array,

This is an actual source of confusion. I think the fact that the given array is not referenced anywhere else shouldn't dismiss that it was declared as const.

For example, the following code passes perfectly:

const test = (value: string[]): void => { value.push("test2"); };
test(["test"] as const);

What's interesting is that it will fail for ["test"] as read-only string[]. Why is mutation on ["test"] as const allowed conditionally?

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Aug 8, 2024

I think the fact that the given array is not referenced anywhere else shouldn't dismiss that it was declared as const

Why? That breaks code like this:

declare function foo<T>(arg: readonly T[]): T[];

// p1: string[], but I wanted ("one" | "two")[]
const p1 = foo(["one", "two"]);

// Actual: p2: ("one" | "two")[] ✅
// Proposed: Error here instead, but
//           what should the user write instead?
const p2 = foo(["one", "two"] as const);

Why is mutation on ["test"] as const allowed conditionally?

See above example - a "fresh" const array literal can be safely treated as mutable

@medikoo
Copy link
Author

medikoo commented Aug 8, 2024

@RyanCavanaugh Why exactly will this break? I'm not sure if I follow this example.

Doesn't ["one", "two"] as const satisfy readonly T[] on its own (without downgrading to mutable)?

I see that foo takes a readonly array of elements of type T and returns a mutable array of elements of type T. I believe there's no assumption that the input and output are the same objects; the only assumption is that they are of the same type. Right?

@RyanCavanaugh
Copy link
Member

Sorry, I meant this:

declare function foo<T>(arg: T[]): T[];

// p1: string[], but I wanted ("one" | "two")[]
const p1 = foo(["one", "two"]);

// Actual: p2: ("one" | "two")[] ✅
// Proposed: Error here instead, but
//           what should the user write instead?
const p2 = foo(["one", "two"] as const);

@typescript-bot
Copy link
Collaborator

This issue has been marked as "Question" and has seen no recent activity. It has been automatically closed for house-keeping purposes.

@typescript-bot typescript-bot closed this as not planned Won't fix, can't repro, duplicate, stale Aug 12, 2024
@medikoo
Copy link
Author

medikoo commented Aug 12, 2024

@RyanCavanaugh I'm not sure if it makes any real difference. What do I miss?

Why exactly there's a room for any error at p2 case?

I read it as: foo receives the tuple of literal types "one" and "two", therefore the expected output structure is ("one" | "two")[].
Similarly as if I'd pass [1, 'string'], the expected output structure would be (number | string)[].

I'm not sure how it explains that in previous example we allow mutation for ["test"] as const input

This issue has been marked as "Question" and has seen no recent activity

?? That's interesting. So if I do not respond to the comment on the issue during the weekend the issue will be autoclosed? :) Where's the work/life balance thing? ;-)

@RyanCavanaugh
Copy link
Member

Why exactly there's a room for any error at p2 case?

You said

Why is mutation on ["test"] as const allowed conditionally?

The proposed behavior is that ["one", "two"] as const be inferred as a readonly array, but it's illegal to pass a readonly array to foo

@medikoo
Copy link
Author

medikoo commented Aug 12, 2024

The proposed behavior is that ["one", "two"] as const be inferred as a readonly array, but it's illegal to pass a readonly array to foo

Thanks for responding. I see now that mutability is tied to type declarations. In this case, the input was declared as T[], indicating a mutable array of type T. That part wasn’t entirely clear to me at first.

what should the user write instead?

I guess ("one" | "two")[] could work in that case(?), but I understand that it would be both inconvenient and error-prone to list all possible items by hand.

Therefore, if there's no convenient syntax to pass this information (as mutable), I assume TypeScript intentionally allows a hack where the user can pass an immutable that can be subtyped into a mutable.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Question An issue which isn't directly actionable in code
Projects
None yet
Development

No branches or pull requests

5 participants