storyblok / richtext

A custom javascript resolver for the Storyblok Richtext field.
MIT License
9 stars 3 forks source link

[Feature Request] Pass default resolver as parameter #104

Open apfelbox opened 1 month ago

apfelbox commented 1 month ago

When customizing the resolver of an element, it would be great to get the default resolver passed in. Because some time you may only want to add some attributes, but keep the remaining logic intact. To reuse the example from your announcement post:

const html = richTextResolver({
  resolvers: {
    [MarkTypes.LINK]: (node, defaultResolver) => {
      node.attrs.class = "text-blue-500 hover:text-blue-700 underline transition-colors duration-200 ease-in-out";
      return defaultResolver(node);
    },
  },
}).render(doc);

To be fair, with the current structure that is kind of inconvenient, as the attrs and the node are combined, but it is still possible.

This would be way more readable if it looked like this:

const html = richTextResolver({
  resolvers: {
    [MarkTypes.LINK]: (tag, attrs, children) => {
      return defaultResolver(
        tag,
        {...attrs, class: "text-blue-500 hover:text-blue-700 underline transition-colors duration-200 ease-in-out"},
        children,
      );
    },
  },
}).render(doc);

But maybe that is out of scope.


Expected Behavior

It would be great to have this feature.

Current Behavior

Not implemented yet

Steps to Reproduce

n/a

markus-gx commented 1 month ago

I guess I just add a comment here, since I have a nearly identical Feature request.

I have a scenario where I need to shift the heading levels (h1, h2, etc.) by n. Essentially, if n = 2, an h1 becomes an h3. I didn’t find a solution within the current implementation. My initial idea was to use the resolver option with [BlockType.HEADING] (node) => {}. This would work, but I’d need to write my own headingResolver.

Since a developer might not think of all possible cases like the authors/contributors with resolving headings, I ended up copying the defaultRenderFn and headingResolver from the richtext class to modify the function with the behavior I needed.

My idea for the PR would be to export the standard resolvers with an additional options parameter to override rules. This could result in something like:

const {render, headingResolver} = richTextResolver({
  resolvers: {
    [BlockTypes.HEADING]: (node) => headingResolver(node, {level: node.level + 2})
  }
})
alvarosabu commented 1 month ago

Hi @apfelbox @markus-gx thanks for the detailed feedback.

I think we can improve the DX further with some utilities like you mentioned, let's discuss which approach would be better with some context:

On the current version 2, none of the resolvers are exposed. All of the resolvers resolve internally to this function renderFn which by default is this for vanilla https://github.com/storyblok/richtext/blob/f14a87ddd62298f2b799d368d26390c00c493386/src/richtext.ts#L15-L23

This function can be override by framework specific render functions.

// Vue
const options: StoryblokRichTextOptions<VNode> = {
  renderFn: h,
};

So if I understand the DX improvement would be to have an exposed wrapper for the renderFn so the user doesn't need to return the template string in the case of vanilla javascript, nor the h or React.createElementright?

apfelbox commented 2 weeks ago

Hi @alvarosabu,

yes, that would work. For my use case, I mainly want to set some attributes and just do the default rendering. So any way (exposing the main renderer / passing the renderer as argument / etc...) would help here.

One thing to keep in mind however is that it would be great, when I overwrite the default renderFn, that this would just work with any other resolvers without change:

const options: StoryblokRichTextOptions<VNode> = {
    renderFn: myRenderFn,
    resolvers: {
        [BlockTypes.HEADING]: (node) => headingResolver(node, {level: node.level + 2})
                                     // ^^^^^^^^^^^^^^^ as changed the renderFn above, 
                                     // I now need to change all my resolvers to use the new default resolver
    },
};

That's why initially I suggested that you might want to pass the default resolver, so that this case automatically works.