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

allow narrowing from any #9999

Closed
zpdDG4gta8XKpMCd opened this issue Jul 28, 2016 · 120 comments
Closed

allow narrowing from any #9999

zpdDG4gta8XKpMCd opened this issue Jul 28, 2016 · 120 comments
Labels
Breaking Change Would introduce errors in existing code Committed The team has roadmapped this issue Help Wanted You can do this Suggestion An idea for TypeScript

Comments

@zpdDG4gta8XKpMCd
Copy link

zpdDG4gta8XKpMCd commented Jul 28, 2016

There are numerous questions as to why any cannot be narrowed from. Despite being natural and intuitive this feature was rejected due to being a serious breaking change to poorly written yet massive code bases.

Please consider putting it behind a flag provided the evident need for it.

TLDR

  • any is not what you think it is (poor naming at play)
  • for narrowing use type unknown = {} | undefined | null | void instead
  • yes, it's a hot unfortunate mess
  • no, there is no painless way to fix it
  • yes, you gonna need to fix your *.d.ts by hands
  • why? to make average JS developers happy with their less than perfect code
  • welcome to the club! get your badge at the reception

UPDATE
@yortus made a branch per #9999 (comment) where narrowing from any works!

try it: npm install typescript-narrowany

@jesseschalken
Copy link
Contributor

Can you specify a use case? any is already compatible with everything and everything is compatible with it, so why do you need to narrow it?

The whole point of any is to opt out of static typing. If you don't want to opt out, but want a type that can be anything but which you have to narrow before assuming it is something, that's what {} is for.

This code passes:

function foo(x:any) {
    if (typeof x === 'string') {
        needs_string(x);
    }
}

function needs_string(x:string) {}

and so does this:

function foo(x:{}) {
    if (typeof x === 'string') {
        needs_string(x);
    }
}

function needs_string(x:string) {}

@yortus
Copy link
Contributor

yortus commented Jul 28, 2016

@jesseschalken here are some use cases:

A. Narrowing the error variable in a catch block, which is of type any and can't be annotated. The user did not 'opt-out' of type checking here, they just have no choice.

B. Retaining type safety when we explicitly use type guards on an any variable:

function validate(x: any) {
    x.whatever(); // no error, but that's OK since we've opted out of checking due to `any`

    if (Array.isArray(x)) {
        // we've EXPLICITLY opted-in to type checking now with a compile-time AND runtime array check

        x.Pop(); // oops typo, but no error!?
        x.whatever(); // no error!?
        x(); // no error!?
    }
}

C. Refactoring using tools like VSCode

Suppose in the following example we use the VSCode 'rename' tool to rename Dog#woof to Dog#bark. The rename will appear to work but will miss the guarded woof reference. The code will still compile without errors, but crash at runtime.

// An example taken directly from #5930 as a reason why any should NOT be narrowed
// By contract, this function accepts only a Dog or a House
function fn(x: any) {
  // For whatever reason, user is checking against a base class rather
  // than the most-specific class. Happens a lot with e.g. HTMLElement
  if (x instanceof Animal) {
    x.woof(); // Disallowed if x: Animal
  } else {
    // handle House case
  }
}

EDIT: Adding a fourth use case that came up later in #9999 (comment):

D. Consuming definitions from lib.d.ts and other third-party libraries. lib.d.ts and many (most?) other type declaration files on DefinitelyTyped use any to mean "this value could be anything". These values cannot be narrowed in consumer code, even though the consumer did not ask to opt-out of type checking.

@yortus
Copy link
Contributor

yortus commented Jul 28, 2016

I think @RyanCavanaugh summed up the issues around narrowing any:

On the one hand, listen to users, and on the other hand, listen to users...

However, TypeScript currently only caters to the one set of users.

👍 for a flag to cater to both sets of users, since both groups have valid needs.

@RyanCavanaugh
Copy link
Member

Serious question @Aleksey-Bykov -- how are there even any anys left in your codebase?

@jesseschalken
Copy link
Contributor

@yortus

A. Eeek. That's unfortunate. There should definitely be a compiler option to make caught errors {} instead of any.

B & C. I see. The problem then is that TypeScript enables you to mix dynamic and static typing, but you can't have the same variable be dynamically typed in one place and statically typed in another place. Dynamic vs static is a property of the variable itself and you can't transition in/out of dynamic mode without creating a new variable or using as with every reference, or redirecting references through a getter.

I don't care personally because I try to stay as far away from any as possible and consider the solution to any problems with any to be to simply remove or insulate myself from it. If the use case of mixed dynamic/static typing wrt the same variable is sufficiently common for mixed static/dynamic codebases I guess it's a worthy concern.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Jul 28, 2016

Unorganized thoughts, some contradictory

  • I find the "rename" scenario very compelling. Being able to safely refactor is a major thing for us and it feels like we're just screwing up here when seen through that lens
  • I don't think you get to have your strictness cake and eat it too. There are lots of good workarounds here -- declaring a parameter as {}, or as { [s: string]: any} and using string indexer notation to access unsafe properties, or writing const xx: {} = x; and narrowing on that once you're doing unsafe things with x. Seeing "real" code that can't reasonably accommodate those workarounds would be useful.
  • There have been a few suggestions for something like T extends any where you get strictness on the known properties of T but anys when accessing other properties. That seems like the both-sides-win behavior that avoids a new flag but accomplishes most of what's desired here
  • We should allow a type annotation of exactly : any or : { } on a catch variable. Why not.
  • Many problems described here would be well-mitigated by a type-based TSLint rule that disallowed dotted property access of expressions of type any. I'd certainly turn that rule on for all my projects

@yortus
Copy link
Contributor

yortus commented Jul 28, 2016

It's possibly just me, but I find {} to be a visually-confusing annotation that makes code intent less clear.

x: {} just looks at a glance like the code is saying 'x is an Object instance', when in reality it means 'x is absolutely anything but don't opt out of type checking'. So it might be an object, array, number, boolean, RegExp, string, symbol, etc.

Probably a stupid suggestion but is it worth considering adding a type keyword that's exactly like {} but more readable? Perhaps x: unknown or similar?

@jesseschalken
Copy link
Contributor

You can do

type unknown = {}

right now if you want.

@yortus
Copy link
Contributor

yortus commented Jul 28, 2016

You can do type unknown = {} right now if you want.

True, as long as you don't mind adding boilerplate for than in every file you need it. Also type aliases lose their pretty name in compiler messages and intellisense.

@zpdDG4gta8XKpMCd
Copy link
Author

@RyanCavanaugh

how are there even any anys left in your codebase?

  • JSON.parse - for a saved state which may vary
  • window.onMessage - for upcoming messages which may vary
  • narrowing exceptions (already mentioned)

but you are right, not that many left

@jesseschalken
Copy link
Contributor

jesseschalken commented Jul 28, 2016

@yortus It depends how your project is set up, but if you define it at the top level, outside a module or namespace, it will be available across the entire project without needing to be imported, just like the things in lib.d.ts are. Fair point about compiler messages/intellisense though.

@zpdDG4gta8XKpMCd
Copy link
Author

@RyanCavanaugh
one more case where we have any is for functions that deal with arbitrary input like deepCopy or areEqual

@yortus
Copy link
Contributor

yortus commented Jul 28, 2016

Dynamic vs static is a property of the variable itself and you can't transition in/out of dynamic mode without creating a new variable

@jesseschalken I assume you are pointing out this is how things currently are, not how they necessarily should be. From #5930 it's appears the team had no problem with the same variable transitioning from untyped to typed in the explicitly protected region of a type guard. After all, their first instinct was to implement narrowing from any.

It only got backed out when a subset of users, whose code is written according to the old proverb "just as all Animals are Dogs, so all non-Animals must be Houses", were offended by the compiler pointing out the flaw in their logic, and would rather silence the compiler than fix their code.

Having said that, their reasoning that "I used any to opt-out of all type checking" is also reasonable. However I agree with the team's apparent first instinct, that sure any does opt you out of type checking, but a type guard is a nice explicit way of saying "opt me back in".

@mcclure
Copy link

mcclure commented Jul 31, 2016

I would just like to repeat what I said in #8677, which is that you need to do something to make try-catching Error subclasses manageable while receiving the benefits of typechecking. That could be allowing instanceof to narrow any, it could be a new instanceof-like construct which narrows (maybe extends), it could be some kind of special construct for catches like the one @bali182 suggests in #8677. However right now the combination of decisions you have made has put users of custom exception classes in a terrible box.

I am writing an TypeScript program that uses exceptions, and right now I have had to multiple times include the following eight(!) line idiomatic construct just to catch an exception:

    } catch (_e) {
        if (_e instanceof SomeCustomError) {
            let e:SomeCustomError = _e
            console.log(e.someField) // Or whatever
        } else {
            throw _e
        }
    }

