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 narrowing in arrow function #32300

Closed
matt-tingen opened this issue Jul 8, 2019 · 17 comments
Closed

Inconsistent narrowing in arrow function #32300

matt-tingen opened this issue Jul 8, 2019 · 17 comments
Labels
Question An issue which isn't directly actionable in code Working as Intended The behavior described is the intended behavior; this is not a bug

Comments

@matt-tingen
Copy link

TypeScript Version: 3.6.0-dev.20190704

Search Terms: inconsistent union narrow const initialization arrow function

Code

interface OpenState {
  isOpen: true;
}

interface ClosedState {
  isOpen: false;
}

type State = OpenState | ClosedState;

const state: State = { isOpen: false };

const arrow = () => state;
function fn() { return state }
class Foo {
  method() { return state }
  boundMethod = () => state
}

type Arrow = typeof arrow
type Fn = typeof fn
type Method = typeof Foo.prototype.method
type BoundMethod = typeof Foo.prototype.boundMethod

Expected behavior:
Arrow === Fn === Method === BoundMethod
What that type should be is #31734

Actual behavior:
Arrow === () => ClosedState
Fn === Method === BoundMethod === () => State

Playground Link: Playground

Related Issues: #31734, #8513, possibly dupe of #29260 but I don't think so

A couple things to highlight:

  1. Changing const state to let state achieves the expected behavior with a value of State.
  2. Hovering over const state indicates that it is State, even though typeof state === ClosedState. I did not create a separate issue for that because it seems likely that that is a symptom of this issue rather than a distinct issue with the language server.
@MartinJohns
Copy link
Contributor

Also:

const state: State = { isOpen: false };
state.isOpen = true; // Type 'true' is not assignable to type 'false'.

@matt-tingen
Copy link
Author

I believe that would fall under #31734 which relates to what type state would be here. This issue is specifically about similar consumptions of the same type having different types.

@sandersn sandersn changed the title Inconsistent narrowing for const union in arrow function Inconsistent return type widening in arrow function Jul 8, 2019
@sandersn sandersn changed the title Inconsistent return type widening in arrow function Inconsistent narrowing in arrow function Jul 8, 2019
@RyanCavanaugh RyanCavanaugh added the Question An issue which isn't directly actionable in code label Jul 8, 2019
@sandersn
Copy link
Member

sandersn commented Jul 8, 2019

The root cause is that control flow narrowing applies to arrows and function expressions. It's not unique to arrows:

const fexpr = function () { return state }

Now typeof fexpr === typeof arrow.

The reason is that function and class declarations are hoisted -- so control flow analysis doesn't apply because the compiler isn't sure when they will run. They use the declared type of state: State. However, () => state is not hoisted, and (optimistically) captures the narrowed type of state: ClosedState at the point that it is captured.

@sandersn sandersn added the Working as Intended The behavior described is the intended behavior; this is not a bug label Jul 8, 2019
@matt-tingen
Copy link
Author

Thanks for the explanation, @sandersn. It makes sense to me that hoisted values would be handled differently, but I don't 100% understand this difference. I need to sit down and formulate some follow-up questions. Is this an acceptable place to ask them or should I create an SO question for follow-up?

Also, does the fact that hovering over state in const state (in the playground, vscode, etc) shows state: State seem like expected behavior in this scenario?

@sandersn
Copy link
Member

sandersn commented Jul 8, 2019

This is an OK place for follow-up since there's discussion here already, but use SO as the default for future questions.

const state displays the declared type, which is the type pre-assignment. I think this makes sense because you're asking for the type at the declaration, after all.

@fatcerberus
Copy link

class declarations are hoisted

I know this isn't really relevant to the issue but: are they? This is a runtime error, e.g.:

new C();  // ReferenceError: Cannot access 'C' before initialization
class C {}

I guess the C identifier itself is hoisted, but the class itself is not. Which makes sense because you can use arbitrary expressions in the extends clause.

@matt-tingen
Copy link
Author

Right, just as with var and function, the declaration is hoisted, but not the initialization. Your example is analogous to:

let C;
new C();
C = class C {};

@fatcerberus
Copy link

fatcerberus commented Jul 9, 2019

Right, but function is fully hoisted: You can safely call the function before it's "in scope".

fn();  // hunky dory
function fn() { console.log("Hello world!"); }

Which is why your fn is treated differently from arrow.

@matt-tingen
Copy link
Author

Yep, my mistake about the function hoisting.

@sandersn, looking at this a bit more, I'm still not clear on the originally discussed typing difference. If it's a limitation, I can understand, but it's not clicking for me why it would be intended behavior. To be clear, my intention is not to nit over this issue's classification, but to understand TS at a deeper level.

