prismicio / prismic-helpers

Set of helpers to manage Prismic data
https://prismic.io/docs/technical-reference/prismicio-helpers
Apache License 2.0
15 stars 9 forks source link

v2.3.1 broke asHTML serializer api (bug or feature 🤷) #64

Closed alejo4373 closed 1 year ago

alejo4373 commented 1 year ago

Versions

Steps to reproduce

  1. Open the following codesandbox link https://codesandbox.io/s/prismicio-helpers-html-serializer-bug-i0eqrs?file=/src/App.tsx
  2. In the codesandbox UI to change project dependencies, switch prismicio/helpers between versions 2.3.1 and 2.3.0
  3. Observe how the outputted HTML is different between v2.3.1 and v2.3.0 by looking at the rendered preview on the right

Reproduction video

https://user-images.githubusercontent.com/18352555/232843813-0eae40dd-ecf5-4fb5-880d-72e6e7349de2.mp4

Additional Notes

I needed to wrap text coming from Prismic with my own text styling and I didn't want to have nested p, h1 or other tags nested of the same kind (because it's invalid html). For example, I didn't want to have:

<p class="my-paragraph"> <!-- p with my styles -->
  <p>hello world <a href="https://example.com">example</a></p> <!-- p serialized from primisc -->
</p>

I needed to provide my own serializer to asHTML to get rid of the outermost tag coming from prismic to avoid this issue and end up with something like this

<p class="my-paragraph"> <!-- p with my styles -->
  hello world <a href="https://example.com">example</a><!-- content of the p coming from primisc -->
</p>
When things broke

In 2.3.0 the custom serializer content argument had the HTML string of the element currently being processed In 2.3.1 the custom serializer content argument had the text content of the element currently being processed and no longer the HTML string

I fixed my case by having the serializer return null in all cases except when it had reached the top or outermost tag as suggested here in the docs

  const htmlWithCustomSerializer =
    asHTML(richTextFieldLink, null, (type, _, content, children) => {
      // Stop once parent element is reached to avoid nested h1, p tags, etc.
      // Given our StyledText would provides the outermost tag
      if (type === richTextFieldLink?.[0]?.type) return children;
      else return null;
    }) || "";

The questions

github-actions[bot] commented 1 year ago

This issue has been labeled as a bug since it was created using the 🚨 Bug Report Template.

Hi there, thank you so much for the report!

Following our Maintenance Process, we will review your bug report and get back to you next Wednesday. To ensure a smooth review of your issue and avoid unnecessary delays, please make sure your issue includes the following:

If you have identified the cause of the bug described in your report and know how to fix it, you're more than welcome to open a pull request addressing it. Check out our quick start guide for a simple contribution process.

If you think your issue is a question (not a bug) and would like quicker support, please close this issue and forward it to an appropriate section on our community forum: https://community.prismic.io

- The Prismic Open-Source Team

lihbr commented 1 year ago

Hey there, thank you so much for contributing and submitting a nice reproduction. I'm sorry we failed to get back to you in a timely manner but I'm glad you had a workaround from the get go.

tl;dr;

Details

What's happening?

asHTML() and other serialization methods (components, hooks, etc.), serialize Prismic rich text fields by relying on an internal default serializer with sensible defaults (https://github.com/prismicio/prismic-client/blob/3bc5b95a9d4bd69e70068a209e1e9e4b04b87c69/src/helpers/asHTML.ts#L119-L171). Basically, it's just saying, in the case of an HTML serialization, that a paragraph should wrap its children with a <p> tag, and so on.

However, we often need something different than these defaults, maybe we're using TailwindCSS and want to apply extra classes, or maybe we don't want to wrap children of specific element types like in your case. In such scenarios, we can provide an extra "custom serializer" to the asHTML() method or its derivatives. This "custom serializer" can either:

  1. Serialize differently some elements
  2. Serialize differently all elements

It's quite rare to fall in the case of n°2, that's why a "custom serializer" can fall back to the internal default serializer by returning a falsy value, null as a convention.

@prismicio/richtext@2.0.1 and earlier had an issue not filling the content property received by the "custom serializer" as expected. This meant that by returning content when this version was installed as a dependency of @prismicio/helpers, it was effectively returning undefined, telling asHTML() to fallback to its internal default serializer, serializing links and other elements as expected.

When this was fixed in @prismicio/richtext@2.1.0 (https://github.com/prismicio/prismic-richtext/pull/55), content was now filled as expected, and the "custom serializer" stopped returning a falsy value, meaning no fallback to the internal default serializer were happening anymore.

Why @prismicio/helpers@2.3.0 and prior?

I'm not completely nominal on that, but I'm gonna assume CodeSandbox has some caching mechanism on dependencies/lock files.

When using @prismicio/helpers@2.3.0 and earlier, it was using an older version of @prismicio/richtext not including this fix: https://github.com/prismicio/prismic-richtext/pull/55 (supposedly 2.0.1). This could also happen locally depending on the project's lock file.

You can see this fork with an explicit @prismicio/richtext installed reproducing the same behavior despite relying on @prismicio/helpers@2.3.0: https://codesandbox.io/s/prismicio-helpers-html-serializer-bug-forked-yrtwtc?file=/src/App.tsx:1019-1026

How to do it?

The recommended way to achieve what you wanted was to return null as you figured out.

asHTML(field, null, (type, node, content, children) => {
    if (type === "paragraph") return children;

    return null;
});

I'd recommend in such scenarios relying on a map serializer instead of a function serializer. This is the equivalent of the above and has simpler types:

asHTML(field, null, {
    paragraph: ({ children }) => children;
});

@prismicio/client v7

Just a friendly notification that we recently released @prismicio/client v7 that unifies our core libraries (@prismicio/types and @prismicio/helpers were merged inside @prismicio/client so you just need to install one package moving forward)

Read the announcement and learn more about it: https://prismic.io/blog/client-v7-new-unified-core-library


Hope this answers your issue, let us know if anything!