GeezerVison

Hi Donald Pipowitch (@donaldpipowitch), I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!

This seems like a small (but important) contribution, so no Contribution License Agreement is required at this point. Real humans will now evaluate your PR.

TTYL, MSBOT;

Related to #12870?

Thanks for the PR. but as discussed in #12870, we do not believe this is a good change.

Thank you for the fast reply, but I'm very sad to hear this. It makes working with noImplicitAny a lot harder.

const foo = {
  bar: 'bar',
  baz: 'baz'
};
Object.keys(foo).forEach(key => console.log(foo[key].length));  // error with noImplicitAny

To fix this you need to write console.log((foo as any)[key].length) or const foo: { [key: string]: string; }. I think both solutions are suboptimal :(

Mohamed Hegazy (@mhegazy) I can't really follow the arguments from #12870 and I'm not sure that is what I want in most cases. Can you give me an example written in TypeScript where my PR would behave bad? And if I want this behaviour wouldn't it be easier to do this than?:

const foo = {
  bar: 'bar',
  baz: 'baz'
};
Object.keys(foo).forEach(key => console.log(foo[key as string].length));

function printProperties(foo: {}) {
    Object.keys(foo).forEach(key => console.log(key.toString())); // Error toString does not exisit on `never` 
}

There is code that looks like this today. changing this would be a breaking change, and would not really provide much value.

you can define your function:

const keys = <{ <T>(o: T): Array<keyof T> }>Object.keys;

function printProperties(foo: {}) {
    Object.keys(foo).forEach(key => console.log(key.toString())); // Error toString does not exisit on `never` 
}

So if I say my foo is an empty object, we need a way to say that keys returns an empty array (or for the sake of it an empty array which would contain string), because never[] is wrong.

But I'd argue that foo is typed in wrong way in the first place. The function should be called something like that, if want to print and accept any object:

function printProperties(foo: { [s: string]: any; }) {
    Object.keys(foo).forEach(key => console.log(key.toString()));
}

I'd say that is more explicit and more correct.

There is code that looks like this today.

Granted... if that is the case and you want to avoid a breaking change here, this PR could be postponed. But I'm still not convinced that this "works as intended" as labeled in #12870.

Mohamed Hegazy (@mhegazy) Just would like to give some feedback. We use noImplicitAny in our config and colleagues which don't know TypeScript in detail are often confused when they write const foo = { bar: 'hello' }; Object.keys(foo).forEach(key => console.log(foo[key])); and see an error message. It's hard to explain why they have to cast foo to any.

Are there proposals to add some kind of modifier or mapping to certain types? So I can map never to something else just inside Object.keys so this legacy code doesn't throw?

function printProperties(foo: {}) {
    Object.keys(foo).forEach(key => console.log(key.toString()));
}

Or would it be possible to invent some special ignore behaviour so I know when Object.keys gets never the forEach can be ignored? (Just dream code here...)

Or a totally different question: Currently it seems that my PR wasn't merged, because it could be a breaking change. Are there already plans for TypeScript 3.0 and maybe some lists where possible breaking changes could be collected and introduced?