refactor: move blog theme component type defs to theme-classic

[Josh-Cena]

Motivation

It shouldn't be set up that way because these components are actually exposed by theme-classic and we can't ensure that other themes implement the same components. Also related to #6157 because PnP can't find content-blog types.

Have you read the Contributing Guidelines on pull requests?

Yes

[netlify]

✔️ [V2]

🔨 Explore the source changes: c4b02ad

🔍 Inspect the deploy log: https://app.netlify.com/sites/docusaurus-2/deploys/61c7e5d884a53900078c9453

😎 Browse the preview: https://deploy-preview-6175--docusaurus-2.netlify.app

[github-actions]

⚡️ Lighthouse report for the changes in this PR:

Category	Score
🟠 Performance	67
🟢 Accessibility	98
🟢 Best practices	100
🟢 SEO	100
🟢 PWA	95

Lighthouse ran on https://deploy-preview-6175--docusaurus-2.netlify.app/

[github-actions]

Size Change: 0 B

Total Size: 662 kB

ℹ️ View Unchanged

Filename	Size
website/.docusaurus/globalData.json	40.1 kB
website/build/assets/css/styles.********.css	102 kB
website/build/assets/js/main.********.js	490 kB
website/build/index.html	29.6 kB

_{compressed-size-action}

[slorber]

It shouldn't be set up that way because these components are actually exposed by theme-classic and we can't ensure that other themes implement the same components.

A theme that creates a page with a component "@theme/XYZ" is well-poised to know which props this component receives, as it is the content plugin that defines those props. Do we really want to do this separation, and let the theme define props it didn't create? I don't know 🤷‍♂️

---

Our TS setup is messy, it's hard to see all the potential issues we have. Maybe we should create a separate issue to discuss current problems and find a methodology to know exactly where a type should live? Currently there doesn't seem to be a strong consensus 😅

[Josh-Cena]

A theme that creates a page with a component "@theme/XYZ" is well-poised to know which props this component receives, as it is the content plugin that defines those props. Do we really want to do this separation, and let the theme define props it didn't create? I don't know 🤷‍♂️

No; the blog plugin doesn't make the components or create the pages. It merely passes the props down to the client. The @theme/* aliases are for the components, not for the props. You can see the plugin's data types as a rest API shape. It doesn't imply anything about the React implementation. All the metadata types are preserved in the plugin type defs anyways.

What are moved are the theme component aliases. Imagine this: @theme/BlogSidebar is a theme-classic component. There's no guarantee that a custom theme would implement this, or that the theme wouldn't implement even more granular components. The plugin doesn't demand any theme component to exist for it to function. Therefore, we should move the provider of these types to theme-classic, because if we implement theme-tailwind with another set of components, we don't want the default type defs from content-blog to get in the way.

[slorber]

No; the blog plugin doesn't make the components or create the pages.

It doesn't imply anything about the React implementation.

The plugin doesn't demand any theme component to exist for it to function.

Somehow it does

It it calls addRoute({component: "@theme/XYZ",props: {age: 42}) then shouldn't it keep the responsibility to declare the theme typings?

The theme component has to implement the TS contract that the content plugin defines. The content plugin is the owner of that contract.

Imagine this: @theme/BlogSidebar is a theme-classic component. There's no guarantee that a custom theme would implement this, or that the theme wouldn't implement even more granular components.

I agree on this. @theme/BlogSidebar shouldn't be a content plugin because it's an implementation detail of the theme itself, and there's no addRoute({component: "@theme/BlogSidebar"})

However, it's not true for all those components you moved, such as @theme/BlogArchivePage

The plugin doesn't demand any theme component to exist for it to function.

It depends 😅

Therefore, we should move the provider of these types to theme-classic, because if we implement theme-tailwind with another set of components, we don't want the default type defs from content-blog to get in the way.

Theoretically I agree for third-party themes, but in our case the goal is to really have a 1-1 mapping between classic/tailwind components IMHO.

Eventually, we could even adopt a React-Native like convention, which basically choose the right implementation at build time:

• BlogSidebar.classic.tsx
• BlogSidebar.tailwind.tsx

[Josh-Cena]

The plugin doesn't demand @theme/BlogArchivePage either, to be exact. That's just a fallback value and is configurable.

What declare module '@theme/BlogArchivePage' does is not to define a contract, but to "polyfill" a type. In a perfect world, we should be able to tell TS that "@theme/BlogArchivePage should actually be resolved as theme-classic/lib/theme/BlogArchivePage/index.d.ts", but the path resolution is too complicated to be done using the paths compiler option, so we just conveniently used a module declaration instead. If you adopt this mental model, you can see that the @theme/BlogArchivePage alias should always be co-located with where the component is actually defined, even when that means duplicating the type def in multiple themes.

[slorber]

My rationale for keeping at least prop types in the plugin is that I'd like props to be typed on the content side too.

For example:

addRoute({
  component: "@theme/XYZ",
  props: {age: 42} // props should be type-checked here
)

We don't have this in place for now but it's something I'd like to have in the future (both a new "props" API + a way to typechecked modules created with createData)

Now for @theme/XYZ I understand it's a configurable fallback value. I'm ok to move it to the classic theme then, that seems fine, as long as the props can remain shared between the theme + content plugin

[Josh-Cena]

After some pondering, I do think it's valuable to put the theme aliases that the blog plugin expects within the plugin's typedefs, as a contract. However, the other theme-classic specific types should still be moved. Because of the number of conflicts I'll open a new one later