TypeScript error with `Owner` interface
robostheimer opened this issue · 3 comments
Recently, I tried to upgrade a package my teams owns to v. 3.2.0 and a type error started to be emitted:
node_modules/@linkedin/hue-web-icons/addon-test-support/setup-icon-sprite.ts(14,11): error TS2551: Property 'unregister' does not exist on type 'Owner'. Did you mean 'register'?
It appears that maybe unregister was removed from the
Owner` interface.
This was actually the result of typing Owner
correctly with the intended semantics for it, per RFC 0821. It is not the full set of everything that happens to have organically grown onto the EngineInstance
and ApplicationInstance
types over most of a decade, but rather the desired (and minimal necessary) interface for an Owner
. Since Owner
as a public type was new at the time of that RFC, we (Framework and TS teams together) decided not to introduce it with a bunch of methods we would just want to turn around and deprecate, causing significant churn later, but instead to introduce it with only the API it needs. The result is a small amount of churn now for folks who need to tweak how they handle it from a TS POV, but hopefully a much lower amount of churn for the community overall.
In the context of the kind of code where you’re seeing it, you can either do an unsafe cast to ApplicationInstance
or you can do a check to make it type safe. The latter is what I recommend! Here’s how that would look:
import { module, } from 'qunit';
import { setupTest } from 'ember-qunit';
import ApplicationInstance from '@ember/application/instance';
import { assert as debugAssert } from '@ember/debug';
module('demo safe approach', function (hooks) {
setupTest(hooks);
hooks.beforeEach(function () {
debugAssert(
'owner should be an AppliationInstance in tests',
this.owner instanceof ApplicationInstance
);
this.owner.unregister('some:registration');
});
});
You can see this working in this playground. You may need to make some further tweaks here, e.g. it might need to be EngineInstance
from @ember/engine/instance
, but this is the general shape of what you need to make this safe.
As a bonus: I told the team responsible for that particular usage that this would be a problem several months ago. 🙃
@chriskrycho , wouldn't it make more sense to cast it in ember-test-helpers
instead of having to do it in a consuming app every time we need access to Owner?
Totally reasonable question! Unfortunately, though, no: we typed it correctly from the POV of @ember/test-helpers
, because it is legal for users to do all sorts of things with owners in tests, including setting their own custom owner—it just has to conform to the Owner
API. People can and do use that in the real world. Whether they should is a different question, but it’s a supported path so our types have to be accurate about it. 😣