[BUGFIX release] Fix types for getOwner
and GlimmerComponent
#20257
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The type for
getOwner()
is intended to returnOwner
in cases where we can safely assume that the object instance does in fact have an owner. However, as pointed out by @bendemboski today in Discord, this does not work correctly in the scenario where we have a component which actually has a signature defined:Presently, we return
Owner | undefined
instead.This PR fixes this by using
GlimmerComponent<any>
instead of plainGlimmerComponent
, since the latter resolves to the defaultGlimmerComponent<unknown>
, which ends up being too narrow to resolve correctly withgetOwner
when any actual signature is applied.As a bonus, rename
FrameworkObject
in this context to the somewhat more verboseKnownFrameworkObject
, which avoids a conflict with the actualFrameworkObject
type used internally.Additionally: The type for
getOwner()
is technically unsafe, but this unsafety is one which end users will rarely hit in practice and, courtesy of Ember's current types, is unavoidable ifgetOwner(someService)
is to correctly returnOwner
instead ofOwner | undefined
.The Typed Ember folks have discussed this point at some length, but I realized on looking at this to make the changes in the first two commits for this PR that we didn't have it committed to the code base anywhere, so this PR also introduces a long comment explaining it. When we switch from preview to stable types, this comment should come along!