fix(rehype-shiki): sequential codeboxes are merged together by AugustinMauroy · Pull Request #7805 · nodejs/nodejs.org

AugustinMauroy · 2025-06-01T14:31:17Z

Description

It's add some logic to merge or not two code snippets

adding concurency take 110ms to 90ms

Validation

test should pass

Related Issues

close #7803

Check List

I have read the Contributing Guidelines and made commit messages that follow the guideline.
I have run pnpm format to ensure the code follows the style guide.
I have run pnpm test to check if all tests are passing.
I have run pnpm build to check if the website builds without errors.
I've covered new added functionality with unit tests if necessary.

vercel · 2025-06-01T14:31:21Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
nodejs-org	✅ Ready (Inspect)	Visit Preview	Jun 28, 2025 2:10pm

Copilot

Pull Request Overview

This PR refactors the rehype-shiki plugin to merge adjacent code blocks (specifically CJS/ESM pairs) into grouped CodeTabs elements while updating the relevant tests.

Refactored code block grouping logic by replacing the visit traversal with a while loop and newChildren array.
Updated tests to verify the new grouping behavior and added concurrency options.
Adjusted the highlighter tests to use the new concurrent test configuration.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
packages/rehype-shiki/src/plugin.mjs	Refactored merging of sequential code blocks into CodeTabs and updated syntax highlighting integration.
packages/rehype-shiki/src/tests/plugin.test.mjs	Revised tests for the grouping functionality with updated expectations and concurrency settings.
packages/rehype-shiki/src/tests/highlighter.test.mjs	Minor test update to include concurrency settings.

Copilot · 2025-06-01T14:31:54Z

packages/rehype-shiki/src/plugin.mjs

