fix(suggestion): 🐛 make clientrect prop optional

[bdbch]

This commit makes the clientRect prop optional - this means that this value can be null. This allows developers using the suggestion extension to know that they have to implement a check for the clientRect before using it.

I think this is the correct way to handle this issue. Instead of building around it, we let the developer know that a clientRect could be potentially not defined, specially when the rendering is to slow and the DOM element for example does not exist.

See #2795 for more information.

[netlify]

✅ Deploy Preview for tiptap-embed ready!

Name	Link
🔨 Latest commit	793b1c9
🔍 Latest deploy log	https://app.netlify.com/sites/tiptap-embed/deploys/6287e86651760300080fd460
😎 Deploy Preview	https://deploy-preview-2813--tiptap-embed.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[dilizarov]

While this is nice, I don't think this actually works.

Down below, you see:

getReferenceClientRect: props.clientRect

When you look at Tippy's typings, it goes something like this:

export interface GetReferenceClientRect {
  (): ClientRect | DOMRect;
  contextElement?: Element;
}

...
export interface Props extends LifecycleHooks, RenderProps {
  getReferenceClientRect: null | GetReferenceClientRect;
  ...
}

Tippy could handle if props.clientRect is null, but in practice I've seen it be the function. Tippy expects () => ClientRect | DOMRect whereas we've got () => ClientRect | DOMRect | null.

It is kinda problematic.

On the one hand, you could run the function yourself, see if it is null and then pass it in if it is not, but that doesn't really work because it could be null by the time Tippy runs it.

[bdbch]

I see. Tricky issue since I wasn't able to reproduce it even by blowing up my machine with 10k console.logs per editor state change. 🤡

I'll think about this a bit and search for a better and cleaner solution.

The worst solution but also the worst case could be that we pass out a fake clientRect object but that feels pretty messy.

[dilizarov]

Perhaps integrating with Tippy is the tough nugget here. While it is a 3rd-party I think it is important because all examples leverage it and so naturally TipTap users will gravitate towards it.

I need to see if there is a way for us to pass a clientRect to Tippy without having them handle the function calling, etc.

[dilizarov]

@bdbch check out this:

tiptap/packages/suggestion/src/suggestion.ts

Lines 254 to 268 in 13a0342

	decorations(state) {
	const { active, range, decorationId } = this.getState(state)
	if (!active) {
	return null
	}
	return DecorationSet.create(state.doc, [
	Decoration.inline(range.from, range.to, {
	nodeName: decorationTag,
	class: decorationClass,
	'data-decoration-id': decorationId,
	}),
	])
	},

So, basically this function seems to be responsible for whether the span decorator is there or not...

Perhaps there is a solution where you're basically storing data on the suggestion that tells you if you're rendering any decoration.

Something like:

  if (!active) {
    this.decoratorExists = false;
    return null;
  }

  this.decoratorExists = true;

Then, this is information that can be used to aid in the clientRect information.

Just thinking out loud.

Basically, the problem is when we query the document, we're expecting the decorator there, but due to possible race conditions I've pointed out, it doesn't have to be there.

While querying the document is useful for DOMRect calculations, perhaps before querying we could have a flag stored that tells us if there is a decorator or not and that is probably immune to race conditions and we can build a solution around that.