-
-
Notifications
You must be signed in to change notification settings - Fork 8.5k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
refactor(blog): theme-common shouldn't depend on blog content plugins #10313
Conversation
New dependencies detected. Learn more about Socket for GitHub ↗︎
|
⚡️ Lighthouse report for the deploy preview of this PR
|
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site configuration. |
Size Change: 0 B Total Size: 1.85 MB ℹ️ View Unchanged
|
Last time we brought this up, I think you were suggesting that |
At the time we introduced Now we can, and started introducing some But the thing is, we have to make a decision: either Having docs/blog code spread in both theme-common and docs/blog client exports is problematic because for some feature I wanted to implement previously, I wanted to import things from both dependencies, and couldn't because it introduces a cycle. I now think it's better to move plugin-specific code to their respective plugin client export. It will also be easier to write public API documentation for those APIs. If you think about it, most of this code is code that supports the plugin on the client side. It is not really related to theming, but rather style-agnostic code that simply manipulates the data/props/context that the plugin made available. Theme-common has reusable utils, but if you think about it, it remains a bit opinionated. If I were to create a new blog plugin from scratch that does not look like the official docusaurus theme, and if I don't plan to use export {
BlogPostProvider,
type BlogPostContextValue,
useBlogPost,
useBlogMetadata,
} from './contexts';
export {
useBlogListPageStructuredData,
useBlogPostStructuredData,
} from './structuredDataUtils';
export {
BlogSidebarItemList,
groupBlogSidebarItemsByYear,
useVisibleBlogSidebarItems,
} from './sidebarUtils'; (one could argue The way I see it:
Hope this makes sense Note that we could eventually split later |
Breaking change?
Internal APIs have been moved, this is not considered a breaking change requiring a new major version, although swizzle users might need to update docs/blog theme-common imports in their code:
Motivation
We have had a coupling problem for a while, that led to issues such as cyclic dependencies when trying to implement certain features in plugin-specific client exports.
All plugin-specific browser code should now be in
@docusaurus/<contentPlugin>/client
and not@docusaurus/theme-common
that should remain plugin-agnostic.For now, I only refactored the blog plugin code. The docs plugin will follow but it's more difficult.
Edit: docs follow-up PR #10316
Test Plan
Unit tests + CI + preview
Test links
https://deploy-preview-10313--docusaurus-2.netlify.app/