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

Primitive type guards require typeof to be on the left hand side of the comparison #9020

Closed
dargoner opened this issue Jun 8, 2016 · 11 comments
Assignees
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue

Comments

@dargoner
Copy link

dargoner commented Jun 8, 2016

Typecript method, the parameter does not support the function and the character of the joint type, prompt TS2345 error

image

@yortus
Copy link
Contributor

yortus commented Jun 8, 2016

The error goes away for me after re-arranging the type guards in on as follows:

    ...
    if (typeof fn == "string") {

    }
    if (typeof fn != "function" || !type) {
        return false;
    }
    ...

@mindplay-dk
Copy link

I'm running into something similar.

I'm getting the error about parentElement here even though the statement is wrapped in isHTMLElement which returns an assertion that should make parentElement known as HTMLElement inside that block.

@yortus
Copy link
Contributor

yortus commented Jun 8, 2016

Note the following compiler behaviour (with typescript@next at least):

function foo(x: string|number) {
    if (typeof x === "string") {
        x // x is narrowed to string here
    }

    if ("string" === typeof x) {
        x // x is NOT narrowed here, still string|number
    }
}

Seems weird that the two equivalent forms are treated differently. But that's what it does at present.

@yortus
Copy link
Contributor

yortus commented Jun 8, 2016

@mindplay-dk I think yours is a different problem - on the error line you linked to, the parentElement reference is inside the body of a nested function expression. Narrowing doesn't cross function boundaries except in very specific cases (see #8849).

@mhegazy mhegazy added the Bug A bug in TypeScript label Jun 8, 2016
@mhegazy mhegazy added this to the TypeScript 2.0 milestone Jun 8, 2016
@sandersn
Copy link
Member

sandersn commented Jun 8, 2016

The spec provides only a few fixed forms for narrowing primitives. typeof x === 'string' narrows, but 'string' === typeof x does not.

As a workaround, you can write a type predicate:

function isString(x: any): x is string {
  return "string" == typeof x;
}
// later...
if (isString(fn) || !type) {
  return false;
}

@sandersn sandersn closed this as completed Jun 8, 2016
@sandersn sandersn added the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Jun 8, 2016
@sandersn
Copy link
Member

sandersn commented Jun 8, 2016

We discussed this offline and decided to change the spec in this case.

@sandersn sandersn reopened this Jun 8, 2016
@sandersn sandersn removed the By Design Deprecated - use "Working as Intended" or "Design Limitation" instead label Jun 8, 2016
@sandersn sandersn changed the title Typecript TS2345 Error Primitive type guards require typeof to be on the left hand side of the comparison Jun 8, 2016
@mhegazy mhegazy modified the milestones: TypeScript 2.1, TypeScript 2.0 Jun 8, 2016
@mindplay-dk
Copy link

Narrowing doesn't cross function boundaries

@yortus why not? this works with 1.8.10, it only started failing with the nightly.

@yortus
Copy link
Contributor

yortus commented Jun 9, 2016

@mindplay-dk control flow based type analysis (landed in #8010) has tightened up type guard type inference. You are right that it appears narrowing does cross function boundaries in v1.8, but the new behaviour is more accurate. The compiler does not know whether the function expression will be called immediately, or perhaps later, by which time parentElement might have been reassigned and no longer satisfy the type guard. That is, the parentElement reference is lexically nested inside the type guard, but may not be temporally nested. So control flow analysis makes the conservative assumption not to narrow inside the function expression body.

The workaround is straightforward but a bit ugly - just introduce a new variable of the narrowed type inside the guarded block:

...
if (isHTMLElement(parentElement)) {
    const parentHTMLElement = parentElement;
    var nodes = toArray(parentElement.querySelectorAll(inlineElementSelector) as NodeListOf<HTMLElement>)
        .filter(el => isInlineWithStyle(window.getComputedStyle(parentHTMLElement), el))```
    ... 

@mindplay-dk
Copy link

@yortus hmm, introducing an arbitrary variable as a work-around... stuff like that raises questions for someone reading it - like, if I were to see something like that, my immediate inclination would be to factor away a useless intermediary variable... ugly in deed :-/

@yortus
Copy link
Contributor

yortus commented Jun 9, 2016

@mindplay-dk yeah it's a smell, and it's not the only place you need to add a superfluous variable to make type inference happy. But in this case at least, I can't imagine how tsc could infer that it's safe to narrow inside the function expression without god-like knowledge of the runtime behaviour of the code.

Maybe in your case it's cleaner to just use an explicit type assertion, i.e. <HTMLElement> parentElement, inside the function expression.

@mindplay-dk
Copy link

@yortus thanks, a type assertion - yes, that's cleaner :-)

@mhegazy mhegazy added the Fixed A PR has been merged for this issue label Jun 13, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Fixed A PR has been merged for this issue
Projects
None yet
Development

No branches or pull requests

5 participants