MakePrimitiveRef generic so that UNDEFINED_REFERENCE can be Reference<undefined> rather than Reference<unknown> #1481
Conversation
…<undefined> rather than Reference<unknown>
NullVoxPopuli
changed the title
| @@ -56,8 +56,8 @@ class ReferenceImpl<T = unknown> implements Reference<T> { | |||
| } | |||
| } | |||
|
|
|||
| export function createPrimitiveRef(value: unknown): Reference { | |||
| const ref = new ReferenceImpl(UNBOUND); | |||
| export function createPrimitiveRef<T = unknown>(value: T): Reference<T> { | |||
There was a problem hiding this comment.
Don’t think you need = unknown here.
I wonder if you will catch any unexpected/unintended usages if you do T extends string | number | boolean | null | undefined (we may have a type alias for that already somewhere)
There was a problem hiding this comment.
Don’t think you need = unknown here.
I will try this out in a followup PR!
Since it's so hard to iterate between this PR and ember, I want try as isolated changes at a time as possible
There was a problem hiding this comment.
Sure. The = unknown is redundant regardless of whether the constraints work out or not. As far as I know it would always have inferred (to unknown in the worst case). For types/interface sometimes you need to say = unknown to be able to reference the type more easily, but I don’t think that’s needed in a function parameter position
There was a problem hiding this comment.
we may have a type alias for that already somewhere
looks like it's just duplicated everywhere -- probably good for better errors, tbh
ran in to this when trying to update glimmer-vm in ember-source, here: emberjs/ember.js#20561
in particular:
this should be resolved after this PR