[2.0] RFC: docusaurus swizzle --wrap/copy

[slorber]

🚀 Feature

We have a feature to wrap existing theme components:
https://docusaurus.io/docs/using-themes#wrapping-theme-components

import OriginalFooter from "@theme-original/Footer";
import React from "react";

export default function Footer(props) {
  return (
    <>
      <div>Before footer</div>
      <OriginalFooter {...props} />
    </>
  );
}

It's a very useful pattern to use to customize your Docusaurus site.
It allows you to enhance a Docusaurus theme without having to copy (and maintain) theme code.

Unfortunately, this feature is not very visible and we don't encourage its usage, as the code above is only found in our doc, and the swizzle CLI command does not produce such code.

Some use-cases:

• Add a custom comp before/after (above/under) any existing theme component (like add comments to blog posts)
• Slightly modify the layout/behavior of an existing theme component (like, add line numbers to code blocks)
• Add custom MDX components

---

I suggest to do some changes:

• Add a docusaurus swizzle --wrap option to produce such code. It wouldn't require the usage of --danger because it's way safer than copying.
• For copying, require explicit usage of --copy option, keep --danger for unsafe components and suggest to prefer usage of --wrap
• No option: suggest use either --wrap or --copy, hint to prefer --wrap

---

Other changes to consider in the long term:

• Rename docusaurus swizzle to something more intuitive like docusaurus theme?
• Create much smaller theme components, so that users naturally have more before/after wrapping possibilities to suit their use-cases.

[Josh-Cena]

Rename docusaurus swizzle to something more intuitive like docusaurus theme?

1. I like swizzle because it sounds funky :P
2. We probably want a verb here instead of a noun
3. Agree that swizzle is not intuitive enough

Create much smaller theme components, so that users naturally have more before/after wrapping possibilities to suit their use-cases.

Agreed. I also think a more hierarchical component structure is better than crowding src/theme with all kinds of components, since it makes navigation and understanding the components' usage easier.

No strong feelings about extending the CLI. I think we can make it fully interactive (just like docusaurus-init) so that running docusaurus swizzle will prompt you for wrap/copy, the theme, and the component, step by step. Right now if the theme name / component name isn't provided it throws an exit code: 1 in my face which I don't like. If user just wants to list the components/themes available but not to actually choose any of them, they can either provide an explicit --list argument or choose a quit option in the interactive console.

[slorber]

I created a swizzle meta issue here: #6114

We can probably keep this one open to have more focused discussions on this specific topic

[Josh-Cena]

From what I imagine, swizzle --wrap would just be trivially a string interpolation 🤷‍♂️

const content = `
import React from 'react';
import ${component} from '@theme-original/${component}';

export default function Wrapped${component.split('/').at(-1)}(props) {
  return (
    <>
      <${component.split('/').at(-1)} {...props} />
    </>
  );
}`;

fs.writeFileSync(`src/theme/${component}.js`, content); 

Anything beyond that?

[roydukkey]

@Josh-Cena This will have issues with components that export anything other than a default. For example, wrapping the Heading component requires:

import Heading, { MainHeading as OriginalMainHeading } from '@theme-original/Heading';
import React, { Fragment } from 'react';

export default Heading;

export const MainHeading: typeof OriginalMainHeading = ({ ...props }) => <Fragment>
	<p>My additionalness.</p>
	<OriginalMainHeading {...props} />
</Fragment>;

I think the user experience would be better if components that support wrapping only ever export a default. So the Heading component might be separated into two.

[Josh-Cena]

Ah... Yes, thanks for reminding! That would be a bit more troublesome... I think we would refactor that as well

[Josh-Cena]

@slorber What do you think about turning on 'import/no-named-export' inside src/theme, and enforce that every theme component only has one default export? This would make them work well with wrapping. However, we can't guarantee that third-party themes would do the same thing, so maybe we should just fail the wrap command if there are named exports (since the command would be ambiguous anyways)?

[slorber]

Did some tests and I believe it should technically be possible to support named exports with export * from "@theme-original/Heading"

