refactor(theme-classic): cleanup of code blocks

[lex111]

Motivation

Just small cleanups in the code.

Have you read the Contributing Guidelines on pull requests?

Yes

Test Plan

Preview (tests pages)

Related PRs

[lex111]

Wonder if we should remove this language CSS class from the code container, as Prism add it to pre element?

[Josh-Cena]

Let's settle on using // TODO in the future, because only this can be tracked by my vscode extension:

[netlify]

✅ [V2]

Name	Link
🔨 Latest commit	3a25e2b
🔍 Latest deploy log	https://app.netlify.com/sites/docusaurus-2/deploys/623c92fdfbab1600089fe29c
😎 Deploy Preview	https://deploy-preview-6987--docusaurus-2.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[Josh-Cena]

Wonder if we should remove this language CSS class from the code container,

The div one is added by MDX, iirc. It's added again in the CodeBlock component so that the div would still get this class name when used in JSX. You can try removing it in our code, and check if it still appears.

[lex111]

As I can see, MDX doesn't produce any div for code blocks -- all these elements inside code blocks are created inside <CodeBlock>, or am I misunderstanding something? Initially I had a question, why do we need to add another (duplicated) CSS class (see below), if it's already added to the <pre> element automatically.

docusaurus/packages/docusaurus-theme-classic/src/theme/CodeBlock/index.tsx

Lines 104 to 105 in 4c0914c

	[`language-${language}`]:
	language && !blockClassName.includes(`language-${language}`),

[Josh-Cena]

In MDXComponents, we map pre to CodeBlock:

docusaurus/packages/docusaurus-theme-classic/src/theme/MDXComponents/Code.tsx

Lines 32 to 37 in c42f22b

	return shouldBeInline ? (
	<code {...props} />
	) : (
	<CodeBlock {...(props as ComponentProps<typeof CodeBlock>)} />
	);
	}

And language-xxx is part of the default blockClassName. You can experiment by removing that [`language-${language}`]: language && !blockClassName.includes(`language-${language}`), ; I just tried, the language-xxx is indeed still there because it's added by MDX.

[lex111]

Uh-huh, I get it, but then why are we trying to manually add a CSS class with language? Does it mean that [`language-${language}`]: language && !blockClassName.includes(`language-${language}`) can be removed?

[Josh-Cena]

Because we allow users to use <CodeBlock language="java"> within React pages. If we don't manually apply this class name, that means div.language-java will not be able to consistently target Java code block containers. I doubt if it happens a lot, but it's always good to be consistent.

[slorber]

🤪 sorry but not sure to understand if this is ready to merge and you both agree or not?

That looks good to me but not 100% sure about the className change

[Josh-Cena]

The current change looks fair to me. I think language-js needs to stay but the current js class name on the inner content can be removed.