That's very awkward and contains a lot of repetition (and therefore potential for typos). Compare with the clear, compact analogous code in C#:

    } catch (SomeCustomError e) {
        log(e.someField) // Or whatever
    }

You should do something to make this less awkward in TypeScript; allowing instanceof to narrow would help a lot while still resulting in code that looks like standard ES6.

This said, two things:

  • I think a compiler flag is not a good solution. I should be able to just write code, paste it into stackoverflow etc and have it be portable, a compiler flag means that some of my typing goes away unless I distribute with build metadata always. If this were some obscure edge case it would be one thing, but the case I'm working with is "catching an exception", which is a major feature.

  • A suggestion is made in this thread that one solution would be special-case allowing the exception in a catch clause to be typed as : {} because technically this opts back in to narrowing-instanceof. I think this is a really bad solution, this is extremely non-discoverable and it would be very nonobvious to a layperson coming across TypeScript code what the : {} tag does. I had to read this thread a couple of times before I understood which of : {} or : any it is that would activate narrowing and why, and I've been writing Typescript for a number of months now. To a new user it would be even more baffling, the :{} would just look like ASCII gibberish.

    Overall, you have an education problem right now around the surprising behavior of type narrowing when any is involved (ie instanceof works sometimes, then in the presence of an any silently loses some of its powers). As I note in a bug on the TypeScript handbook I filed last night "instanceof" type narrowing section is incorrect or ambiguous TypeScript-Handbook#353, the handbook is actually currently incorrect in how it describes the current behavior here! Ideally the solution to what to do with instanceof would make the education problem easier rather than worse.

@jesseschalken
Copy link
Contributor

@mcclure If it's any help you can reduce some of that boilerplate with something like this:

function narrowError<T>(e:{}, c:new(...x:any[]) => T):T {
    if (e instanceof c) {
        return e;
    } else {
        throw e;
    }
}

class SomeCustomError {
    constructor(public someField:string) {}
}

try {
    // ...
} catch (e_) {
    let e = narrowError(e_, SomeCustomError);
    console.log(e.someField);
}

@zpdDG4gta8XKpMCd
Copy link
Author

@RyanCavanaugh but then how come this works?

function isArray(value: any): value is any[] {
     return Object.prototype.toString.call(value) === '[object Array]';
}
const something = Math.random () > 0.5 ? [] : 1;
if (isArray(something)) {
    console.log(something.length);
}

@RyanCavanaugh
Copy link
Member

@Aleksey-Bykov because something is any[] | number ?

@zpdDG4gta8XKpMCd
Copy link
Author

but it doesn't matter since isArray(value: any) declared with any, does it?

@RyanCavanaugh
Copy link
Member

The parameter type of a type guard isn't used in the narrowing logic

@zpdDG4gta8XKpMCd
Copy link
Author

😮 🔫

@zpdDG4gta8XKpMCd
Copy link
Author

zpdDG4gta8XKpMCd commented Aug 1, 2016

it just blows out the rest of my mind, why the type of the parameter which is being narrowed isn't used? from what do we narrow then? why would we care to specify it at all?

@alitaheri
Copy link

alitaheri commented Aug 1, 2016

It's used for the function body. I use it for generics Action<any> => Action<Payload>

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Aug 1, 2016

I don't understand what the alternative would be. Many type guards can effectively determine any input value, and need to do some weird stuff in their implementation, so any as a parameter type makes a lot of sense. What else would you write?

@alitaheri
Copy link

alitaheri commented Aug 1, 2016

Honestly, I treat any like I treat dynamic from C#. any means opt out of type-system. Not "this can be anything" nor opt-out a little bit. I think instead of changing the behavior of any ( which does what it should: out-out! period ) it would be better to introduce a new primitive like never something like dynamic or mixed that can be anything without opting out of type-system. just a thought though. The current behavior of any is very useful for rapid development. I use it at first then when the API is stable I remove all occurrences replacing them with solid types.

Besides, other problems the any type causes should be resolved on their own. we shouldn't change the behavior of any.

P.S. @Aleksey-Bykov an Array with extra params checked against Array.isArray will be run-time valid but compile-time invalid. it will break a lot of code to change this behavior.

@zpdDG4gta8XKpMCd
Copy link
Author

zpdDG4gta8XKpMCd commented Aug 1, 2016

well... i thought that there is no alternative at all to begin with, hence this topic - allow narrowing from any, which makes me feel silly because narrowing from any does work already, just probably not to anything and not from anything, and what those from and to are is a question yet to be explored

