Skip to content
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

feat(theme-classic): syntax-highlighting for inline code #8717

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

zepatrik
Copy link

@zepatrik zepatrik commented Feb 27, 2023

Pre-flight checklist

  • I have read the Contributing Guidelines on pull requests.
  • If this is a code change: I have written unit tests and/or added dogfooding pages to fully verify the new behavior.
  • If this is a new API or substantial change: the PR has an accompanying issue (closes #0000) and the maintainers have approved on my working plan.

Motivation

Currently, there is no way to highlight the syntax in inline code.

Test Plan

I have added a section to the docs that uses the feature. I am not sure how to properly test this going further.

Test links

Screenshot from 2023-02-27 16-37-23

from https://deploy-preview-8717--docusaurus-2.netlify.app/docs/markdown-features/code-blocks/#syntax-highlighting

Deploy preview: https://deploy-preview-8717--docusaurus-2.netlify.app/

Related issues/PRs

I did not really find any.

@facebook-github-bot
Copy link
Contributor

Hi @zepatrik!

Thank you for your pull request and welcome to our community.

Action Required

In order to merge any pull request (code, docs, etc.), we require contributors to sign our Contributor License Agreement, and we don't seem to have one on file for you.

Process

In order for us to review and merge your suggested changes, please sign at https://code.facebook.com/cla. If you are contributing on behalf of someone else (eg your employer), the individual CLA may not be sufficient and your employer may need to sign the corporate CLA.

Once the CLA is signed, our tooling will perform checks and validations. Afterwards, the pull request will be tagged with CLA signed. The tagging process may take up to 1 hour after signing. Please give it that time before contacting us about it.

If you have received this in error or have any questions, please contact us at cla@meta.com. Thanks!

packages/docusaurus-theme-classic/src/theme-classic.d.ts Outdated Show resolved Hide resolved
const {
prism: {defaultLanguage},
} = useThemeConfig();
const language = languageProp ?? defaultLanguage ?? 'text';
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is also the worst-case fallback in the CodeBlock.

@zepatrik
Copy link
Author

I am not quite sure why the CI fails on this branch 😅 It does not really seem related, but 🤷

@Josh-Cena
Copy link
Collaborator

Josh-Cena commented Feb 27, 2023

@zepatrik _chalk.default.bold is not a function means an error was thrown in SSR. There's something terribly wrong with our server-side error logging, so this error gets logged instead. To debug, you can try editing this part:

} catch (err) {
// We are not using logger in this file, because it seems to fail with some
// compilers / some polyfill methods. This is very likely a bug, but in the
// long term, when we output native ES modules in SSR, the bug will be gone.
// prettier-ignore
console.error(chalk.red(`${chalk.bold('[ERROR]')} Docusaurus server-side rendering could not render static page with path ${chalk.cyan.underline(locals.path)}.`));
const isNotDefinedErrorRegex =
/(?:window|document|localStorage|navigator|alert|location|buffer|self) is not defined/i;
if (isNotDefinedErrorRegex.test((err as Error).message)) {
// prettier-ignore
console.info(`${chalk.cyan.bold('[INFO]')} It looks like you are using code that should run on the client-side only.
To get around it, try using ${chalk.cyan('`<BrowserOnly>`')} (${chalk.cyan.underline('https://docusaurus.io/docs/docusaurus-core/#browseronly')}) or ${chalk.cyan('`ExecutionEnvironment`')} (${chalk.cyan.underline('https://docusaurus.io/docs/docusaurus-core/#executionenvironment')}).
It might also require to wrap your client code in ${chalk.cyan('`useEffect`')} hook and/or import a third-party library dynamically (if any).`);
}
throw err;
}

Try removing all the chalk. calls, then run yarn build locally.

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Feb 27, 2023
@facebook-github-bot
Copy link
Contributor

Thank you for signing our Contributor License Agreement. We can now accept your code for this (and any) Meta Open Source project. Thanks!

@zepatrik
Copy link
Author

Thanks for the pointer @Josh-Cena, now it built locally at least.

@netlify
Copy link

netlify bot commented Feb 27, 2023

[V2]

Built without sensitive environment variables

Name Link
🔨 Latest commit 02a6e5c
🔍 Latest deploy log https://app.netlify.com/sites/docusaurus-2/deploys/63fe3650ba9ad90008cc8b51
😎 Deploy Preview https://deploy-preview-8717--docusaurus-2.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

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

@github-actions
Copy link

github-actions bot commented Feb 27, 2023

⚡️ Lighthouse report for the deploy preview of this PR

URL Performance Accessibility Best Practices SEO PWA Report
/ 🟠 87 🟢 97 🟢 92 🟢 100 🟢 90 Report
/docs/installation 🟠 80 🟢 100 🟢 100 🟢 100 🟢 90 Report

@slorber
Copy link
Collaborator

slorber commented Mar 3, 2023

Thanks, that looks like a nice feature to add.

Will review it soon, would like to try running this PR with my new visual diffing test project here, to see if there is any unexpected visual side-effect on our site: https://github.com/slorber/docusaurus-visual-tests

Something cool we could add later: a provider + docs/blog frontMatter to allow defining an inline code block language for a whole page or a whole children tree. If a blog post is 100% about TS that's cool to enable inline syntax highlighting everywhere by simply using markdown inline code blocks.

@zepatrik
Copy link
Author

zepatrik commented Mar 3, 2023

provider + docs/blog frontMatter to allow defining an inline code block language for a whole page or a whole children tree

Agreed, that would be very cool! Also fairly easy to implement, but the interface might not be straight-forward. But yeah, a topic for another day.

Btw, I saw that you are aiming for a v3 release already, is there any timeline for that? Will v2 be discontinued like v1 or maintained in parallel?

@slorber
Copy link
Collaborator

slorber commented Mar 16, 2023

Btw, I saw that you are aiming for a v3 release already, is there any timeline for that? Will v2 be discontinued like v1 or maintained in parallel?

As soon as MDX 2 + React 18 are ready, working on it but it's not so easy.

Will v2 be discontinued like v1 or maintained in parallel?

v3 will not be a total rewrite like v2 was so it should be easier to upgrade

https://docusaurus.io/community/release-process

CleanShot 2023-03-16 at 14 37 41@2x

@zepatrik
Copy link
Author

zepatrik commented May 2, 2023

Hey @slorber did you already try this PR with your visual diff tool? Do you need me to do something else to get this merged?

@slorber
Copy link
Collaborator

slorber commented May 3, 2023

@zepatrik until Docusaurus v3 is out this PR is not a top priority for me, unfortunately.

I'll review it when I can

Rebasing could trigger the Argos CI visual diff but I'm not sure the setup works great currently

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants