Skip to content

Commit

Permalink
Disable a type safety/correctness assertion
Browse files Browse the repository at this point in the history
In `CoreObject.create()`, we currently have an unsafe cast when
constructing an instance with props, because the code assumes one of
two things. Either: the `factory` is always set, *or* it is safe to
call `setFactoryFor` with `undefined`. A number of acceptance tests are
(incorrectly? Unclear!) relying on the ability to run through this path
with `factory` being `undefined`. It's *possible* that actually means
that the type for `setFactoryFor` should allow `undefined`, but we
typed it the other way for good reason! Accordingly, this *casts*
`factory`, and the commented-out `assert()` is here in the hope that we
can enable it after addressing tests in the future.
  • Loading branch information
chriskrycho committed Nov 17, 2022
1 parent 900b66e commit 8172773
Showing 1 changed file with 10 additions and 2 deletions.
12 changes: 10 additions & 2 deletions packages/@ember/object/core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -768,9 +768,17 @@ class CoreObject {

if (props !== undefined) {
instance = new this(getOwner(props));
// TODO(SAFETY): at present, we cannot actually rely on this being set,
// because a number of acceptance tests are (incorrectly? Unclear!)
// relying on the ability to run through this path with `factory` being
// `undefined`. It's *possible* that actually means that the type for
// `setFactoryFor()` should allow `undefined`, but we typed it the other
// way for good reason! Accordingly, this *casts* `factory`, and the
// commented-out `assert()` is here in the hope that we can enable it
// after addressing tests *or* updating the call signature here.
let factory = getFactoryFor(props);
assert(`missing factory when creating object ${instance}`, factory !== undefined);
setFactoryFor(instance, factory);
// assert(`missing factory when creating object ${instance}`, factory !== undefined);
setFactoryFor(instance, factory as NonNullable<typeof factory>);
} else {
instance = new this();
}
Expand Down

0 comments on commit 8172773

Please sign in to comment.