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

Overridden Model methods have incorrect this type #9404

Closed
seanCodes opened this issue May 12, 2024 · 1 comment · Fixed by #9450
Closed

Overridden Model methods have incorrect this type #9404

seanCodes opened this issue May 12, 2024 · 1 comment · Fixed by #9450

Comments

@seanCodes
Copy link
Contributor

seanCodes commented May 12, 2024

Reproduction

Example: https://github.com/seanCodes/ember-data-bugs/blob/main/app/models/user.ts#L11
Repo: seanCodes/ember-data-bugs

Description

When overriding base Model methods (save, deleteRecord, serialize) in a custom model, methods/properties on the custom model can’t be accessed—according to TypeScript—when in reality (when the code is run) they can.

This seems to be caused by this in the overridden method being a generic ModelInstance rather than an actual instance of the custom model. From what I can tell there's no way to change this, since the overridden method needs the same signature as the base method:

export default class FooModel extends Model {
  [ResourceType] = 'foo' as const

  private myMethod() {
    // ...
  }

  override save = function <ModelInstance extends MinimalLegacyRecord>(
    this: ModelInstance,
    options?: Record<string, unknown> | undefined,
  ): Promise<ModelInstance> {
    if (this.currentState.isNew && this.currentState.isDeleted) {
      return Promise.resolve(this)
    }

    // ERROR: "Property 'myMethod' does not exist on type 'ModelInstance'.(2339)"
    this.myMethod()

    return this[RecordStore].saveRecord(this, options)
  }
}

Versions

Legend: production dependency, optional only, dev only

ember-data-bugs@0.0.0 …/ember-data-bugs

devDependencies:
ember-source 5.8.0
Legend: production dependency, optional only, dev only

ember-data-bugs@0.0.0 …/ember-data-bugs

devDependencies:
ember-cli 5.8.1
Legend: production dependency, optional only, dev only

ember-data-bugs@0.0.0 …/ember-data-bugs

devDependencies:
@ember-data/adapter 5.4.0-alpha.64
@ember-data/graph 5.4.0-alpha.64
@ember-data/json-api 5.4.0-alpha.64
@ember-data/legacy-compat 5.4.0-alpha.64
@ember-data/model 5.4.0-alpha.64
@ember-data/request 5.4.0-alpha.64
@ember-data/request-utils 5.4.0-alpha.64
@ember-data/serializer 5.4.0-alpha.64
@ember-data/store 5.4.0-alpha.64
@ember-data/tracking 5.4.0-alpha.64
ember-data 5.4.0-alpha.64
@runspired
Copy link
Contributor

save and deleteRecord really should never be clobbered like this and the type restraint is steering you in the right direction as using anything not specified by the MinimalLegacyRecord only ensures a lack of compatibility with the library overall.

serialize is a bit more of a hook and clobbering it is a bit safer, we could come up with a way to allow doing so but it would likely mean typing it as an any as nothing else would allow extension (for the same reason that User extends Model does not satisfy Model as you note in #9405)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

2 participants