turns out i don't know what type guards really are, i thought that typguards are predicates that operate on a value of a type they are declared for

still trying to wrap my head around the idea that there are types (only one type?) that get ignored when used for parameters of type guards at least (anything else?)

@zpdDG4gta8XKpMCd
Copy link
Author

zpdDG4gta8XKpMCd commented Aug 1, 2016

and of course this one doesn't work:

declare const something : any;
function isArray(value: any): value is any[] {
     return Object.prototype.toString.call(value) === '[object Array]';
}
if (isArray(something)) {
    console.log(something.length);
}

how can i trust what i see after i've seen that:

function isArray(value: any): value is any[] {
     return Object.prototype.toString.call(value) === '[object Array]';
}
if (isArray(something)) {
    console.log(something.length);
}

which might or might not work depending what any is.... .... .... ... ..

@alitaheri
Copy link

@Aleksey-Bykov anything inferred/declared as any cannot change it's type to anything rather than any no matter what. type guards don't change the type of their parameter. they change the type of argument passed at call site. if that argument is any then it cannot be changed.

@zpdDG4gta8XKpMCd
Copy link
Author

ok, i think i got it, it's just trickier than i though it was:

  • typeguards operate on a type of the argument not on the type of their parameter, the type of the parameters just sets the boundaries to the type of an acceptable argument

uff da, that's been a ride

wiki needs a topic on this, thanks

@jesseschalken
Copy link
Contributor

Object because it's uselessly close to {}

I thought they were the same thing?

@RyanCavanaugh
Copy link
Member

The distinction is super subtle but they are not identical. One observable difference is

let x: Object = { hasOwnProperty : 4 }; // error
let y: {} = { hasOwnProperty : 4 }; // ok

@jesseschalken
Copy link
Contributor

let y: {} = { hasOwnProperty : 4 }; // ok
y.hasOwnProperty('foo'); // also ok? huh?

Sorry for getting off topic, but shouldn't { hasOwnProperty : 4 } be an invalid object literal? It's just shorthand for let y = new Object(); y.hasOwnProperty = 4; and you can't assign a method to a number.

@RyanCavanaugh
Copy link
Member

RyanCavanaugh commented Aug 12, 2016

We can't have the notion of an invalid object literal. That's basically nonsense.

This all happens because of apparent members. These are properties that appear during property access and in the source of an assignability check, but not in other places. They're there to mimic JS's prototype chain and boxing behavior. So when you write let x = { hasOwnProperty: 4}, that's fine, and we know that you can't call hasOwnProperty on x (because it shadows the copy given to it by Object#prototype). If you alias away a property (as shown in your example y) you can make a lot of bad things happen which is why we try to not make that happen in normal use.

Apparent members are also why you can invoke methods like toFixed or substr on primitives, even though primitives themselves have no members. The primitives gain the apparent members of their corresponding object types (strings get the apparent members of String, etc).

@jesseschalken
Copy link
Contributor

I see. So basically everything has the methods from Object, but compatibility with Object isn't actually enforced until something is assigned to Object directly. That's certainly not sound, but I assume it's just another compromise made to avoid scaring people with too much soundness, so meh.

@RyanCavanaugh RyanCavanaugh added Help Wanted You can do this Committed The team has roadmapped this issue Breaking Change Would introduce errors in existing code and removed In Discussion Not yet reached consensus labels Aug 12, 2016
@RyanCavanaugh RyanCavanaugh added this to the TypeScript 2.1 milestone Aug 12, 2016
@RyanCavanaugh
Copy link
Member

👍 for the behavior described at #9999 (comment)

Summary:

  • Narrowing to primitives continues to work as it does today
  • Narrowing via instanceof or a user-defined type predicate will narrow unless the narrowed-to type is exactly Function or exactly Object (in which case the expression remains of type any)
  • No commandline switch - this is the behavior in all cases

@yortus do you want to update your fork and own the PR for this? It should be a quick change compared to what you have -- just remove the switch logic and compare the target using === to globalFunctionType / globalObjectType

@yortus
Copy link
Contributor

yortus commented Aug 13, 2016

@RyanCavanaugh PR submitted: #10319 #10334

@DanielRosenwasser DanielRosenwasser modified the milestones: TypeScript 2.0.1, TypeScript 2.1 Aug 15, 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
Breaking Change Would introduce errors in existing code Committed The team has roadmapped this issue Help Wanted You can do this Suggestion An idea for TypeScript
Projects
None yet
Development

No branches or pull requests