Skip to content

Change WidgetType.updateDOM from type to this#81

Merged
marijnh merged 1 commit intocodemirror:mainfrom
not-my-profile:from-this
Mar 12, 2026
Merged

Change WidgetType.updateDOM from type to this#81
marijnh merged 1 commit intocodemirror:mainfrom
not-my-profile:from-this

Conversation

@not-my-profile
Copy link
Copy Markdown
Contributor

With this TypeScript understands that it's the same type as the subclass.

With `this` TypeScript understands that it's the same type as the subclass.
@marijnh
Copy link
Copy Markdown
Member

marijnh commented Mar 11, 2026

The this type means more than just "of the same class as this" in TypeScript, unfortunately. It asserts that the value is actually equal to this, and will cause weird type narrowing to happen. So unfortunately, though I agree it would be better to have a more precise type for this parameter, this isn't appropriate here.

@not-my-profile
Copy link
Copy Markdown
Contributor Author

I'm not sure that's the case? Do you have an example for the "weird type narrowing" because I can't manage to reproduce it:

class Foo {
  prop: string = "";

  test(other: this) {
    if (this.prop == 'certain') {
      this.prop; // narrowed to 'certain'
      other.prop; // still string
    }
  }
}

TS Playground

@marijnh
Copy link
Copy Markdown
Member

marijnh commented Mar 12, 2026

It seems you are right. I distinctly remember giving up on this in the past because it misbehaved, but I cannot reproduce the problem in this case. The docs claim that it just dynamically takes the type of the current class, which seems safe. Let's try this then.

@marijnh marijnh merged commit fd252fa into codemirror:main Mar 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants