docs: document siteConfig.markdown + better mdx-loader retrocompat #8419
Conversation
slorber
added
the
pr: documentation
slorber
added
the
to backport
✅ [V2]
To edit notification comments on pull requests, go to your Netlify site settings. |
⚡️ Lighthouse report for the deploy preview of this PR
|
| @@ -173,7 +173,7 @@ export async function mdxLoader( | |||
| ...(reqOptions.beforeDefaultRemarkPlugins ?? []), | |||
| ...getAdmonitionsPlugins(reqOptions.admonitions ?? false), | |||
| ...DEFAULT_OPTIONS.remarkPlugins, | |||
| ...(reqOptions.markdownConfig.mermaid ? [mermaid] : []), | |||
| ...(reqOptions.markdownConfig?.mermaid ? [mermaid] : []), | |||
There was a problem hiding this comment.
This will be flagged by type-aware linting as unnecessary; we need to mark markdownConfig as optional in the options type. I'm not sure all this is necessary, though; markdownConfig is definitely required into the future.
There was a problem hiding this comment.
this is a bit unnecessary for sure but at least plugin authors will have less things breaking in v2
This will be flagged by type-aware linting as unnecessary;
We don't any anything being reported afaik, is this linting active?
I'm ok to just have this in v2, and remove this asap in v3 on the mdx 2 branch? We don't have to keep it, and it can be a temporary thing
There was a problem hiding this comment.
We don't any anything being reported afaik, is this linting active?
No, but I occasionally turn it on and fix reports locally. It's quite slow so I'm hesitant to add it to the CI.
I'm ok to just have this in v2, and remove this asap in v3 on the mdx 2 branch? We don't have to keep it, and it can be a temporary thing
That's reasonable, but people relying on internal APIs and coercing us to make changes sounds like a bad precedent. I'm okay to accept the change though.
There was a problem hiding this comment.
👍
Let's say it's not a commitment to always do this, and only a best effort we can do in case a small change from us can prevent a few plugins to break
There was a problem hiding this comment.
OK, but the type of Options.markdownConfig still needs to be marked as optional ^^
There was a problem hiding this comment.
I'd rather not to. Somehow it's not optional, and we want users eventually using MDXLoaderOptions to be forced to provide it appropriately, and yet allow JS users to not have any runtime users. That sometimes makes sense to program defensively assuming all users (plugin authors here) won't use TS.
|
Size Change: 0 B Total Size: 886 kB ℹ️ View Unchanged
|
slorber
removed
the
to backport
Motivation
Add missing
siteConfig.markdownAPI ref docImprove retro compatibility as requested in #8298
Test Plan
preview
Test links
Deploy preview: https://deploy-preview-8419--docusaurus-2.netlify.app/docs/api/docusaurus-config#markdown
Related issues/PRs
#8298