NordSecurity / storyblok-rich-text-astro-renderer

Storyblok Rich Text Renderer for Astro
MIT License
24 stars 6 forks source link

Option to pass content as prop #129

Open atsuyaourt opened 8 months ago

atsuyaourt commented 8 months ago

It turns out that Astro has a built-in component that provides syntax highlighting for code blocks (<Code />). However, the string that needs to be converted should be passed as a prop, making it incompatible with your current implementation.

The following won't work:

---
import { Code } from "astro:components";
---
<RichTextRenderer
  content={text}
  schema={{
    nodes: {
      ...,
      code_block: ({ attrs }) => ({
        component: Code,
        props: {
          lang: attrs.class?.split("-")[1],
          theme: "solarized-dark",
        },
      }),
      ...
    },
    ...
  }}
  ...
/>

I've implemented a workaround by introducing a "metaprop" called contentPropName to specify the prop name to which the content will be passed. For instance, the following will render correctly because the component requires the content to be passed to a prop called code:

---
import { Code } from "astro:components";
---
<RichTextRenderer
  content={text}
  schema={{
    nodes: {
      ...,
      code_block: ({ attrs }) => ({
        component: Code,
        props: {
          lang: attrs.class?.split("-")[1],
          theme: "solarized-dark",
+         contentPropName: "code",
        },
      }),
      ...
    },
    ...
  }}
  ...
/>

I can create a pull request for these changes, or perhaps you have other suggestions?

edvinasjurele commented 7 months ago

@atsuyaourt Thank you for pointing this out.

I see the problem, currently RichTextRenderer.astro only allows content be passed as children <Component {...props}>{resolvedContent}</Component>;.

The solution provided is one of the ways, though I am not super sure if this is the most intuitive way. However, adding some JSDoc comments as a clear explanation should help the users out. We should not forget an example in demo app with this Astro native Code component.

Let me take a look into alternatives.

Edo-San commented 7 months ago

I stumbled upon the exact same issue! 😅

Some random thoughts

Using slots is the easiest way to make the implementation recursive. Aside from that, the "leaves" of the recursion tree might need to be rendered via props. If you render a node mapping its content to any Component props, I'd say that 99% of the time it's where recursion stops and you're rendering an atom component. I think that's what we're talking about, right now.

In order for this to be possible:

WDYT?

edvinasjurele commented 7 months ago

Hey guys, we surely need to solve this somehow 💪

I was thinking of either going the way the contentPropName way or alternatively just passing content things to resolverFn to give full flexibility. However, this "leaves" the recursion tree which may potentially impose an issue of not rendering stuff down the tree. We must think about this aspect, and provide examples in the demo app, but I am also pretty certain such cases might be about the handling of nodes at the very end of the tree.

Maybe

<RichTextRenderer
  text={{
    type: "doc";
    content: ...; // the passed content
  }}
  schema={{ ... }} // same schema
/>

There is also <Astro.self (ref) to call file itself when user has rich text configuration like in the demo https://github.com/atsuyaourt/storyblok-rich-text-astro-renderer/blob/main/demo/src/storyblok/RichText.astro (did not go into it that deep, so have to doublecheck, but from top of my head)

edvinasjurele commented 7 months ago

Hey @atsuyaourt and @Edo-San check the https://github.com/NordSecurity/storyblok-rich-text-astro-renderer/pull/168

Thanks @Edo-San a lot for the help and POC!

atsuyaourt commented 7 months ago

Thank you so much for these insights! I agree that exposing the whole Node object is a better approach to solving this issue. The PR looks good to me, and I can't wait to try it asap!

edvinasjurele commented 6 months ago

Getting back to this issue, it seems the node object expose that we recently implemented, does not entirely help in all scenarios due to when content is handled manually we are escaping the rendering cycle of RichTextRenderer and we must somehow handle that content ourselves:

heading: ({ attrs: { level }, content }) => ({
  component: Text,
  props: {
    variant: `h${level}`,
    text: content?.[0].text,  // here we cannot handle content array easily in some case, thus this works only for some minor or end-of-tree nodes which does not contain complicated nodes.
  },
}),

However in complicated nodes like bullet_list / ordered_list, where list_item can be anything, not only text but also other block nodes (ref). see types:

type ListItem = {
  type: "list_item";
  content?: Array<
    Paragraph | Blok | BulletList | OrderedList | HorizontalRule | Image | Emoji
  >;
};

type BulletList = {
  type: "bullet_list";
  content?: Array<ListItem>;
};

export type OrderedList = {
  type: "ordered_list";
  attrs: {
    order: number;
  };
  content?: Array<ListItem>;
};

This implies that @atsuyaourt proposed idea of having contentPropName field would make sense here, as it would not touch the content directly, but rather leave an indication how to handle content changing from slot strategy to prop.

We have to give this ticket a second go and maybe potentially add this handling possibility too 🤔 Re-opening ticket for now.

edvinasjurele commented 6 months ago

As a workaround for now, we do have a custom RichTextRenderer.astro in the repository with extending of:

if (type === 'bullet_list' || type === 'ordered_list') {
  return <Component {...props} items={resolvedContent} />;
}        

and then resolvers configuration is:

bullet_list: ({ type }) => {
    return {
      type,
      component: List,
      props: {
        styleType: 'disc',
      },
    };
  },
  ordered_list: ({ type }) => {
    return {
      type,
      component: List,
      props: {
        styleType: 'numbered',
      },
    };
  },
},