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

Defer indexed access T[K] with non-generic K #12770

Merged
merged 6 commits into from
Dec 10, 2016
Merged

Conversation

ahejlsberg
Copy link
Member

@ahejlsberg ahejlsberg commented Dec 8, 2016

With this PR we defer resolution of indexed access types T[K] where T is generic and K is non-generic.

function getValue<T extends { value: string | number }>(obj: T): T['value'] {
    return obj.value;
}

let x1 = getValue({ value: 'hello' });  // string
let x2 = getValue({ value: 42 }); // number

In the example above, the indexed access type T['value'] is permitted because T is constrained to a type that has a property named value. The type T['value'] behaves like a subtype of string | number because that is the type of value in the constraint type, and when instantiation substitutes an actual type for T, the type of the value property in that actual type is substituted for T['value'].

Note that for backwards compatibility reasons, the expression obj.value is still eagerly resolved to type string | number. The expression must be explicitly coerced to T['value'] using a type annotation or a type assertion in order to get the deferred behavior.

This PR also implements better circularity detection and reporting for indexed access types. For example:

type T1 = {
    x: T1['x'];  // Error
};

Note that circularities may sometimes result only for certain arguments to type parameters.

type T2<K extends 'x' | 'y'> = {
    x: T2<K>[K];  // Error
    y: number;
 }
 
declare let obj: T2<'x'>;
let x = obj.x;

If the type of obj above is changed to T2<'y'> the circularity error goes away.

Fixes #12651.
Fixes #12723.
Fixes #12744.

# Conflicts:
#	src/compiler/checker.ts
#	tests/baselines/reference/keyofAndIndexedAccess.js
#	tests/baselines/reference/keyofAndIndexedAccess.symbols
#	tests/baselines/reference/keyofAndIndexedAccess.types
#	tests/cases/conformance/types/keyof/keyofAndIndexedAccess.ts
@eschwartz
Copy link

I just want to point out that once this is fixed, we should be able to implement a better EventEmitter definition, with typed events.

For example

// Encapulate an event's name and data
interface EventType<TName extends string, TData extends any> {
  name: TName;
  data: TData;
}

class EventEmitter<TEventType extends EventType<string, any>> {
  on<TType extends TEventType>(name: TType["name"], listener:(event: TType["data"]) => void):this;
  /*...*/
}

Usage:

type EventTypeA = EventType<'a', { foo: string; }>;
type EventTypeB = EventType<'b', { bar: string; }>;

const emitter = new EventEmitter<EventTypeA | EventTypeB>();

// emitter.on requires that the event name and data match the provided EventType
emitter.on<EventTypeA>('a', (evt) => {
  evt.foo;
});
emitter.on(EventTypeB>('b', (evt) => {
  evt.bar;
});

@eschwartz
Copy link

Any idea when this might be released? Should we be expecting a 2.1.5 to come out soon with this fix?

(also, thanks for all the work on this. This is really cool stuff, and I'm having a ton of fun using it)

@rotemdan
Copy link

rotemdan commented Dec 10, 2016

I believe I've found an alternative, though less elegant, approach to my dispatcher example that doesn't rely on discriminated unions and might work with the new inference capability described here without any need for additional syntax:

type DispatcherSchema = { [name: string]: { argTypes: any[]; returnType: any } };

class Dispatcher<S extends DispatcherSchema> {
	dispatch<K extends keyof S>(name: K, args: S[K]['argTypes']): S[K]['returnType'] {
		// ...
	}
}

type MySchema = {
	"read": { argTypes: [string, number]; returnType: string[]; };
	"write": { argTypes: [string, string[], boolean]; returnType: number; };
}

const dispatcher = new Dispatcher<MySchema>();

var result = dispatcher.dispatch("read", ["myfile.txt", 35]); // ok
var result = dispatcher.dispatch("read", "myfile.txt"); // already errors
var result = dispatcher.dispatch("reed", ["myfile.txt", 35]); // already errors
var result = dispatcher.dispatch("write", ["myfile.txt", "oops", true]); // might error with this PR?

It seems promising but I have not tested this yet though (I'm waiting for the pull request to be merged and arrive to the nightly builds).

(If this does prove to work I'll probably start applying this pattern literally immediately in my code...)

@ahejlsberg ahejlsberg merged commit 57cb4ac into master Dec 10, 2016
@ahejlsberg ahejlsberg deleted the deferIndexedAccess branch December 10, 2016 23:15
@eschwartz
Copy link

Just downloaded typescript@2.2.0-dev.20161212, and my EventEmitter example is working great 👍

@eschwartz
Copy link

Any chance this can make it into 2.1.5?

@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
None yet
Projects
None yet
5 participants