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

Components: Fix race conditions in SyntaxHighlighter #18158

Merged
merged 2 commits into from
May 6, 2022

Conversation

okonet
Copy link
Contributor

@okonet okonet commented May 6, 2022

Issue: #18090

What I did

Replace code with div to prevent PrismJS to highlight the code after React has finished doing so.

Apparently, PrimsJS that is pre-bundled with the syntax highlighter library was running globally after it would load and replace already highlighted code with [object Object].

How to test

Check out instructions in #18090

@nx-cloud
Copy link

nx-cloud bot commented May 6, 2022

☁️ Nx Cloud Report

CI is running/has finished running commands for commit ca24412. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this branch


✅ Successfully ran 1 target

Sent with 💌 from NxCloud.

We can't use `code` since PrismJS races for it.
See https://github.com/storybookjs/storybook/issues/18090
*/
const Code = styled.div(({ theme }) => ({
Copy link
Member

Choose a reason for hiding this comment

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

For anyone watching, it's easy to miss:

We're changing the <code> to a <div>.

We do this because Prismjs wants to format code directly on the DOM itself.
It's targeting these DOM-nodes..

We can either patch Prismjs or make sure the DOM-nodes it targets don't exists.. and that'd what we opted to do here.

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

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

Great work @okonet @ndelangen 👏

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

Successfully merging this pull request may close these issues.

3 participants