web-infra-dev / rspress

🦀💨 A fast Rspack-based static site generator.
https://rspress.dev
MIT License
1.46k stars 137 forks source link

[Feature]: doc layout components should merge className #1612

Open ap0nia opened 5 days ago

ap0nia commented 5 days ago

What problem does this feature solve?

Currently, all of the internal components used by the DocLayout will completely override the className property. For example, ul will always have "list-disc pl-5 my-4 leading-7". I think it should respect any className it gets from props, and attempt to merge them.

The motivation for this is that I wrote a custom rehype plugin that adds custom classes to MDX elements, but these are stripped away by the components.

What does the proposed API look like?

  1. The simplest solution would be to concatenate the props.
export const Ul = (props: ComponentProps<'ul'>) => {
  return <ul {...props} className={`list-disc pl-5 my-4 leading-7 ${props.className}`} />;
};

However, this may add an extra space if there were no classes provided.

  1. A nice solution would be to use the common cn utility for merging TailwindCSS classes.
import { clsx, type ClassValue } from "clsx"
import { twMerge } from "tailwind-merge"

export function cn(...inputs: ClassValue[]) {
  return twMerge(clsx(inputs))
}

export const Ul = (props: ComponentProps<'ul'>) => {
  return <ul {...props} className={cn("list-disc pl-5 my-4 leading-7", props.className)} />;
};

This would require additional packages to be installed, but they are small and might be useful in other places.

  1. Finally, it may also be feasible to implement an internal implementation of cn to prevent additional dependencies.
export function cn(...inputs: unknown[]) {
  return inputs.filter(Boolean).join(' ')
}

export const Ul = (props: ComponentProps<'ul'>) => {
  return <ul {...props} className={cn("list-disc pl-5 my-4 leading-7", props.className)} />;
};

If any of these solutions seem acceptable, I can open a corresponding PR.

Timeless0911 commented 5 days ago

The built-in TailwindCSS has caused some issues like style pollution since there are no prefix in built-in tailwind css classnames, we may change to use css modules or adding prefix for built-in tailwind classnames which has not been decided yet.

Therefore, I think option 1 or 3 is better, and below shows we mix the use of tailwindcss and css modules 😂

export const Blockquote = (props: ComponentProps<'blockquote'>) => {
  return (
    <blockquote
      {...props}
      className={`border-l-2 border-solid border-divider pl-4 my-6 transition-colors duration-500 ${styles.blockquote}`}
    />
  );
};

You can implement a function that judge if there were no classes provided. If provided, do concat. If not, return do nothing.