More specifically, I'm getting caught up on "control flow analysis doesn't apply because the compiler isn't sure when they will run". If fn were invoked prior to const state, it would throw. If invoked after, I would expect it to have the narrowed return type since it's no longer "taking advantage of hoisting" so to speak and would behave the same as a function expression. Am I missing something in thinking that the uninitialized declaration of a const cannot be accessed?

In short, I don't see any cases where state can actually be used as State without being guaranteed to throw.

Additionally, as @fatcerberus pointed out, class implementations are not hoisted so it seems like they would be treated in a similar manner to arrow/function expressions.

@fatcerberus
Copy link

I’m actually a bit confused myself because I had thought local narrowings were always discarded at function boundaries, except for IIFEs where the narrowings are retained inside the function body for obvious reasons. Your case doesn’t involve an IIFE so I’d have expected even the arrow function to automatically widen to State.

@RyanCavanaugh
Copy link
Member

The underlying question here is whether, in a given position, TS should use the declared type or the control-flowed type.

For a declaration, which inherently does not belong in a control flow graph, it doesn't make any sense to talk about the control-flowed type, because it has no place in the graph from which to compute which narrowings are in effect. The code inside those declarations can run at any "time" (including times at which a const is TDZ'd, for example)

This isn't the case for an expression that initialized a const - this initialization takes place at a knowable time in the control flow graph, and it makes sense to use the control-flowed type. Here we don't widen because it's a const binding; if you change const to let you'll see that arrow has type () => State.

You can call these behaviors inconsistent, which they are, except that different code really should cause different behavior. "For consistency, this program should be treated identically to this nearly-equivalent program" is always at most two hops away from a paradox

@matt-tingen
Copy link
Author

Thanks, Ryan, that helped to bring the issue into focus a bit more.

Now I understand why accessing a let in a hoisted scope would use the declared type for that let; there's any number of narrowings that may occur after declaration and the hoisted scope could be invoked after any (or none) of them, so we just have to use the widest type which is the declared type.

For const, though, it still seems to me that the widest type would be the narrowed initialization. Sure, the hoisted function could be called while the const is in the TDZ, but wouldn't the type of the const effectively be the narrowed type | never (which simplifies to the narrowed type)? The declared type may be the most correct technically but I'm failing to see how it's of any use practically.

@RyanCavanaugh
Copy link
Member

It's really unknowable which behavior you "wanted" when you typeof an expression from a place that isn't in the same control flow graph as the originating expression. We conversely sometimes get bugs that people don't want the control-flowed type when using typeof in a control-graphed position, for example, so it's not something that is clearly settled.

I think more to the point, if you wrote

const x: T = e;

instead of

const x = e;

when T is a supertype of typeof e, there is some intent communicated by that - disregarding the type annotation when outside the control flow graph is arguably actively discarding intent written by the program's author.

@fatcerberus
Copy link

disregarding the type annotation when outside the control flow graph is arguably actively discarding intent written by the program's author.

I agree with this, but the correlary to that is: why should const x: T = ... be doing any narrowing to a subtype of T in the first place; I think that’s the more surprising behavior to me. If I declare it const with an explicit type, I’d personally expect it to always be exactly the type I said it was, even if there’s something more specific that matches.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 10, 2019

People write code like this and expect it to work:

type Method = "GET" | "POST" | "PATCH";
declare function doSomething(x: "GET" | "POST") { ... }

// later
const x: Method = "GET";
doSomething(x);

It'd be really weird to have const behave worse than let in this regard, since you definitely want narrowing based on assignments to lets. It'd also be a new and worse inconsistency to have initializations be ignored but not assignments.

@matt-tingen
Copy link
Author

matt-tingen commented Jul 10, 2019

I disagree that there's necessarily intent there.
I often write const x: T = e because it provides code completion and verifies against typos in ways that an assertion would not necessarily catch e.g.

interface Foo {
    bar?: number
}

const foo = { baz: 3 } as Foo // Compiles; allows for typo
const foo2: Foo = { baz: 3 }  // Error; catches typo

Perhaps I need to be doing both.

#7481 would also remedy this if it were implemented with a prefix notation.


FWIW, I was also surprised that const x: T = ... narrows. IMO, that is #31734, though it is highly related to this.

It was particularly surprising that hovering over const x displays what is a seemingly unusable type.

@typescript-bot
Copy link
Collaborator

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

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 Working as Intended The behavior described is the intended behavior; this is not a bug
Projects
None yet
Development

No branches or pull requests

6 participants