refactor(v2): improve and unify theme config types#4433
refactor(v2): improve and unify theme config types#4433armano2 wants to merge 1 commit intofacebook:mainfrom
Conversation
366fbe6 to
6a353d8
Compare
|
Deploy preview for docusaurus-2 ready! Built without sensitive environment variables with commit 366fbe6 |
|
⚡️ Lighthouse report for the changes in this PR:
Lighthouse ran on https://deploy-preview-4433--docusaurus-2.netlify.app/ |
|
Deploy preview for docusaurus-2 ready! Built without sensitive environment variables with commit db2171d |
|
[V1] Deploy preview failure Built without sensitive environment variables with commit 366fbe6 https://app.netlify.com/sites/docusaurus-1/deploys/604df3349d18b600084d6bc9 |
|
[V1] Deploy preview failure Built without sensitive environment variables with commit 6a353d8 https://app.netlify.com/sites/docusaurus-1/deploys/604df4eb881f23000797f673 |
There was a problem hiding this comment.
Thanks for the cleanup :)
There's something that bothers: you moved all the theme types to @docusaurus/types but this type package is meant to be used for core types, not plugin/theme types, as these types are specific to one theme in particular, but some users might want to use Docusaurus with a totally different theme with different types. That's why {} is a valid/default type and the core ThemeConfig should remain Record<string,any> or something similar. Note it's possible to use Docusaurus with multiple themes at the same time, and we end up with a composition where the final theme config is the theme config of the 2 underlying themes (ClassicThemeConfig & LiveCodeBlockThemeConfig)
In practice, the plan is to share a common ThemeConfig type across multiple themes (classic/bootstrap/tailwind), but keep the core agnostic of those types, that's why the types were put in @docusaurus/theme-common
For now, it's not necessary to take any care of the bootstrap theme, it is not ready for usage at all.
|
ok, I'm going to update this with module augmentations |
|
[V1] Deploy preview failure Built without sensitive environment variables with commit 7532e80 https://app.netlify.com/sites/docusaurus-1/deploys/604edd5fa8fc4a000790fb35 |
|
[V1] Deploy preview failure Built without sensitive environment variables with commit db2171d https://app.netlify.com/sites/docusaurus-1/deploys/60548bd4733f1800088c5e7c |
7532e80 to
db2171d
Compare
|
Closing because of how quickly things have evolved. A lot of things have already been addressed. Thanks @armano2 anyways :D |
Motivation
i'v tried making this change minimal, and not touch to much of code, but going from
anyto proper type showed some errors#4385 (comment)
Have you read the Contributing Guidelines on pull requests?
yes