Skip to content
This repository has been archived by the owner on Sep 12, 2019. It is now read-only.

type instability of the newValue change argument #8

Open
jasongrout opened this issue Dec 12, 2015 · 9 comments
Open

type instability of the newValue change argument #8

jasongrout opened this issue Dec 12, 2015 · 9 comments

Comments

@jasongrout
Copy link
Member

The type instability of the newValue and oldValue arguments to an event is frustrating. It means that pretty much all event handlers need to cast newValue and oldValue (like https://github.com/jasongrout/jupyter-js-notebook/blob/4cf0e81d019f13bb8992c68b57cc25f4fb5f27bf/src/NotebookWidget.ts#L103)

@sccolbert
Copy link
Member

Do you have a suggestion for something better?

@sccolbert
Copy link
Member

We could make newValue and oldValue just T[] instead of T | T[], which has precedence: https://msdn.microsoft.com/en-us/library/system.collections.specialized.notifycollectionchangedeventargs(v=vs.110).aspx

But I imagine that would just cause a different set of complaints.

@jasongrout
Copy link
Member Author

Actually, I would prefer T[] just for consistency. I can imagine people not using types might complain that it is a bit harder, but newValue[0] is not too much harder than newValue.

@sccolbert
Copy link
Member

It's also two extra memory allocations on every change, when they aren't strictly needed (and why I originally went this route)

@jasongrout
Copy link
Member Author

On the other hand, if Typescript was smart enough to have type literals and the switch statement as a type guard, the problem would go away from the typescript perspective. That may be coming soon, at least for string literal typing.

@jasongrout
Copy link
Member Author

Yeah, I was also wondering about efficiency.

@sccolbert
Copy link
Member

newValue as T is only 2 characters longer than newValue[0]

@sccolbert
Copy link
Member

and the same length as <T>newValue

but I guess if you are spelling out T it would be savings

@jasongrout
Copy link
Member Author

See microsoft/TypeScript#1003 - const enum literals may solve the issue, in that typescript can be smarter about inferring types.

# for free to subscribe to this conversation on GitHub. Already have an account? #.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants