Proposal to change type declarations for bound methods

Problem

In ProseMirror, some methods like view.dispatch and schema.nodeFromJSON are bound to their class instances, allowing them to be passed around easily.

let dispatch = view.dispatch;
dispatch(view.state.tr); // Works fine

However, most methods in ProseMirror (and JavaScript generally) aren’t bound. Passing these unbound methods around can lead to runtime errors.

const destroy = view.destroy;
destroy(); // Uncaught TypeError: Cannot read properties of undefined (reading 'docView')

The typescript-eslint package offers a rule called unbound-method to catch these mistakes. It’s very helpful.

const destroy = view.destroy;
//              ~~~~~~~~~~~~
//              ESLint error:
//              Avoid referencing unbound methods which may
//              cause unintentional scoping of `this`.
//              If your function does not access `this`, you
//              can annotate it with `this: void`, or consider
//              using an arrow function instead.
//              https://typescript-eslint.io/rules/unbound-method

Sadly it can’t recognize when methods are already bound like const dispatch = view.dispatch. There are three workarounds without changing prosemirror-view code:

  1. const dispatch = view.dispatch.bind(view)
  2. const dispatch = (tr: Transaction) => view.dispatch(tr)
  3. /* eslint-disable-next-line @typescript-eslint/unbound-method */

The first two add unnecessary runtime overhead, and the third is tedious to write.

Solution

To fix this ESLint error, I suggest changing the TypeScript type declaration of view.dispatch to add this: void to the function:

diff --git a/src/index.ts b/src/index.ts
index 3cc47f4..6361e18 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -71,7 +71,7 @@ export class EditorView {
     this.directPlugins = props.plugins || []
     this.directPlugins.forEach(checkStateComponent)

-    this.dispatch = this.dispatch.bind(this)
+    this.dispatch = this.dispatch.bind(this as unknown as void)

     this.dom = (place && (place as {mount: HTMLElement}).mount) || document.createElement("div")
     if (place) {
@@ -490,10 +490,10 @@ export class EditorView {
   /// [`updateState`](#view.EditorView.updateState) with the result.
   /// This method is bound to the view instance, so that it can be
   /// easily passed around.
-  dispatch(tr: Transaction) {
-    let dispatchTransaction = this._props.dispatchTransaction
+  dispatch(this: void, tr: Transaction) {
+    let dispatchTransaction = (this as unknown as EditorView)._props.dispatchTransaction
     if (dispatchTransaction) dispatchTransaction.call(this, tr)
-    else this.updateState(this.state.apply(tr))
+    else (this as unknown as EditorView).updateState((this as unknown as EditorView).state.apply(tr))
   }

This change only affects the TypeScript type declaration, not the JavaScript output (dist/index.js and dist/index.cjs), making it a safe update. With this change, typescript-eslint will recognize that view.dispatch is already bound and won’t flag const dispatch = view.dispatch.

Alternative Solution 1

For slightly better readability with minimal runtime impact, we could use a temporary variable let self = this:

diff --git a/src/index.ts b/src/index.ts
index 3cc47f4..26d9b67 100644
--- a/src/index.ts
+++ b/src/index.ts
@@ -71,7 +71,7 @@ export class EditorView {
     this.directPlugins = props.plugins || []
     this.directPlugins.forEach(checkStateComponent)

-    this.dispatch = this.dispatch.bind(this)
+    this.dispatch = this.dispatch.bind(this as unknown as void)

     this.dom = (place && (place as {mount: HTMLElement}).mount) || document.createElement("div")
     if (place) {
@@ -490,10 +490,11 @@ export class EditorView {
   /// [`updateState`](#view.EditorView.updateState) with the result.
   /// This method is bound to the view instance, so that it can be
   /// easily passed around.
-  dispatch(tr: Transaction) {
-    let dispatchTransaction = this._props.dispatchTransaction
-    if (dispatchTransaction) dispatchTransaction.call(this, tr)
-    else this.updateState(this.state.apply(tr))
+  dispatch(this: void, tr: Transaction) {
+    let self = this as unknown as EditorView
+    let dispatchTransaction = self._props.dispatchTransaction
+    if (dispatchTransaction) dispatchTransaction.call(self, tr)
+    else self.updateState(self.state.apply(tr))
   }

Alternative Solution 2

A more elegant TypeScript approach would use an arrow function:

class EditorView {
  dispatch = (tr: Transaction) => {
    // ...
    // `this` works fine here
  }
}

This eliminates the need for .bind(this) and avoids the awkward this as unknown as EditorView type casting. However, it would change the JavaScript output and break backwards compatibility for code that extends EditorView and overrides the dispatch method.

A detailed example to explain the compatibility issue
// Current implementation
class EditorView1 {
  constructor() {
    this.dispatch = this.dispatch.bind(this)
  }

  dispatch() {
    console.log('EditorView1.dispatch', this)
  }
}

class SubEditorView1 extends EditorView1 {
  constructor() {
    super()
  }

  dispatch() {
    console.log('SubEditorView1.dispatch start')
    super.dispatch()
    console.log('SubEditorView1.dispatch end')
  }
}

// Alternative implementation by using arrow function
class EditorView2 {
  constructor() {
  }

  dispatch = () => {
    console.log('EditorView2.dispatch', this)
  }
}

class SubEditorView2 extends EditorView2 {
  dispatch() {
    console.log('SubEditorView2.dispatch start') // This line is not called
    super.dispatch()
    console.log('SubEditorView2.dispatch end') // This line is not called
  }

  constructor() {
    super()
  }
}

const subEditorView1 = new SubEditorView1()
subEditorView1.dispatch()
// Print:
// SubEditorView1.dispatch start
// EditorView1.dispatch SubEditorView1 { dispatch: [Function: bound dispatch] }
// SubEditorView1.dispatch end

const subEditorView2 = new SubEditorView2()
subEditorView2.dispatch()
// Print:
// EditorView2.dispatch SubEditorView2 { dispatch: [Function: dispatch] }

I’d be happy to submit a PR if you think this would be a good idea.

Does that last approach still help silence the linter rule? Maybe because the rule checks whether the function is defined as a method or a property? It seems less ugly than the others, in any case.

Yes, the last one (arrow function) does silence the linter rule.

It’s even prettier than the current code IMHO since you don’t need to write this.dispatch.bind(this) in the constructor.

Does this kludge, which declares dispatch as a property but secretly defines it as a method (in case some Smalltalk freak actually went and overrode it), look okay to you?

Oh that’s perfect! I can confirm it works well with the typescript-eslint rule.