Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Documentation: Recommended return type of synchronous hasMany relationships is wrong #1449

Open
mspoehr opened this issue Sep 16, 2021 · 1 comment
Labels
docs types:core:data Something is wrong with the Ember Data type definitions types:core Something is wrong with the Ember type definitions

Comments

@mspoehr
Copy link

mspoehr commented Sep 16, 2021

According to ember-cli-typescript's ember-data Model documentation, the type of synchronous has many relationships is EmberArray<T extends Model>:

The type returned by the @hasmany decorator depends on whether the relationship is { async: true } (which it is by default).

  • If the value is true, the type you should use is DS.PromiseManyArray<Model>, where Model is the type of the model you are creating a relationship to.
  • If the value is false, the type is EmberArray<Model>, where Model is the type of the model you are creating a relationship to.

This is incorrect. According to Ember 3.28's documentation:

In contrast to async relationship, accessing a sync relationship will always return a ManyArray instance containing the existing local resources. But it will error on access when any of the known related resources have not been loaded.

This makes sense because it would mean that async relationships return a type of PromiseManyArray<Model>, while sync relationships return ManyArray<Model>. Ember-cli-typescript's types for DS even define this:

type AsyncHasMany<T extends Model> = PromiseManyArray<T>;
type SyncHasMany<T extends Model> = ManyArray<T>;

I've updated my typings of sync relationships to MutableArray<Model> instead of EmberArray<Model>, as ManyArray extends MutableArray and I don't want to import DS. Is this the correct approach? My recommendation would be to update the documentation to reflect this approach.


Small code example to drive my point home:

import Model, { hasMany } from '@ember-data/model';
import EmberArray from '@ember/array';
import Process from './process';

export default class Thread extends Model {
  @hasMany('process', { async: false })
  declare processes: EmberArray<Process>

  addProcess(process: Process) {
    this.processes.addObject(process);
  }
}

With EmberArray<Process>, addObject has a type error:

Property 'addObject' does not exist on type 'Array<Process>'.

However, the code does work if typechecking is disabled, and there are no type errors when using processes: MutableArray<Process> from @ember/array/mutable.

@chriskrycho
Copy link
Member

Thanks for the report! This makes sense; Ember Data has made a lot of progress and the types have changed a lot over the 3.x series (in ways which are backwards compatible, to be clear), and we haven't kept up!

@chriskrycho chriskrycho added docs types:core Something is wrong with the Ember type definitions labels Sep 16, 2021
@chriskrycho chriskrycho added the types:core:data Something is wrong with the Ember Data type definitions label Sep 29, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs types:core:data Something is wrong with the Ember Data type definitions types:core Something is wrong with the Ember type definitions
Projects
None yet
Development

No branches or pull requests

2 participants