ember-cli-typescript icon indicating copy to clipboard operation
ember-cli-typescript copied to clipboard

[@types/ember__object bug] set() is not strongly typed

Open mschorsch opened this issue 6 years ago • 3 comments
trafficstars

Which package(s) does this problem pertain to?

  • [ x ] @types/ember__object

What are instructions we can follow to reproduce the issue?

ember new sample; cd ./sample # Create a new ember app
ember install ember-decorators # Set up decorator support
ember install ember-cli-typescript # Set up typescript support
Reproduction Case
import EmberObject, {set} from "@ember/object";

class Sample extends EmberObject {

  name!: string;
}

const sample: Sample = Sample.create({ name: "test" });
sample.set("name", 12346); // No error => Incorrect
sample.set<Sample>("name", 12456); // Error => Correct
set(sample, "name", 12346); // Error => Correct

Now about that bug. What did you expect to see?

In all cases typescript should throw an error.

What happened instead?

sample.set("name", 12346); is accepted by the typescript compiler.

Note

Observable in package @ember/object/observable contains an overloaded function set with the following definitions:

set<K extends keyof this>(key: K, value: this[K]): this[K];
set<T>(key: keyof this, value: T): T;

The second function seems to be unnecessary.

Workaround and possible solution

Remove set<T>(key: keyof this, value: T): T; from Observable.

mschorsch avatar Mar 20 '19 19:03 mschorsch

Thanks for opening this! We've all been super busy with wrapping up EmberConf and traveling – one of us will take a look in the next few (work) days!

chriskrycho avatar Mar 22 '19 01:03 chriskrycho

I believe this is the same as (or caused by the maybe-temporary fix for) https://github.com/typed-ember/ember-cli-typescript/issues/286

dfreeman avatar Oct 24 '19 14:10 dfreeman

It's caused by the tempory fix in #286.

https://github.com/typed-ember/ember-cli-typescript/issues/286#issuecomment-508133056

The "fix" should be reverted because we have no type safety anymore ...

mschorsch avatar Oct 24 '19 15:10 mschorsch

As far as I can tell, this remains an issue on the TS side, but is largely irrelevant since every use of this.get() and this.set() in Ember can be safely replaced with a normal ES property access or assignment statement, including in Ember Data. Closing as “not planned” accordingly.

chriskrycho avatar Sep 28 '23 22:09 chriskrycho