+                languages: actualLanguagesArray.join('|'),
+              },
+              children: codeBlocksToGroup.map(block =>
+                JSON.parse(JSON.stringify(block))


Consider using a dedicated deep clone utility instead of JSON.parse(JSON.stringify(...)) to improve performance and maintainability when cloning code blocks.

packages/rehype-shiki/src/__tests__/plugin.test.mjs

codecov · 2025-06-01T14:32:34Z

❌ 5 Tests Failed:

Tests completed	Failed	Passed	Skipped
12	5	7	0

View the top 3 failed test(s) by shortest run time

test::should create CodeTabs for a sequence n snippet of text code blocks

Stack Traces | 0.0035s run time

Error [ERR_TEST_FAILURE]: Should create a single CodeTabs with all text code blocks
+ actual - expected

  {
    children: [
      {
+       data: {
+         meta: 'displayName="Text 1"'
+       },
+       properties: {
+         className: [
+           'language-text'
+         ]
+       },
+       tagName: 'code'
-       children: [
-         {
-           data: {
-             meta: 'displayName="Text 1"'
-           },
-           properties: {
-             className: [
-               'language-text'
-             ]
-           },
-           tagName: 'code'
-         }
-       ],
-       tagName: 'pre'
      },
-     {
-       children: [
-         {
-           data: {
-             meta: 'displayName="Text 2"'
-           },
-           properties: {
-             className: [
-               'language-text'
-             ]
-           },
-           tagName: 'code'
-         }
-       ],
-       tagName: 'pre'
-     },
-     {
-       children: [
-         {
-           data: {
-             meta: 'displayName="Text 3"'
-           },
-           properties: {
-             className: [
-               'language-text'
-             ]
-           },
-           tagName: 'code'
-         }
-       ],
-       tagName: 'pre'
-     }
    ],
+   tagName: 'pre'
-   properties: {
-     displayNames: 'Text 1|Text 2|Text 3',
-     languages: 'text|text|text'
-   },
-   tagName: 'CodeTabs',
-   type: 'element'
  }

    at new Promise (<anonymous>)
    at Array.map (<anonymous>) {
  code: 'ERR_TEST_FAILURE',
  failureType: 'testCodeFailure',
  cause: AssertionError [ERR_ASSERTION]: Should create a single CodeTabs with all text code blocks
  + actual - expected
  
    {
      children: [
        {
  +       data: {
  +         meta: 'displayName="Text 1"'
  +       },
  +       properties: {
  +         className: [
  +           'language-text'
  +         ]
  +       },
  +       tagName: 'code'
  -       children: [
  -         {
  -           data: {
  -             meta: 'displayName="Text 1"'
  -           },
  -           properties: {
  -             className: [
  -               'language-text'
  -             ]
  -           },
  -           tagName: 'code'
  -         }
  -       ],
  -       tagName: 'pre'
        },
  -     {
  -       children: [
  -         {
  -           data: {
  -             meta: 'displayName="Text 2"'
  -           },
  -           properties: {
  -             className: [
  -               'language-text'
  -             ]
  -           },
  -           tagName: 'code'
  -         }
  -       ],
  -       tagName: 'pre'
  -     },
  -     {
  -       children: [
  -         {
  -           data: {
  -             meta: 'displayName="Text 3"'
  -           },
  -           properties: {
  -             className: [
  -               'language-text'
  -             ]
  -           },
  -           tagName: 'code'
  -         }
  -       ],
  -       tagName: 'pre'
  -     }
      ],
  +   tagName: 'pre'
  -   properties: {
  -     displayNames: 'Text 1|Text 2|Text 3',
  -     languages: 'text|text|text'
  -   },
  -   tagName: 'CodeTabs',
  -   type: 'element'
    }
  
      at TestContext.<anonymous> (file:.../runner/work/nodejs.org/nodejs..../src/__tests__/plugin.test.mjs:767:12)
      at Test.runInAsyncScope (node:async_hooks:214:14)
      at Test.run (node:internal/test_runner/test:1047:25)
      at Test.start (node:internal/test_runner/test:944:17)
      at node:internal/test_runner/test:1440:71
      at node:internal/per_context/primordials:483:82
      at new Promise (<anonymous>)
      at new SafePromise (node:internal/per_context/primordials:451:29)
      at node:internal/per_context/primordials:483:9
      at Array.map (<anonymous>) {
    generatedMessage: false,
    code: 'ERR_ASSERTION',
    actual: { tagName: 'pre', children: [Array] },
    expected: { tagName: 'CodeTabs', type: 'element', properties: [Object], children: [Array] },
    operator: 'deepStrictEqual'
  }
}

test::should create CodeTabs for a sequence n snippet of ESM/CJS code blocks

Stack Traces | 0.00613s run time

Error [ERR_TEST_FAILURE]: Should create a single CodeTabs with all ESM code blocks
+ actual - expected

  {
    children: [
      {
+       data: {
+         meta: 'displayName="ESM"'
+       },
+       properties: {
+         className: [
+           'language-esm'
+         ]
+       },
+       tagName: 'code'
-       children: [
-         {
-           data: {
-             meta: 'displayName="ESM"'
-           },
-           properties: {
-             className: [
-               'language-esm'
-             ]
-           },
-           tagName: 'code'
-         }
-       ],
-       tagName: 'pre'
      },
-     {
-       children: [
-         {
-           data: {
-             meta: 'displayName="ESM"'
-           },
-           properties: {
-             className: [
-               'language-esm'
-             ]
-           },
-           tagName: 'code'
-         }
-       ],
-       tagName: 'pre'
-     },
-     {
-       children: [
-         {
-           data: {
-             meta: 'displayName="ESM"'
-           },
-           properties: {
-             className: [
-               'language-esm'
-             ]
-           },
-           tagName: 'code'
-         }
-       ],
-       tagName: 'pre'
-     },
-     {
-       children: [
-         {
-           data: {
-             meta: 'displayName="ESM"'
-           },
-           properties: {
-             className: [
-               'language-esm'
-             ]
-           },
-           tagName: 'code'
-         }
-       ],
-       tagName: 'pre'
-     }
    ],
+   tagName: 'pre'
-   properties: {
-     displayNames: 'ESM|ESM|ESM|ESM',
-     languages: 'esm|esm|esm|esm'
-   },
-   tagName: 'CodeTabs',
-   type: 'element'
  }

    at new Promise (<anonymous>)
    at Array.map (<anonymous>) {
  code: 'ERR_TEST_FAILURE',
  failureType: 'testCodeFailure',
  cause: AssertionError [ERR_ASSERTION]: Should create a single CodeTabs with all ESM code blocks
  + actual - expected
  
    {
      children: [
        {
  +       data: {
  +         meta: 'displayName="ESM"'
  +       },
  +       properties: {
  +         className: [
  +           'language-esm'
  +         ]
  +       },
  +       tagName: 'code'
  -       children: [
  -         {
  -           data: {
  -             meta: 'displayName="ESM"'
  -           },
  -           properties: {
  -             className: [
  -               'language-esm'
  -             ]
  -           },
  -           tagName: 'code'
  -         }
  -       ],
  -       tagName: 'pre'
        },
  -     {
  -       children: [
  -         {
  -           data: {
  -             meta: 'displayName="ESM"'
  -           },
  -           properties: {
  -             className: [
  -               'language-esm'
  -             ]
  -           },
  -           tagName: 'code'
  -         }
  -       ],
  -       tagName: 'pre'
  -     },
  -     {
  -       children: [
  -         {
  -           data: {
  -             meta: 'displayName="ESM"'
  -           },
  -           properties: {
  -             className: [
  -               'language-esm'
  -             ]
  -           },
  -           tagName: 'code'
  -         }
  -       ],
  -       tagName: 'pre'
  -     },
  -     {
  -       children: [
  -         {
  -           data: {
  -             meta: 'displayName="ESM"'
  -           },
  -           properties: {
  -             className: [
  -               'language-esm'
  -             ]
  -           },
  -           tagName: 'code'
  -         }
  -       ],
  -       tagName: 'pre'
  -     }
      ],
  +   tagName: 'pre'
  -   properties: {
  -     displayNames: 'ESM|ESM|ESM|ESM',
  -     languages: 'esm|esm|esm|esm'
  -   },
  -   tagName: 'CodeTabs',
  -   type: 'element'
    }
  
      at TestContext.<anonymous> (file:.../runner/work/nodejs.org/nodejs..../src/__tests__/plugin.test.mjs:667:12)
      at Test.runInAsyncScope (node:async_hooks:214:14)
      at Test.run (node:internal/test_runner/test:1047:25)
      at Test.start (node:internal/test_runner/test:944:17)
      at node:internal/test_runner/test:1440:71
      at node:internal/per_context/primordials:483:82
      at new Promise (<anonymous>)
      at new SafePromise (node:internal/per_context/primordials:451:29)
      at node:internal/per_context/primordials:483:9
      at Array.map (<anonymous>) {
    generatedMessage: false,
    code: 'ERR_ASSERTION',
    actual: { tagName: 'pre', children: [Array] },
    expected: { tagName: 'CodeTabs', type: 'element', properties: [Object], children: [Array] },
    operator: 'deepStrictEqual'
  }
}

test::If there are a sequence of codeblock of CJS/ESM, it should create pairs of CodeTabs

Stack Traces | 0.00849s run time

Error [ERR_TEST_FAILURE]: Should create two CodeTabs groups

4 !== 2

    at new Promise (<anonymous>)
    at Array.map (<anonymous>) {
  code: 'ERR_TEST_FAILURE',
  failureType: 'testCodeFailure',
  cause: AssertionError [ERR_ASSERTION]: Should create two CodeTabs groups
  
  4 !== 2
  
      at TestContext.<anonymous> (file:.../runner/work/nodejs.org/nodejs..../src/__tests__/plugin.test.mjs:290:12)
      at Test.runInAsyncScope (node:async_hooks:214:14)
      at Test.run (node:internal/test_runner/test:1047:25)
      at Test.start (node:internal/test_runner/test:944:17)
      at node:internal/test_runner/test:1440:71
      at node:internal/per_context/primordials:483:82
      at new Promise (<anonymous>)
      at new SafePromise (node:internal/per_context/primordials:451:29)
      at node:internal/per_context/primordials:483:9
      at Array.map (<anonymous>) {
    generatedMessage: false,
    code: 'ERR_ASSERTION',
    actual: 4,
    expected: 2,
    operator: 'strictEqual'
  }
}

To view more test analytics, go to the Test Analytics Dashboard
_{📋 Got 3 mins? Take this short survey to help us improve Test Analytics.}

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Augustin Mauroy <97875033+AugustinMauroy@users.noreply.github.com>

avivkeller · 2025-06-01T14:50:21Z

packages/rehype-shiki/src/__tests__/plugin.test.mjs

+    const firstGroup = parent.children[0];
+    assert.strictEqual(
+      firstGroup.tagName,
+      'CodeTabs',
+      'Group 1 should be CodeTabs'
+    );
+    assert.strictEqual(
+      firstGroup.properties.languages,
+      'cjs|esm',
+      'Group 1 languages should be cjs|esm'
+    );
+    assert.strictEqual(
+      firstGroup.properties.displayNames,
+      'CJS|ESM',
+      'Group 1 displayNames should be CJS|ESM'
+    );
+    assert.ok(
+      Array.isArray(firstGroup.children) && firstGroup.children.length === 2,
+      'Group 1 should contain 2 code blocks'
+    );
+    assert.strictEqual(
+      firstGroup.children[0].children[0].properties.className[0],
+      'language-cjs',
+      'Group 1, Block 1 should be CJS'
+    );
+    assert.strictEqual(
+      firstGroup.children[1].children[0].properties.className[0],
+      'language-esm',
+      'Group 1, Block 2 should be ESM'
+    );


Can we use deepStrictEqual or partialDeepStrictEqual instead of comparing each with it's own assert?

avivkeller · 2025-06-01T14:50:51Z

packages/rehype-shiki/src/__tests__/plugin.test.mjs

@@ -19,18 +19,21 @@ mock.module('unist-util-visit', {
  namedExports: { visit: mockVisit, SKIP: Symbol() },


Suggested change

namedExports: { visit: mockVisit, SKIP: Symbol() },

namedExports: { visit: mockVisit },

If you aren't relying on SKIP anymore, no need to mock it

avivkeller · 2025-06-01T14:52:29Z

packages/rehype-shiki/src/plugin.mjs

+    // First pass: Group adjacent code blocks into <CodeTabs>
+    const newChildren = [];
+    let i = 0;
+    while (i < tree.children.length) {


I'm not a huge fan that we are using a while loop and a visit, is there an alternative?

avivkeller · 2025-06-01T14:52:49Z

packages/rehype-shiki/src/plugin.mjs

+                    (lang2 === 'cjs' && lang3 === 'esm') ||
+                    (lang2 === 'esm' && lang3 === 'cjs')


Maybe make a groupableLangs constant?

avivkeller · 2025-06-01T14:53:24Z

packages/rehype-shiki/src/plugin.mjs

+      const lang = preElement.properties.className
+        ?.find(cn => cn.startsWith(languagePrefix))
+        ?.substring(languagePrefix.length);


getLanguage?

ovflowd · 2025-06-02T11:26:16Z

apps/site/pages/en/about/index.mdx

 });
 ```

+```cjs


But that is intentional. Sequential codeboxes are supposed to be merged together afaik.

take a look to the issue. In doc it's supposed to be separated

Then let's follow it the way it is supposed to be.

packages/rehype-shiki/src/plugin.mjs

ovflowd · 2025-06-27T20:04:33Z

@AugustinMauroy are you going to put some effort here?

AugustinMauroy · 2025-06-28T13:19:01Z

@ovflowd I'm waiting for consensus on #7805 (comment)

Co-authored-by: Manish Kumar ⛄ <manishprivet@protonmail.com> Signed-off-by: Augustin Mauroy <97875033+AugustinMauroy@users.noreply.github.com>

AugustinMauroy · 2025-06-28T14:08:34Z

okay let's me put some energy on that

avivkeller · 2025-07-19T13:58:45Z

@AugustinMauroy Are you still working on this, or has it staled out?

AugustinMauroy · 2025-07-19T14:00:17Z

@AugustinMauroy Are you still working on this, or has it staled out?

I tried a few things yesterday but I couldn't get anywhere, just endless loops...
If you want to take over the job, go ahead.

avivkeller · 2025-07-19T14:05:54Z

Perhaps open an help-wanted issue with more details on the issue?

AugustinMauroy · 2025-07-19T14:21:11Z

Perhaps open an help-wanted issue with more details on the issue?

already exist #7803

fix(rehype-shiki): sequential codeboxes are merged together

6624ef4

Copilot AI review requested due to automatic review settings June 1, 2025 14:31

AugustinMauroy requested a review from a team as a code owner June 1, 2025 14:31

vercel bot deployed to Preview June 1, 2025 14:31 View deployment

Copilot AI reviewed Jun 1, 2025

View reviewed changes

fix: typo

9516d70

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> Signed-off-by: Augustin Mauroy <97875033+AugustinMauroy@users.noreply.github.com>

vercel bot deployed to Preview June 1, 2025 14:35 View deployment

AugustinMauroy marked this pull request as draft June 1, 2025 14:39

avivkeller reviewed Jun 1, 2025

View reviewed changes

WIP

bc69c5b

vercel bot deployed to Preview June 1, 2025 21:11 View deployment

ovflowd reviewed Jun 2, 2025

View reviewed changes

manishprivet reviewed Jun 17, 2025

View reviewed changes

packages/rehype-shiki/src/plugin.mjs Outdated Show resolved Hide resolved

Update packages/rehype-shiki/src/plugin.mjs

7e06603

Co-authored-by: Manish Kumar ⛄ <manishprivet@protonmail.com> Signed-off-by: Augustin Mauroy <97875033+AugustinMauroy@users.noreply.github.com>

vercel bot deployed to Preview June 28, 2025 14:10 View deployment

avivkeller closed this Aug 8, 2025

AugustinMauroy deleted the fix-shiki-code-tab branch October 30, 2025 09:37

		@@ -19,18 +19,21 @@ mock.module('unist-util-visit', {
		namedExports: { visit: mockVisit, SKIP: Symbol() },

	namedExports: { visit: mockVisit, SKIP: Symbol() },
	namedExports: { visit: mockVisit },

		(lang2 === 'cjs' && lang3 === 'esm') \|\|
		(lang2 === 'esm' && lang3 === 'cjs')

               });
               ```
+              ```cjs

Uh oh!

Conversation

AugustinMauroy commented Jun 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Validation

Related Issues

Check List

Uh oh!

vercel bot commented Jun 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Jun 1, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

codecov bot commented Jun 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

❌ 5 Tests Failed:

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

ovflowd commented Jun 27, 2025

Uh oh!

AugustinMauroy commented Jun 28, 2025

Uh oh!

AugustinMauroy commented Jun 28, 2025

Uh oh!

avivkeller commented Jul 19, 2025

Uh oh!

AugustinMauroy commented Jul 19, 2025

Uh oh!

avivkeller commented Jul 19, 2025

Uh oh!

AugustinMauroy commented Jul 19, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

AugustinMauroy commented Jun 1, 2025 •

edited

Loading

vercel bot commented Jun 1, 2025 •

edited

Loading

codecov bot commented Jun 1, 2025 •

edited

Loading