payload icon indicating copy to clipboard operation
payload copied to clipboard

Lexical HTML link converter leads to an endless loop if pages link each other.

Open yobottehg opened this issue 1 year ago • 3 comments

Link to reproduction

Reproduction

Payload Version

Beta 78

Node Version

20 LTS

Next.js Version

15 - 104

Describe the Bug

Given the following:

  • A collection uses a lexical field (directly or through blocks makes no difference)
  • There is an HTML converter for that collection
  • There is a custom HTML converter for the link node
  • The link node HTML converter uses req.payload.findByID to get some attributes of the entry for generating the url
  • 2 entries of that collection exists which link each other
  • Upon save of the link in the second entry the whole node process will go in an endless loop and the complete CMS is unusable.

Additionally, after the docker container has been restarted and the CMS works again that a simple click on the collection is enough to get the whole CMS again to be unusable.

So there are 2 things here:

  1. We need to prevent endless loops for the HTML Link node converter. Perhaps it would be the best if the existing Link feature would expose a function called generate_url() which defaults to return the ID but can be overwritten with a function where we could safely generate an url based on the properties of that entry without using req.payload.findByID.
  2. HTML converter for the entries should IMHO not be called when only the collection overview page is visited and the contents of that field are not displayed inside the list.

Additional information can be found in this discord discussion: https://discord.com/channels/967097582721572934/1274009889106038846

Why is it necessary to use something like that in the link converter?

Here our use case:

  ...
  const payloadArgs = {
      depth: 0,
      draft: false,
      req: req,
      showHiddenFields: false,
      id: doc?.value as string,
    }

  const result = await req.payload.findByID({
          ...payloadArgs,
          collection: 'pages',
        })
        const locale = req.locale ?? 'de'
        href = result.paths?.[locale] ?? ''
  ...

We use a virtual / computed field called paths which holds the locale specific paths which are generated via the parent relationship of the nested docs plugin and we need the locale of the request to add the correct lang code.

Perhaps this is related to https://github.com/payloadcms/payload/issues/7722

Adapters and Plugins

db-postgres lexical

yobottehg avatar Aug 18 '24 11:08 yobottehg

Following this issue since I'm also having this problem when two entries in the same collection are linked with each other

tiago-meireles avatar Aug 19 '24 13:08 tiago-meireles

Having the same issue.

jancbeck avatar Aug 19 '24 13:08 jancbeck

Added a reproduction with steps to reproduce here:

https://github.com/yobottehg/payload/blob/beta-repro-7743/test/_community/config.ts

Additional details found while reproduction:

  • Does not need 2 pages linking each other. It is enough if one page is linking itself.
  • Setting maxDepth: 0 on the LinkFeature does not make any difference.

yobottehg avatar Aug 21 '24 09:08 yobottehg

Fixed by https://github.com/payloadcms/payload/pull/7890

Now, for custom converters, you will have to pass through depth and currentDepth (those are now parts of the html converter args) in order to avoid infinite recursion.

Instead of the local API, I recommend using the populate function now exported from richtext-lexical. Example: https://github.com/payloadcms/payload/blob/beta/packages/richtext-lexical/src/features/upload/server/feature.server.ts#L118

This handles checking and incrementing depth for you, and is more efficient

AlessioGr avatar Aug 27 '24 19:08 AlessioGr

This issue has been automatically locked. Please open a new issue if this issue persists with any additional detail.

github-actions[bot] avatar Sep 06 '24 20:09 github-actions[bot]