Still, for our own codebase we only have very few named exports, I'll refactor that and apply the rule 'import/no-named-export' @Josh-Cena.

We can figure out later for third-party themes, there are probably not a lot of themes out there and it's not the most common customization pattern.

---

For some reasons I don't understand yet (maybe related to CJS/ESM modules config 🤷‍♂️ ), the following does not work while it should.

import React from 'react';

export function MainHeading() {
  return <div>test2</div>;
}

export default function HeadingWrapper() {
  return <div>test</div>;
}

Edit: oh found out: it's because the default export is not a component 😅

---

Another problem is that when using swizzle --eject we might eject to components importing theme-common:

import {useThemeConfig} from '@docusaurus/theme-common';

In such case the dependency must be explicitly added to the website.

Maybe we should automate this and add a console warning to explain why it was added?

---

Another issue is the TypeScript story

When using swizzle --wrap --typescript, it's not so easy to decide what exactly should be the output.

Note that the theme does not expose types for @theme-original imports.

And the export type Props convention that we use can't really be enforced everywhere (in particular third-party themes).

This output does not look so good to me, we should try to find something better?

import React, {ComponentProps} from 'react';

import Heading from '@theme-original/Heading';
import type HeadingComponentType from '@theme/Heading';

export default function HeadingWrapper(
  props: ComponentProps<typeof HeadingComponentType>,
) {
  return <Heading {...props} />;
}

In this specific case, it's not even good because the export is not a component but a component factory.

In such case it's quite impossible to write a wrapper of a higher-order function, it's quite specific to the context and there's no generic way to do so.

We can only wrap regular React components in a generic way, and we probably need a way to disable the --wrap feature for a few theme APIs IMHO.

---

Another similar case to think about: theme hooks.

Do we allow/disallow to wrap them?

Do we move all of them to @theme-common

What could a wrapper look like?

import useHideableNavbar from "@theme-original/hooks/useHideableNavbar"

export default 
  function useHideableNavbarWrapper(
    ...params: Parameters<typeof useHideableNavbar>
  ): ReturnType<typeof useHideableNavbar> {
  return useHideableNavbar(...params);
} 

That looks quite ugly TS code for the end-user

We can't even be sure that the hook returns a value

IMHO in general the hooks we have shouldn't be wrapped, but we might have cases later where this might become useful. I'm thinking of useAlgoliaContextualFacetFilters

We can probably disallow wrapping with the CLI (or fail with a nice error message?) and explain to the user to do the wrapping manually 🤷‍♂️

[Josh-Cena]

In such case the dependency must be explicitly added to the website.

Not always needed? Extraneous dependencies are bad but don't prevent sites from building unless you are using PnP. Would still be nice to fix it though

I would want to move all docs hooks, at least, to theme-common since we already have docUtils.

About TypeScript, I think we can just go with emitting no types for props? Or default to @theme/* since types are non-blocking and TS users usually know enough to fix the situation if needed?

[slorber]

Not always needed? Extraneous dependencies are bad but don't prevent sites from building unless you are using PnP. Would still be nice to fix it though

Agree, this is not a blocker as it can be fixed manually anyway, just a think to keep in mind.

I would want to move all docs hooks, at least, to theme-common since we already have docUtils.

Yes, that's my next plan to move much more things to theme-common, starting with those hooks. I'll start now but let me know if you want to split the work.

[Josh-Cena]

Great, you can go ahead 👍 I would be relaxing a little

[slorber]

About TypeScript, I think we can just go with emitting no types for props? Or default to @theme/* since types are non-blocking and TS users usually know enough to fix the situation if needed?

May not be a blocker to get started.

But this means that we enable implicit any on the default config?

I think it's better to be strict by default so that swizzle eject users can have type errors when we refactor a theme component's prop, without having to read the changelogs.

This can also prevent weird errors where we add a new prop, thinking it's harmless but the user is spreading props to an element that should not receive it.

Hopefully someday TS will allow a module to implement an interface:
microsoft/TypeScript#38511