[Bug]: In addOptions. the type of this.parent is wrong.

[mgreystone]

Affected Packages

core

Version(s)

2.9.1

Bug Description

When extending a node & implementing addOptions, the type of this.parent is incorrect.

For a Node<T>, this.parent is typed as () => T, denoting that it is safe to call directly like this.parent(). However, this.parent might be undefined, as is documented, so we must call this.parent?.().

Because of the inaccurate type, it is easy to forget to add the ?. optional chaining. Furthermore, linting rules such as no-unnecessary-condition can automatically strip the ?. out, which can cause type errors be thrown at runtime.

import { Heading, type Level } from '@tiptap/extension-heading'

const MyHeading = Heading.extend({
  addOptions() {
    const levels: Level[] = [1,2,3]
    return {
      ...this.parent(), // this throws even though typescript says it is safe
      levels,
    }
  }
})

Browser Used

Chrome

Code Example URL

No response

Expected Behavior

In the addOptions method, this.parent should have a type of (() => T) | undefined to match the implementation & documentation.

Additional Context (Optional)

No response

Dependency Updates

• Yes, I've updated all my dependencies.

[nperez0111]

Yep, you are right here. Pushing a patch for it

[nperez0111]

Resolved with 2.10.0

[nperez0111]

Reopening because I'm going to revert this change. It is more trouble than it is worth. If someone can find a way to update the typings in a way that don't break people. Feel free to contribute

[Mathias-S]

Some context: The attempts to resolve this issue caused the following new type errors in code that was previously working:

import Link from "@tiptap/extension-link";

const MyLink = Link.extend({
  addOptions() {
//^^^^^^^^^^ Type error
    return {
      ...this.parent?.(),
    };
  },
});

See #5852 for the issue about the error above.

import Paragraph from "@tiptap/extension-paragraph";
import { mergeAttributes } from "@tiptap/react";

const MyStrangeParagraph = Paragraph.extend({
  renderHTML({ HTMLAttributes }) {
    return [
      `h1`,
      mergeAttributes(this.options.HTMLAttributes, HTMLAttributes),
      //              ^^^^^^^^^^^^^^^^^^^^^^^^^^^ 'undefined' is not assignable to 'Record<string, any>'
      0,
    ];
  },
});

See #5859 for more details about the error above.

So any attempts to fix this issue should also ensure that the aforementioned cases won't have any type errors.

[mgreystone]

That's a bummer, but i understand that it would require a breaking change to the types. Do we think that the change would be acceptable in version 3? We have to remember to add // eslint-disable-next-line no-unnecessary-condition before all of our uses of this.parent?.() or else our eslint config auto-strips out all of the nullish chaining, which more often than not causes runtime errors.

[nperez0111]

I would be willing to get it in version 2 if there were an easy way to handle it.

I tired doing this for like an hour and basically what I had come up with was that the types of the first .create needs to be different than a .extend. This isn't the easiest thing to support especially since it is already generic over two parameters so it gets confusing very fast.