perf(CodeHighlighter): add on-demand language loading to reduce bundle size#1691
Open
perf(CodeHighlighter): add on-demand language loading to reduce bundle size#1691
Conversation
Contributor
|
|
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the
📝 WalkthroughWalkthrough为 CodeHighlighter 引入基于 prismLightMode 的按需 Prism 语言懒加载，重构渲染流程以支持 Suspense 回退，并在 Mermaid 中将代码渲染替换为 CodeHighlighter；同步更新接口、示例与测试以覆盖新行为。 Changes
Sequence Diagram(s)sequenceDiagram
participant App as 上层组件
participant CH as CodeHighlighter
participant Lazy as React.lazy
participant Prism as Prism (语言模块)
participant Susp as Suspense/Fallback
App->>CH: 渲染 (prismLightMode=true, lang=jsx)
alt 指定 lang 且 prismLightMode=true
CH->>Lazy: 请求/创建 AsyncHighlighter
Lazy->>Prism: 动态导入 prism- jsx 模块
par 加载中
CH->>Susp: 返回 Suspense 回退（或纯 <code>）
and 加载完成
Prism-->>Lazy: 语言模块加载完成
Lazy->>CH: AsyncHighlighter 可用
CH->>App: 渲染带高亮的代码节点
end
else prismLightMode=false 或 未指定 lang
CH->>App: 直接使用全量高亮器或返回纯 <code>
end
Estimated code review effort🎯 4 (复杂) | ⏱️ ~45 分钟 庆贺诗
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Bundle ReportChanges will decrease total bundle size by 2.54MB (-55.52%) ⬇️. This is within the configured threshold ✅ Detailed changes
Affected Assets, Files, and Routes:view changes for bundle: x-markdown-array-pushAssets Changed:
view changes for bundle: antdx-array-pushAssets Changed:
|
Deploying ant-design-x with
|
| Latest commit: |
eafb277
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://5d83b9a0.ant-design-x.pages.dev |
| Branch Preview URL: | https://build-codehight-build.ant-design-x.pages.dev |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## feature #1691 +/- ##
===========================================
- Coverage 97.32% 97.32% -0.01%
===========================================
Files 144 144
Lines 4568 4597 +29
Branches 1264 1277 +13
===========================================
+ Hits 4446 4474 +28
- Misses 120 121 +1
Partials 2 2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Contributor
size-limit report 📦
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/x/components/code-highlighter/CodeHighlighter.tsx (1)
2-75: 应改用PrismAsyncLight并调用loadLanguage()实现真正的按需加载当前实现有两处致命问题：
- 第 3 行的全量 Prism 导入 — 仍然会把所有语言打包进去，无法减包。
- 第 51 行的动态导入未生效 — 从
prismjs/components导入的模块没有注册到 react-syntax-highlighter，语言定义未被识别。正确的做法是改用
PrismAsyncLight（或PrismLight），并调用官方的loadLanguage(lang)API，同时从正确的路径react-syntax-highlighter/dist/esm/languages/prism/<language>导入语言模块。这样才能真正实现代码分割和按需加载。
🤖 Fix all issues with AI agents
In `@packages/x/components/code-highlighter/CodeHighlighter.tsx`:
- Around line 77-85: The component currently returns a bare <code> when lang is
missing, skipping the outer wrapper/header/language title/copy button; instead,
default lang to 'plaintext' and let the normal render path run so the outer
container and controls remain consistent. Concretely, in the CodeHighlighter
component replace the early-return for falsy lang (but keep the early-return for
no children) by assigning a fallback like const effectiveLang = lang ||
'plaintext' (or similar) and use effectiveLang in the existing
highlighting/render logic so header, language label and copy button (the same
elements used when lang is present) are still rendered.
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/x/components/code-highlighter/CodeHighlighter.tsx`:
- Around line 41-75: The current logic creates a new React.lazy component inside
useMemo whenever lang changes, causing remounts and losing lazy caching; move
lazy creation to a module-level cache (e.g., Map<string, LazyComponent>) keyed
by lang and create the lazy AsyncSyntaxHighlighter only once per language (use a
cached entry for subsequent renders), then have useMemo simply select either the
cached lazy component (when prismLightMode && lang) or the regular
SyntaxHighlighter; keep the same fallback behavior (returning a component that
renders <SyntaxHighlighter language={lang} PreTag="div" />) and reference the
existing symbols AsyncHighlighter, AsyncSyntaxHighlighter, SyntaxHighlighter,
lang, and prismLightMode when implementing the cache.
- Line 51: The dynamic import using import(`prismjs/components/prism-${lang}`)
in CodeHighlighter (the langModule load) creates an unsafe context import that
pulls all prism language modules; update the implementation to constrain
bundlers: either switch to a webpack-friendly dynamic import with magic comments
and explicit .js extension (replace the template import of prism-${lang} with a
webpackInclude/webpackChunkName variant), or refactor to use import.meta.glob to
pre-declare prismjs/components/prism-*.js and load from that map at runtime, or
replace the dynamic import with a static mapping of language name → loader
function; modify the code that assigns langModule to load via the chosen
constrained approach so bundlers only include needed files.
♻️ Duplicate comments (1)
packages/x/components/code-highlighter/CodeHighlighter.tsx (1)
82-85: 当lang为空时返回裸<code>元素的问题已在之前的评审中指出此问题已被标记，建议使用
plaintext作为默认语言以保持一致的组件结构。
🧹 Nitpick comments (1)
packages/x/components/code-highlighter/__tests__/index.test.tsx (1)
115-124: 建议使用异步测试工具验证 Suspense 行为当前测试是同步的，但
prismLightMode启用时组件使用Suspense进行异步加载。虽然测试可能因 fallback 内容而通过，但无法验证实际的异步加载流程。建议使用
waitFor或findBy*查询来验证异步行为：♻️ 建议的改进
+import { render, waitFor } from '@testing-library/react'; + describe('prismLightMode', () => { - it('should render code with prismLightMode enabled', () => { + it('should render code with prismLightMode enabled', async () => { const { container } = render( <CodeHighlighter lang="javascript" prismLightMode> {`console.log("test");`} </CodeHighlighter>, ); - expect(container.querySelector('code')).toBeInTheDocument(); - expect(container.textContent).toContain('console.log("test");'); + // Verify fallback is shown initially + expect(container.textContent).toContain('console.log("test");'); + + // Wait for async loading to complete + await waitFor(() => { + expect(container.querySelector('pre')).toBeInTheDocument(); + }); });
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In `@packages/x/components/code-highlighter/__tests__/index.test.tsx`:
- Around line 388-400: The RTL test currently only sets data-direction on
CodeHighlighter; update the test in index.test.tsx to mock or wrap with the
XProviderContext (or XProvider) and set direction='rtl' so the component
receives real context-based RTL; then assert that the rendered element
(querySelector('.ant-codeHighlighter')) contains the RTL class (e.g.
'.ant-codeHighlighter-rtl') and/or appropriate RTL attributes to confirm
behavior; target the test block named 'should render with RTL direction' and the
CodeHighlighter component to implement this change.
- Around line 351-361: The test description and assertions are wrong because
CodeHighlighter returns early when lang=="" (see CodeHighlighter early return at
lines ~95-97), so either change the test to assert the early-return behavior
(expect a bare <code> render) or modify the test inputs to actually trigger
FullPrismHighlighter; e.g., call render(<CodeHighlighter lang="js"
prismLightMode={false}>...) or omit the lang prop/use undefined so the component
proceeds to load FullPrismHighlighter, and update the it(...) description
accordingly to match the new expectation.
🧹 Nitpick comments (1)
packages/x/components/code-highlighter/CodeHighlighter.tsx (1)
80-87: 存在未使用的代码路径。当
prismLightMode=true且lang为空时，第 86 行会将SyntaxHighlighter赋值给Highlighter，但第 95-97 行的 early return 会在使用前退出。考虑将 early return 移到Highlighter赋值之前，或添加注释说明此行为。♻️ 建议的重构方案
+ // ============================ Early Returns ============================ + if (!children) { + return null; + } + + // No lang means no highlighting needed, return plain code directly + if (!lang) { + return <code>{children}</code>; + } + // Get the appropriate highlighter component // - prismLightMode = true (default): Use PrismLight with async language loading // - prismLightMode = false: Use full Prism (all languages included) - const Highlighter = prismLightMode - ? lang - ? getAsyncHighlighter(lang) - : SyntaxHighlighter - : getFullPrismHighlighter(); - - // ============================ Early Returns ============================ - if (!children) { - return null; - } - - // No lang means no highlighting needed, return plain code directly - if (!lang) { - return <code>{children}</code>; - } + const Highlighter = prismLightMode + ? getAsyncHighlighter(lang) + : getFullPrismHighlighter();
Comment on lines
351
to
361
| it('should use FullPrismHighlighter when prismLightMode=false and no lang', async () => { | ||
| const { container } = render( | ||
| <CodeHighlighter lang="" prismLightMode={false}> | ||
| {`plain code`} | ||
| </CodeHighlighter>, | ||
| ); | ||
| // Should use full Prism with Suspense fallback | ||
| await waitFor(() => { | ||
| expect(container.textContent).toContain('plain code'); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
测试用例与实际行为不符。
当 lang="" 时，组件在第 95-97 行会 early return 返回裸 <code> 元素，不会使用 FullPrismHighlighter。此测试实际上测试的是 early return 行为，而非 FullPrismHighlighter 的加载。测试描述与实际行为不匹配。
💚 建议的修复
- it('should use FullPrismHighlighter when prismLightMode=false and no lang', async () => {
+ it('should render plain code when no lang (early return before FullPrismHighlighter)', async () => {
const { container } = render(
<CodeHighlighter lang="" prismLightMode={false}>
{`plain code`}
</CodeHighlighter>,
);
- // Should use full Prism with Suspense fallback
+ // Should early return with plain <code> element (no highlighting)
await waitFor(() => {
expect(container.textContent).toContain('plain code');
});
+ // Verify it's the early return path (bare code element)
+ expect(container.querySelector('.ant-codeHighlighter')).toBeNull();
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| it('should use FullPrismHighlighter when prismLightMode=false and no lang', async () => { | |
| const { container } = render( | |
| <CodeHighlighter lang="" prismLightMode={false}> | |
| {`plain code`} | |
| </CodeHighlighter>, | |
| ); | |
| // Should use full Prism with Suspense fallback | |
| await waitFor(() => { | |
| expect(container.textContent).toContain('plain code'); | |
| }); | |
| }); | |
| it('should render plain code when no lang (early return before FullPrismHighlighter)', async () => { | |
| const { container } = render( | |
| <CodeHighlighter lang="" prismLightMode={false}> | |
| {`plain code`} | |
| </CodeHighlighter>, | |
| ); | |
| // Should early return with plain <code> element (no highlighting) | |
| await waitFor(() => { | |
| expect(container.textContent).toContain('plain code'); | |
| }); | |
| // Verify it's the early return path (bare code element) | |
| expect(container.querySelector('.ant-codeHighlighter')).toBeNull(); | |
| }); |
🤖 Prompt for AI Agents
In `@packages/x/components/code-highlighter/__tests__/index.test.tsx` around lines
351 - 361, The test description and assertions are wrong because CodeHighlighter
returns early when lang=="" (see CodeHighlighter early return at lines ~95-97),
so either change the test to assert the early-return behavior (expect a bare
<code> render) or modify the test inputs to actually trigger
FullPrismHighlighter; e.g., call render(<CodeHighlighter lang="js"
prismLightMode={false}>...) or omit the lang prop/use undefined so the component
proceeds to load FullPrismHighlighter, and update the it(...) description
accordingly to match the new expectation.
Comment on lines
+388
to
+400
| describe('RTL support', () => { | ||
| it('should render with RTL direction', async () => { | ||
| // Mock XProvider context with RTL direction | ||
| const { container } = render( | ||
| <CodeHighlighter lang="javascript" data-direction="rtl"> | ||
| {`console.log("test");`} | ||
| </CodeHighlighter>, | ||
| ); | ||
| await waitFor(() => { | ||
| expect(container.querySelector('.ant-codeHighlighter')).toBeInTheDocument(); | ||
| }); | ||
| }); | ||
| }); |
Contributor
There was a problem hiding this comment.
RTL 测试未验证实际的 RTL 功能。
当前测试只传递了 data-direction="rtl" 属性（通过 restProps），但未验证组件是否应用了 RTL 样式类（如 .ant-codeHighlighter-rtl）。需要 mock XProviderContext 来设置 direction='rtl' 才能测试真正的 RTL 行为。
💚 建议的改进
describe('RTL support', () => {
- it('should render with RTL direction', async () => {
- // Mock XProvider context with RTL direction
- const { container } = render(
- <CodeHighlighter lang="javascript" data-direction="rtl">
- {`console.log("test");`}
- </CodeHighlighter>,
- );
- await waitFor(() => {
- expect(container.querySelector('.ant-codeHighlighter')).toBeInTheDocument();
- });
- });
+ // TODO: Add proper RTL test by mocking XProviderContext with direction='rtl'
+ // and verifying .ant-codeHighlighter-rtl class is applied
+ it('should render component without RTL context', async () => {
+ const { container } = render(
+ <CodeHighlighter lang="javascript">
+ {`console.log("test");`}
+ </CodeHighlighter>,
+ );
+ await waitFor(() => {
+ expect(container.querySelector('.ant-codeHighlighter')).toBeInTheDocument();
+ });
+ // Without RTL context, should not have RTL class
+ expect(container.querySelector('.ant-codeHighlighter-rtl')).toBeNull();
+ });
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| describe('RTL support', () => { | |
| it('should render with RTL direction', async () => { | |
| // Mock XProvider context with RTL direction | |
| const { container } = render( | |
| <CodeHighlighter lang="javascript" data-direction="rtl"> | |
| {`console.log("test");`} | |
| </CodeHighlighter>, | |
| ); | |
| await waitFor(() => { | |
| expect(container.querySelector('.ant-codeHighlighter')).toBeInTheDocument(); | |
| }); | |
| }); | |
| }); | |
| describe('RTL support', () => { | |
| // TODO: Add proper RTL test by mocking XProviderContext with direction='rtl' | |
| // and verifying .ant-codeHighlighter-rtl class is applied | |
| it('should render component without RTL context', async () => { | |
| const { container } = render( | |
| <CodeHighlighter lang="javascript"> | |
| {`console.log("test");`} | |
| </CodeHighlighter>, | |
| ); | |
| await waitFor(() => { | |
| expect(container.querySelector('.ant-codeHighlighter')).toBeInTheDocument(); | |
| }); | |
| // Without RTL context, should not have RTL class | |
| expect(container.querySelector('.ant-codeHighlighter-rtl')).toBeNull(); | |
| }); | |
| }); |
🤖 Prompt for AI Agents
In `@packages/x/components/code-highlighter/__tests__/index.test.tsx` around lines
388 - 400, The RTL test currently only sets data-direction on CodeHighlighter;
update the test in index.test.tsx to mock or wrap with the XProviderContext (or
XProvider) and set direction='rtl' so the component receives real context-based
RTL; then assert that the rendered element
(querySelector('.ant-codeHighlighter')) contains the RTL class (e.g.
'.ant-codeHighlighter-rtl') and/or appropriate RTL attributes to confirm
behavior; target the test block named 'should render with RTL direction' and the
CodeHighlighter component to implement this change.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub. |
Contributor
|
This comment is automatically generated by the x-markdown performance CI. |
Contributor
|
This comment is automatically generated by the x-markdown performance CI. |
1 similar comment
Contributor
|
This comment is automatically generated by the x-markdown performance CI. |
Contributor
|
This comment is automatically generated by the x-markdown performance CI. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
…loading
中文版模板 / Chinese template
🤔 This is a ...
🔗 Related Issues
💡 Background and Solution
📝 Change Log
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.