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

[Suggestion utility] Outdated items promise resolutions should be ignored #2544

Closed
1 of 2 tasks
Maximaximum opened this issue Feb 17, 2022 · 3 comments
Closed
1 of 2 tasks
Assignees
Labels
Type: Bug The issue or pullrequest is related to a bug

Comments

@Maximaximum
Copy link

What’s the bug you are facing?

The problem with the current Suggestion utility is that it does not seem to properly handle renderer.items(props) return values when they are promises.

Imagine a situation when the user puts the cursor into the tiptap editor, and then subsequently types @, then J, o, h (so that eventually the content in the editor becomes @Joh. So, the renderer.items function gets called 3 times, with these props.query values: J, Jo, Joh. Let's assume each of the calls return a Promise.

Now, imagine a race condition so that these 3 Promises are resolved in an order that is different from the original renderer.items calls order. Ie, they are resolved in the order: Jo, then Joh and eventually J.

Now, the problem is that in such case the renderer.onUpdate() function will be called in the Jo, Joh, J order (according to the order of promise resolutions). Which effectively means that eventually the suggestion popup will render the J query result, not the Joh query result, which is not the expected behavior.

This is the diagram of the actual behavior:

user input          =  J----------------Jo---------------Joh---------------------------------------
promise resolutions =  ------------------------20items(Jo)----------19items(Joh)---------80items(J)
rendered output     =  ------------------------20items(Jo)----------19items(Joh)---------80items(J)

Instead, the expected behavior would be:

user input          =  J----------------Jo---------------Joh---------------------------------------
promise resolutions =  ------------------------20items(Jo)----------19items(Joh)-------------------
rendered output     =  ------------------------20items(Jo)----------19items(Joh)-------------------

So, if a promise is resolved after any of its subsequent promises have been resolved, its value should be ignored (renderer.onUpdate should not be called).

Or, another way to say it: whenever a promise is resolved, the lib should stop listening to any of the previous unresolved promises.

One more note: it might seem that these race conditions should be handled by the code that uses the Suggestion util, but IMHO this is a behavior that everyone would expect from using the Suggestion util in any scenario. Unless I'm missing something.

How can we reproduce the bug on our side?

Just open the codesandbox provided

Can you provide a CodeSandbox?

https://codesandbox.io/s/tiptap-mention-promise-bug-reproduction-6yvvu1?file=/README.md

What did you expect to happen?

The expected behavior would be:

user input          =  J----------------Jo---------------Joh---------------------------------------
promise resolutions =  ------------------------20items(Jo)----------19items(Joh)-------------------
rendered output     =  ------------------------20items(Jo)----------19items(Joh)-------------------

So, if a promise is resolved after any of its subsequent promises have been resolved, its value should be ignored (renderer.onUpdate should not be called).

Or, another way to say it: whenever a promise is resolved, the lib should stop listening to any of the previous unresolved promises.

Anything to add? (optional)

No response

Did you update your dependencies?

  • Yes, I’ve updated my dependencies to use the latest version of all packages.

Are you sponsoring us?

  • Yes, I’m a sponsor. 💖
@rfgamaral
Copy link
Contributor

@Maximaximum Wondering if you could follow this pattern (or similar) to work around this issue? Ideally this would be fixed in the Suggestion utility, but until then, maybe that pattern will help you?

@Maximaximum
Copy link
Author

@floriankrueger I’ve created this ticket specifically for fixing the issue in the Suggestion utility, because otherwise the utility seems to be malfunctioning.

Handling the issue outside of the utility might work as a temporary solution, but the issue itself should be fixed in the utility IMHO.

@philippkuehn philippkuehn self-assigned this Apr 6, 2022
@stale
Copy link

stale bot commented Jul 6, 2022

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Bug The issue or pullrequest is related to a bug
Projects
None yet
Development

No branches or pull requests

3 participants