chore: upgrade alauda doom #89
Conversation
yangxiaolang
temporarily deployed
to
translate
GitHub Actions
Inactive
WalkthroughAdds UI components (Card, LinkCard), a DocLayout layout, a sidebar-menu hook, extensive CSS theming updates, several import/source reorganizations to Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (6)
theme/layout/index.tsx (1)
9-9: Consider removing explicit React import.With React 17+ and the JSX transform configured in tsconfig.json (
"jsx": "react-jsx"), the explicitimport React from "react"is unnecessary. However, if you prefer to keep it for consistency or compatibility reasons, this is a minor stylistic choice.theme/components/SidebarMenu/useSidebarMenu.tsx (1)
40-40: Consider extracting the mobile breakpoint as a constant.The hardcoded
768breakpoint appears here and might be used elsewhere in the theme. Consider extracting it to a shared constant or configuration file for maintainability.// Example: theme/constants.ts or similar export const MOBILE_BREAKPOINT = 768; // Then import and use: const isMobile = window.innerWidth <= MOBILE_BREAKPOINT;theme/layout/HomeLayout/index.tsx (1)
6-6: Consider removing explicit React import.With the JSX transform configured in tsconfig.json (
"jsx": "react-jsx"), the explicitimport React from "react"is unnecessary. However, this is a minor stylistic preference.theme/components/HomeContent/index.tsx (1)
21-21: Optional: Explicit React import may be unnecessary.Since React 17+, the explicit
import React from "react"is not required for JSX transformation. The file already imports React hooks on Line 3, which is sufficient.If you prefer to keep imports minimal, you can remove this line:
-import React from "react";theme/components/Card/index.tsx (1)
3-9: Consider clarifying the content vs. children prop usage.The Card component accepts both
contentandchildrenprops, and both will be rendered if provided (lines 28-29). While this offers flexibility, it may lead to confusion about which prop to use.Consider either:
- Documenting the intended usage pattern (e.g., "use
contentfor simple content,childrenfor complex layouts")- Or making them mutually exclusive if that's the intended behavior
If mutual exclusivity is preferred, you could apply this pattern:
export const Card: React.FC<CardProps> = ({ title, content, style, className = "", children, }) => { return ( <div className={`rp-border rp-block rp-border-solid rp-border-divider-light rp-rounded-lg rp-bg-bg rp-p-4 ${className}`} style={style} > {title && ( <div className="rp-text-lg rp-font-semibold rp-mb-3 rp-text-text-1"> {title} </div> )} - {content && <div className="rp-text-text-2">{content}</div>} - {children && <div className="rp-text-text-2">{children}</div>} + <div className="rp-text-text-2">{content || children}</div> </div> ); };Alternatively, keep the current flexible approach but add a JSDoc comment to clarify the intended usage.
Also applies to: 28-29
theme/layout/DocLayout/index.tsx (1)
47-48: Consider making sidebar/footer visibility configurable or removing dead code.Lines 47-48 hardcode
showSidebar = falseandshowDocFooter = false, which makes several code paths unreachable:
- The sidebar rendering logic (lines 70-88) will always render the placeholder
- The
DocFooter(line 116) will never be rendered- The
beforeSidebar,afterSidebar,beforeDocFooter, andafterDocFooterprops inDocLayoutPropsbecome effectively unusedWhile the Chinese comments indicate this is intentional ("强制隐藏" = force hide), consider either:
Making these configurable by adding them to
DocLayoutProps:export interface DocLayoutProps { showSidebar?: boolean; showDocFooter?: boolean; // ... other props }Or removing the dead code entirely to improve maintainability:
- Remove unused conditional branches
- Remove unused props from the interface
- Simplify the component logic
The current approach leaves dead code paths that may confuse future maintainers.
Also applies to: 70-88, 116-116
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (14)
package.json(1 hunks)styles/index.css(1 hunks)theme/components/Card/index.tsx(1 hunks)theme/components/Empty/index.tsx(1 hunks)theme/components/HomeContent/index.tsx(2 hunks)theme/components/LinkCard/index.tsx(1 hunks)theme/components/PostList/index.tsx(2 hunks)theme/components/SidebarMenu/index.ts(1 hunks)theme/components/SidebarMenu/useSidebarMenu.tsx(1 hunks)theme/index.tsx(1 hunks)theme/layout/DocLayout/index.tsx(1 hunks)theme/layout/HomeLayout/index.tsx(1 hunks)theme/layout/index.tsx(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
theme/components/Card/index.tsx (1)
theme/index.tsx (1)
Card(10-10)
theme/components/LinkCard/index.tsx (1)
theme/index.tsx (1)
LinkCard(11-11)
theme/layout/DocLayout/index.tsx (3)
theme/index.tsx (1)
DocLayout(5-5)theme/components/SidebarMenu/index.ts (1)
useSidebarMenu(1-1)theme/components/SidebarMenu/useSidebarMenu.tsx (1)
useSidebarMenu(4-124)
theme/components/PostList/index.tsx (1)
plugins/plugin-post-resolver/types.ts (1)
PostInfo(5-16)
🪛 Biome (2.1.2)
styles/index.css
[error] 23-23: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
font-weight is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (15)
theme/components/SidebarMenu/index.ts (1)
1-2: LGTM!Clean re-export pattern for the new sidebar menu hook.
theme/layout/index.tsx (1)
2-3: Verify theme-original exports are available.The imports have been switched from
@rspress/core/themeto@rspress/core/theme-original. Ensure thatBadge,LastUpdated, andLayoutare properly exported from the theme-original module in the upgraded version.theme/components/PostList/index.tsx (1)
2-3: Import updates align with PR objectives.The migration to
theme-originalfor Badge and the addition of LinkCard import from the relative path are consistent with the broader refactoring in this PR.Also applies to: 9-10
tsconfig.json (1)
13-14: LGTM! i18n alias supports improved type safety.The new path alias enables type-safe i18n imports as seen in PostList. The alias correctly points to
./docs/i18n.json, andresolveJsonModuleis already enabled in the compiler options.theme/components/SidebarMenu/useSidebarMenu.tsx (1)
50-63: Well-structured hook with clear separation of concerns.The toggle functions correctly ensure mutually exclusive visibility between sidebar and outline, and the generated menu JSX includes proper ARIA attributes for accessibility. The returned API is comprehensive and well-organized.
Also applies to: 66-113, 115-124
package.json (1)
5-5: Verify that version 1.17.1 of @alauda/doom exists.The npm registry currently shows version 1.12.7 as the latest available version of @alauda/doom. Before proceeding with this upgrade, confirm that version 1.17.1 has been released and is accessible in your configured npm registry. If it exists, review the changelog for breaking changes and verify support for the theme-original import pattern in that specific version.
theme/components/HomeContent/index.tsx (2)
2-2: LGTM: Consistent migration to theme-original.The import source change from
@rspress/core/themeto@rspress/core/theme-originalaligns with the PR's broader refactoring to use theme-original APIs.
16-16: LGTM: Using the new local Card component.The local Card import from
../Cardcorrectly integrates with the newly introduced Card component, ensuring consistency across the updated theme architecture.theme/components/LinkCard/index.tsx (1)
1-35: LGTM: Well-structured LinkCard component.The LinkCard component is cleanly implemented with:
- Proper TypeScript interface definition
- Appropriate use of semantic HTML (
<a>tag)- Flexible props for customization (title, description, style, className)
- Integration with the new RPpress v2 utility classes
The component follows React best practices and integrates well with the theme architecture.
theme/index.tsx (3)
3-3: LGTM: DocLayout properly integrated into theme exports.The DocLayout import and export expand the public theme API appropriately, making the new layout component available alongside existing layouts.
Also applies to: 5-5
7-7: LGTM: Consistent theme-original migration.The change from
@rspress/core/themeto@rspress/core/theme-originalaligns with the PR's broader migration strategy and maintains consistency across all theme files.
9-11: LGTM: Local components properly exposed.The Card and LinkCard components are correctly exported from their respective modules, making them available as part of the public theme API.
styles/index.css (2)
22-23: False positive: Lines 22-23 are not duplicate properties.The static analysis tool incorrectly flagged these lines as duplicates. They serve different purposes:
- Line 22 defines a CSS custom property (
--tw-font-weight)- Line 23 applies the actual
font-weightstyle propertyBoth are necessary and not duplicates.
57-308: LGTM: Comprehensive and well-organized CSS utilities.The RPpress v2 utility classes are well-structured with:
- Clear organization by category (layout, spacing, borders, typography, etc.)
- Consistent naming convention with
rp-prefix- Proper responsive and dark mode support
- Integration with CSS custom properties for theming
The utilities effectively support the new Card, LinkCard, and DocLayout components introduced in this PR.
theme/layout/DocLayout/index.tsx (1)
1-13: LGTM: Well-structured DocLayout implementation.The DocLayout component demonstrates good architectural patterns:
- Clear props interface with flexible before/after hooks for customization
- Proper integration with rspress theme-original components
- Effective use of React hooks (useFrontmatter, useWatchToc, useSidebarMenu)
- Responsive design with mobile menu support via useSidebarMenu
- Conditional rendering based on page type and frontmatter configuration
The component provides a solid foundation for document layout with appropriate customization points.
Also applies to: 14-46, 49-69, 89-115, 117-147
| @@ -1,4 +1,4 @@ | |||
| import { useThemeState } from "@rspress/core/theme"; | |||
| import { useThemeState } from "@rspress/core/theme-original"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find package.json to check RSPress version
find . -name "package.json" -type f | head -5 | xargs head -30
# Search for theme-original usage in codebase
echo ""
echo "=== Searching for theme-original usage ==="
rg "theme-original" --type ts --type tsx -B 2 -A 2
# Search for useThemeState usage
echo ""
echo "=== Searching for useThemeState usage ==="
rg "useThemeState" --type ts --type tsx -B 2 -A 2
# Check if the import file mentioned exists
echo ""
echo "=== Checking the specific file ==="
cat -n "theme/components/Empty/index.tsx" 2>/dev/null || echo "File not found"Repository: alauda/knowledge
Length of output: 4692
🏁 Script executed:
#!/bin/bash
# Check yarn.lock or package.json for @rspress/core version
echo "=== Checking for @rspress/core in package files ==="
rg "@rspress/core" . --type yaml --type toml --type json -A 2 | head -50
# Check if node_modules exists and search for theme-original
echo ""
echo "=== Searching node_modules for @rspress/core ==="
if [ -d "node_modules/@rspress/core" ]; then
ls -la node_modules/@rspress/core/ | head -20
echo ""
echo "=== Package.json exports from @rspress/core ==="
if [ -f "node_modules/@rspress/core/package.json" ]; then
cat node_modules/@rspress/core/package.json | grep -A 50 '"exports"'
fi
else
echo "node_modules/@rspress/core not found"
# Check what's in @alauda/doom dependencies
echo ""
echo "=== Checking @alauda/doom dependencies ==="
if [ -d "node_modules/@alauda/doom" ]; then
cat node_modules/@alauda/doom/package.json | head -50
fi
fi
# Search for theme-original more broadly
echo ""
echo "=== Searching for any theme-original reference ==="
find node_modules -name "*theme*original*" -o -name "*theme-original*" 2>/dev/null | head -10Repository: alauda/knowledge
Length of output: 294
🏁 Script executed:
#!/bin/bash
# Check for yarn.lock or package-lock.json
echo "=== Looking for lock files ==="
ls -la | grep -E "lock|yarn"
echo ""
echo "=== Checking yarn.lock for @rspress/core ==="
if [ -f "yarn.lock" ]; then
grep -A 5 "@rspress/core" yarn.lock | head -50
fi
echo ""
echo "=== Checking @alauda/doom package.json for its dependencies ==="
# Check if there's a node_modules/@alauda/doom
if [ -d "node_modules/@alauda/doom" ]; then
cat node_modules/@alauda/doom/package.json 2>/dev/null | jq '.dependencies, .devDependencies, .peerDependencies' | head -100
else
echo "node_modules not available. Checking root package.json for all deps"
cat package.json | jq '.' | head -100
fi
echo ""
echo "=== Checking if there are any RSPress references in codebase ==="
rg "rspress" -i . --type js --type ts --type tsx --type json -B 1 -A 1 | head -50Repository: alauda/knowledge
Length of output: 2529
🏁 Script executed:
#!/bin/bash
# Check yarn cache or search for @rspress/[email protected] documentation
echo "=== Checking RSPress 2.0.0-rc.2 exports ==="
# Let's look at what the yarn.lock entry says about @rspress/core
echo "Getting full @rspress/core entry from yarn.lock:"
grep -A 30 '"@rspress/core@npm:2.0.0-rc.2":' yarn.lock
# Check if there's any reference to theme-original in the entire yarn.lock
echo ""
echo "=== Searching yarn.lock for 'theme-original' ==="
grep -i "theme-original" yarn.lock
# Check for 'useThemeState' anywhere
echo ""
echo "=== Searching yarn.lock for 'useThemeState' ==="
grep -i "useThemeState" yarn.lockRepository: alauda/knowledge
Length of output: 1301
🏁 Script executed:
#!/bin/bash
# Let's search for any theme-related imports or exports in RSPress docs/examples
echo "=== Checking if there are other theme imports in project ==="
rg "from ['\"]@rspress" . -A 1 | head -30
# Look at the @alauda/doom version to understand the context better
echo ""
echo "=== Checking @alauda/doom version in yarn.lock ==="
grep -A 10 "@alauda/doom@npm:.*1.17" yarn.lockRepository: alauda/knowledge
Length of output: 2349
🏁 Script executed:
#!/bin/bash
# Let's verify what exports actually exist in @rspress/core by checking the actual RSPress source
echo "=== Checking if @rspress/core really exports theme-original ==="
# Search for any reference to "theme-original" in the entire project
echo "Files using @rspress/core/theme-original:"
rg "@rspress/core/theme-original" . --type ts --type tsx | wc -l
echo ""
echo "All imports of this path:"
rg "@rspress/core/theme-original" . --type ts --type tsxRepository: alauda/knowledge
Length of output: 267
🏁 Script executed:
#!/bin/bash
# Check without file type filtering
echo "=== Finding all theme-original references ==="
grep -r "theme-original" . --include="*.ts" --include="*.tsx" 2>/dev/null
echo ""
echo "=== Check if there's a theme-original entry point or custom export ==="
# Check the theme directory structure
find theme -type f -name "*.ts" -o -name "*.tsx" 2>/dev/null | head -20
echo ""
echo "=== Check theme/index.tsx to see what's exported ==="
cat theme/index.tsxRepository: alauda/knowledge
Length of output: 1769
🏁 Script executed:
#!/bin/bash
# Check if @alauda/doom provides theme-original export
echo "=== Checking @alauda/doom's yarn.lock entry more thoroughly ==="
grep -B 5 -A 50 '"@alauda/doom@npm:1.17.1":' yarn.lock | head -100
echo ""
echo "=== Look for any @rspress references in @alauda/doom dependencies ==="
grep -A 50 '"@alauda/doom@npm:1.17.1":' yarn.lock | grep -i "rspress"Repository: alauda/knowledge
Length of output: 194
🏁 Script executed:
#!/bin/bash
# Let's search the yarn.lock for @alauda/doom differently
echo "=== Searching for @alauda/doom in yarn.lock ==="
grep "@alauda/doom" yarn.lock | head -20
echo ""
echo "=== Get the full entry for @alauda/doom ==="
awk '/"@alauda\/doom@npm:1\.17\.1":/,/^"[^@]/' yarn.lock | head -100
echo ""
echo "=== Search entire yarn.lock for rspress in dependencies ==="
grep -B 10 "rspress" yarn.lock | grep -E "^\"|rspress"Repository: alauda/knowledge
Length of output: 3102
🏁 Script executed:
#!/bin/bash
# Check if there's a TypeScript path alias or other configuration mapping theme-original
echo "=== Checking tsconfig.json for path aliases ==="
cat tsconfig.json 2>/dev/null | grep -A 20 "paths"
echo ""
echo "=== Checking rspress.config.ts for any alias configuration ==="
find . -name "rspress.config.ts" -o -name "rspress.config.js" | xargs cat 2>/dev/null
echo ""
echo "=== Check if theme-original might be re-exported from theme/index.tsx ==="
# We already know theme/index.tsx exports from theme-original, but let's confirm what it should be
cat theme/index.tsxRepository: alauda/knowledge
Length of output: 707
🏁 Script executed:
#!/bin/bash
# Final verification - check what actually exists in @rspress/core exports
echo "=== Checking @rspress/[email protected] package structure from yarn.lock ==="
awk '/"@rspress\/core@npm:2\.0\.0-rc\.2":/,/^"/' yarn.lock | grep -E "exports|bin|main|types" | head -20
echo ""
echo "=== Summary of theme-original usage in codebase ==="
echo "Files importing from @rspress/core/theme-original:"
grep -r "@rspress/core/theme-original" . --include="*.ts" --include="*.tsx" 2>/dev/null | wc -l
echo ""
echo "Functions imported from this path:"
grep -r "from ['\"]@rspress/core/theme-original['\"]" . --include="*.ts" --include="*.tsx" -A 0 2>/dev/null | sed 's/.*from.*//' | sort -uRepository: alauda/knowledge
Length of output: 280
🏁 Script executed:
#!/bin/bash
# Let's extract the exact imports from each file to understand what's being used
echo "=== Detailed import analysis ==="
grep -B 1 "from ['\"]@rspress/core/theme-original['\"]" . -r --include="*.ts" --include="*.tsx" 2>/dev/null
echo ""
echo "=== Check what components are in standard @rspress/core/theme ==="
# Look at how other @rspress/core/runtime imports work in the project for reference
grep -r "from ['\"]@rspress/core" . --include="*.ts" --include="*.tsx" 2>/dev/null | grep -o "from ['\"]@rspress/core[^'\"]*['\"]" | sort -uRepository: alauda/knowledge
Length of output: 1199
Replace non-existent @rspress/core/theme-original imports with @rspress/core/theme.
In RSPress v2.0, the package migration is rspress/theme => @rspress/core/theme. The import path @rspress/core/theme-original does not exist in @rspress/[email protected] (the version declared in yarn.lock). Update all imports in the following files:
theme/components/Empty/index.tsx(useThemeState)theme/components/HomeContent/index.tsx(useFullTextSearch)theme/components/PostList/index.tsx(Badge)theme/layout/DocLayout/index.tsx(multiple exports)theme/layout/index.tsx(Badge, LastUpdated, Layout)theme/index.tsx(wildcard export)
Change from @rspress/core/theme-original to @rspress/core/theme.
🤖 Prompt for AI Agents
theme/components/Empty/index.tsx (line 1) and the other files listed:
theme/components/HomeContent/index.tsx, theme/components/PostList/index.tsx,
theme/layout/DocLayout/index.tsx, theme/layout/index.tsx, theme/index.tsx —
replace any import source string "@rspress/core/theme-original" with
"@rspress/core/theme"; keep the same named imports (useThemeState,
useFullTextSearch, Badge, LastUpdated, Layout, and any wildcard exports) and
ensure import syntax remains valid for TypeScript/ES modules.
| export const PostList: FC<PostListProps> = ({ postList }) => { | ||
| const notEmpty = useMemo(() => postList.length > 0, [postList]); | ||
| const t = useI18n(); | ||
| const t = useI18n<typeof import("i18n")>(); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
echo "=== Checking docs/i18n.json ==="
if [ -f "docs/i18n.json" ]; then
echo "✓ docs/i18n.json exists"
echo ""
echo "=== File content preview ==="
head -50 docs/i18n.json
echo ""
echo "=== Checking for lastUpdatedText key ==="
if grep -q "lastUpdatedText" docs/i18n.json; then
echo "✓ lastUpdatedText key found"
grep "lastUpdatedText" docs/i18n.json | head -5
else
echo "✗ lastUpdatedText key NOT found"
fi
else
echo "✗ docs/i18n.json NOT found"
fi
echo ""
echo "=== Checking PostList component ==="
if [ -f "theme/components/PostList/index.tsx" ]; then
echo "✓ PostList/index.tsx exists"
echo ""
echo "=== Lines 15-25 (context around line 18) ==="
sed -n '15,25p' theme/components/PostList/index.tsx
echo ""
echo "=== Lines 48-57 (context around lines 52-53) ==="
sed -n '48,57p' theme/components/PostList/index.tsx
else
echo "✗ PostList/index.tsx NOT found"
fiRepository: alauda/knowledge
Length of output: 1913
Add lastUpdatedText key to docs/i18n.json with translations for all supported locales.
The code at lines 52-53 calls t("lastUpdatedText"), but this key is missing from docs/i18n.json. Add the translation key in the same bilingual format as existing entries (with both "en" and "zh" values).
🤖 Prompt for AI Agents
In theme/components/PostList/index.tsx around line 18 and where
t("lastUpdatedText") is called (lines ~52-53), the i18n key "lastUpdatedText" is
referenced but missing from docs/i18n.json; add a new top-level
"lastUpdatedText" entry in docs/i18n.json with both "en" and "zh" values (and
any other supported locales) following the existing bilingual format used in the
file so t("lastUpdatedText") resolves for all locales.
| useEffect(() => { | ||
| const handleClickOutside = (event: MouseEvent) => { | ||
| const target = event.target as HTMLElement; | ||
|
|
||
| // 如果点击的是菜单按钮，不关闭 | ||
| if (target.closest('.rp-doc-layout__menu-button')) { | ||
| return; | ||
| } | ||
|
|
||
| // 如果点击在侧边栏外部，关闭侧边栏 | ||
| if ( | ||
| sidebarLayoutRef.current && | ||
| !sidebarLayoutRef.current.contains(target) && | ||
| !target.closest('.rp-doc-layout__menu') | ||
| ) { | ||
| setIsSidebarOpen(false); | ||
| } | ||
|
|
||
| // 如果点击在大纲外部，关闭大纲 | ||
| if ( | ||
| asideLayoutRef.current && | ||
| !asideLayoutRef.current.contains(target) && | ||
| !target.closest('.rp-doc-layout__menu') | ||
| ) { | ||
| setIsOutlineOpen(false); | ||
| } | ||
| }; | ||
|
|
||
| // 只在移动端添加点击外部关闭逻辑 | ||
| const isMobile = window.innerWidth <= 768; | ||
| if (isMobile) { | ||
| document.addEventListener('click', handleClickOutside); | ||
| return () => { | ||
| document.removeEventListener('click', handleClickOutside); | ||
| }; | ||
| } | ||
| }, []); |
There was a problem hiding this comment.
Mobile detection doesn't respond to window resize.
The mobile check at Line 40 (window.innerWidth <= 768) only executes once when the component mounts. If a user resizes their browser window from desktop to mobile (or vice versa), the click-outside listener won't be added or removed accordingly, which could lead to inconsistent behavior.
Consider refactoring to handle window resize events:
useEffect(() => {
const handleClickOutside = (event: MouseEvent) => {
const target = event.target as HTMLElement;
// 如果点击的是菜单按钮，不关闭
if (target.closest('.rp-doc-layout__menu-button')) {
return;
}
// 如果点击在侧边栏外部，关闭侧边栏
if (
sidebarLayoutRef.current &&
!sidebarLayoutRef.current.contains(target) &&
!target.closest('.rp-doc-layout__menu')
) {
setIsSidebarOpen(false);
}
// 如果点击在大纲外部，关闭大纲
if (
asideLayoutRef.current &&
!asideLayoutRef.current.contains(target) &&
!target.closest('.rp-doc-layout__menu')
) {
setIsOutlineOpen(false);
}
};
- // 只在移动端添加点击外部关闭逻辑
- const isMobile = window.innerWidth <= 768;
- if (isMobile) {
- document.addEventListener('click', handleClickOutside);
- return () => {
- document.removeEventListener('click', handleClickOutside);
- };
- }
+ // 只在移动端添加点击外部关闭逻辑
+ const checkMobile = () => window.innerWidth <= 768;
+
+ const updateListener = () => {
+ const isMobile = checkMobile();
+ if (isMobile) {
+ document.addEventListener('click', handleClickOutside);
+ } else {
+ document.removeEventListener('click', handleClickOutside);
+ }
+ };
+
+ updateListener();
+ window.addEventListener('resize', updateListener);
+
+ return () => {
+ document.removeEventListener('click', handleClickOutside);
+ window.removeEventListener('resize', updateListener);
+ };
}, []);📝 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.
| useEffect(() => { | |
| const handleClickOutside = (event: MouseEvent) => { | |
| const target = event.target as HTMLElement; | |
| // 如果点击的是菜单按钮，不关闭 | |
| if (target.closest('.rp-doc-layout__menu-button')) { | |
| return; | |
| } | |
| // 如果点击在侧边栏外部，关闭侧边栏 | |
| if ( | |
| sidebarLayoutRef.current && | |
| !sidebarLayoutRef.current.contains(target) && | |
| !target.closest('.rp-doc-layout__menu') | |
| ) { | |
| setIsSidebarOpen(false); | |
| } | |
| // 如果点击在大纲外部，关闭大纲 | |
| if ( | |
| asideLayoutRef.current && | |
| !asideLayoutRef.current.contains(target) && | |
| !target.closest('.rp-doc-layout__menu') | |
| ) { | |
| setIsOutlineOpen(false); | |
| } | |
| }; | |
| // 只在移动端添加点击外部关闭逻辑 | |
| const isMobile = window.innerWidth <= 768; | |
| if (isMobile) { | |
| document.addEventListener('click', handleClickOutside); | |
| return () => { | |
| document.removeEventListener('click', handleClickOutside); | |
| }; | |
| } | |
| }, []); | |
| useEffect(() => { | |
| const handleClickOutside = (event: MouseEvent) => { | |
| const target = event.target as HTMLElement; | |
| // 如果点击的是菜单按钮，不关闭 | |
| if (target.closest('.rp-doc-layout__menu-button')) { | |
| return; | |
| } | |
| // 如果点击在侧边栏外部，关闭侧边栏 | |
| if ( | |
| sidebarLayoutRef.current && | |
| !sidebarLayoutRef.current.contains(target) && | |
| !target.closest('.rp-doc-layout__menu') | |
| ) { | |
| setIsSidebarOpen(false); | |
| } | |
| // 如果点击在大纲外部，关闭大纲 | |
| if ( | |
| asideLayoutRef.current && | |
| !asideLayoutRef.current.contains(target) && | |
| !target.closest('.rp-doc-layout__menu') | |
| ) { | |
| setIsOutlineOpen(false); | |
| } | |
| }; | |
| // 只在移动端添加点击外部关闭逻辑 | |
| const checkMobile = () => window.innerWidth <= 768; | |
| const updateListener = () => { | |
| const isMobile = checkMobile(); | |
| if (isMobile) { | |
| document.addEventListener('click', handleClickOutside); | |
| } else { | |
| document.removeEventListener('click', handleClickOutside); | |
| } | |
| }; | |
| updateListener(); | |
| window.addEventListener('resize', updateListener); | |
| return () => { | |
| document.removeEventListener('click', handleClickOutside); | |
| window.removeEventListener('resize', updateListener); | |
| }; | |
| }, []); |
🤖 Prompt for AI Agents
In theme/components/SidebarMenu/useSidebarMenu.tsx around lines 11 to 47, the
current mobile check (window.innerWidth <= 768) runs only on mount so the
click-outside listener is never added or removed when the user resizes the
window; change the effect to track viewport breakpoint changes by adding a
resize listener that updates an isMobile flag (or state/ref) and attach/detach
the document click handler whenever the breakpoint crosses the mobile threshold
(use a small debounce on resize), and ensure all listeners are cleaned up on
unmount to prevent leaks.
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
theme/components/PostList/index.tsx (1)
49-53: Missing i18n key already flagged in previous review.The missing
lastUpdatedTexttranslation key has already been identified in a previous review. Please ensure this key is added todocs/i18n.jsonwith translations for all supported locales.
🧹 Nitpick comments (3)
theme/components/SidebarMenu/useSidebarMenu.tsx (2)
40-40: Address or clarify the TODO comment.The TODO comment lacks context about what needs discussion. Consider either resolving the issue or adding more detail about what requires discussion.
Do you want me to help identify what might need discussion here, or would you like to open an issue to track this?
19-24: Consider memoizing the callback and JSX for better performance.The
toggleOutlinefunction andsidebarMenuJSX are recreated on every render, which could cause unnecessary re-renders in consuming components.Apply these optimizations:
-import { useRef, useState } from "react"; +import { useRef, useState, useCallback, useMemo } from "react"; import { useI18n } from "@rspress/core/runtime"; import { useDynamicToc, ReadPercent, SvgWrapper, IconArrowRight, } from "@rspress/core/theme-original"; export function useSidebarMenu() { const [isSidebarOpen, setIsSidebarOpen] = useState(false); const [isOutlineOpen, setIsOutlineOpen] = useState(false); const sidebarLayoutRef = useRef<HTMLElement>(null); const asideLayoutRef = useRef<HTMLElement>(null); const headers = useDynamicToc(); const t = useI18n(); - const toggleOutline = () => { + const toggleOutline = useCallback(() => { setIsOutlineOpen((prev) => !prev); if (isSidebarOpen) { setIsSidebarOpen(false); } - }; + }, [isSidebarOpen]); - const sidebarMenu = ( + const sidebarMenu = useMemo(() => ( <div className="rp-doc-layout__menu"> <button type="button" disabled={headers.length === 0} onClick={(e) => { e.preventDefault(); e.stopPropagation(); toggleOutline(); }} className="rp-sidebar-menu__right" > <span>{t("outlineTitle")}</span> <ReadPercent size={14} strokeWidth={2} /> {/* TODO: discussion */} {headers.length !== 0 && ( <SvgWrapper icon={IconArrowRight} style={{ transform: isOutlineOpen ? "rotate(90deg)" : "rotate(0deg)", transition: "transform 0.2s ease-out", }} /> )} </button> </div> - ); + ), [headers.length, isOutlineOpen, toggleOutline, t]); return { isSidebarOpen, isOutlineOpen, sidebarMenu, asideLayoutRef, sidebarLayoutRef, toggleOutline, }; }Also applies to: 26-52
theme/layout/HomeLayout/index.tsx (1)
6-6: Consider removing the unused React import.With
"jsx": "react-jsx"configured intsconfig.json, the explicitReactimport is unnecessary. The JSX transform handles this automatically.Apply this diff to remove the unused import:
-import React from "react";
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
styles/index.css(1 hunks)theme/components/BreadCrumb/index.tsx(2 hunks)theme/components/Card/index.tsx(1 hunks)theme/components/Checkbox/index.tsx(0 hunks)theme/components/DocID/index.tsx(1 hunks)theme/components/EditOnGithub/index.tsx(1 hunks)theme/components/HomeBanner/index.tsx(2 hunks)theme/components/HomeContent/index.tsx(3 hunks)theme/components/LinkCard/index.tsx(1 hunks)theme/components/Pagination/index.tsx(1 hunks)theme/components/PostList/index.tsx(2 hunks)theme/components/SidebarMenu/useSidebarMenu.tsx(1 hunks)theme/layout/HomeLayout/index.tsx(2 hunks)theme/layout/index.tsx(1 hunks)tsconfig.json(1 hunks)virtual-modules.d.ts(1 hunks)
💤 Files with no reviewable changes (1)
- theme/components/Checkbox/index.tsx
🚧 Files skipped from review as they are similar to previous changes (3)
- theme/components/LinkCard/index.tsx
- theme/layout/index.tsx
- theme/components/HomeContent/index.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
theme/components/Card/index.tsx (1)
theme/index.tsx (1)
Card(10-10)
theme/components/PostList/index.tsx (2)
virtual-modules.d.ts (1)
PostInfo(3-14)plugins/plugin-post-resolver/types.ts (1)
PostInfo(5-16)
🪛 Biome (2.1.2)
styles/index.css
[error] 23-23: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
font-weight is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🔇 Additional comments (18)
theme/components/Pagination/index.tsx (1)
18-18: LGTM: Type safety improvement for i18n.The explicit generic type parameter aligns with the new
"i18n"path alias intsconfig.jsonand provides better type inference for translation keys.theme/components/EditOnGithub/index.tsx (1)
46-46: LGTM: Consistent i18n typing pattern.The typing update is consistent with other components in this PR and improves type safety.
theme/components/HomeBanner/index.tsx (1)
12-12: LGTM: Consistent i18n typing.The explicit type parameter follows the same pattern as other components in this PR.
tsconfig.json (1)
12-22: LGTM: TypeScript configuration supports the i18n typing pattern.The addition of
baseUrland the"i18n"path alias correctly supports the typeduseI18n<typeof import("i18n")>()pattern used throughout the codebase. The expandedincludepatterns appropriately cover the theme and plugin directories.theme/layout/HomeLayout/index.tsx (1)
24-28: LGTM: Footer styling aligns with CSS theming updates.The simplified class names (
home-footer,home-footer__content) align with the token-based theming approach introduced instyles/index.css.virtual-modules.d.ts (1)
1-26: LGTM: Well-structured type declarations for virtual modules.The type declarations properly expose
PostInfoand related exports for virtual modules. ThePostInfointerface is consistent withplugins/plugin-post-resolver/types.ts.theme/components/PostList/index.tsx (2)
1-9: LGTM: Import reorganization aligns with theme refactoring.The import changes correctly source
Badgefromtheme-originaland add the necessaryLinkCarddependency, consistent with the broader theme reorganization in this PR.
17-17: LGTM: Consistent i18n typing pattern.theme/components/Card/index.tsx (1)
3-9: LGTM: Well-defined CardProps interface.The interface provides a flexible API with optional properties for various use cases.
styles/index.css (6)
57-74: LGTM: Card component styling.The card component styling is well-structured with consistent use of CSS variables for theming. The separation of card, card__title, and card__content provides good modularity.
76-103: LGTM: Link card component styling.The link-card component is well-implemented with smooth hover transitions and consistent theming. The use of
border-bottominstead of a full border is appropriate for list-style layouts.
105-117: LGTM: Post-meta styling aligns with component refactor.The post-meta styling correctly replaces the utility classes previously used in the DocID component, providing centralized and maintainable styling with responsive behavior.
119-121: LGTM: Breadcrumb styling aligns with component refactor.The breadcrumb styling appropriately replaces the
rp-mb-10utility class previously used in the BreadCrumb component.
123-146: LGTM: Home footer styling.The home-footer component styling is well-implemented with proper responsive behavior and consistent theming through CSS variables.
148-194: LGTM: Responsive document layout menu.The rp-doc-layout__menu styling is properly implemented with:
- Appropriate fixed positioning and z-index
- Responsive display behavior (visible only below 1280px)
- Well-defined interactive states for the menu button
theme/components/BreadCrumb/index.tsx (2)
7-7: LGTM: Improved type safety with typed i18n hook.Adding the type parameter to
useI18nprovides better type checking and IDE support, aligning with the PR-wide typing improvements.
17-17: LGTM: Semantic class name aligns with CSS refactor.The className change from
rp-mb-10tobreadcrumbaligns with the new centralized styling instyles/index.cssand follows semantic naming conventions.theme/components/DocID/index.tsx (1)
11-11: LGTM: Consolidates utility classes into semantic class.The className change from multiple utility classes to the single
post-metaclass improves maintainability while preserving the visual styling. The new class instyles/index.cssprovides equivalent responsive behavior.
| .paginationItem { | ||
| font-weight: 500; | ||
| cursor: pointer; | ||
| font-size: var(--text-sm); | ||
| line-height: var(--tw-leading, var(--text-sm--line-height)); | ||
|
|
||
| --tw-font-weight: var(--font-weight-medium) /* 500 */; | ||
| font-weight: var(--font-weight-medium) /* 500 */; | ||
|
|
||
| margin-inline: calc(var(--spacing) * 1.5) /* 0.375rem = 6px */; | ||
| padding-inline: calc(var(--spacing) * 3) /* 0.75rem = 12px */; | ||
| padding-block: calc(var(--spacing) * 2) /* 0.5rem = 8px */; | ||
| display: flex; | ||
| align-items: center; | ||
| } |
There was a problem hiding this comment.
Remove duplicate font-weight declaration.
The font-weight property is declared twice (line 17 and line 23), causing the second declaration to override the first. Since line 23 uses the CSS variable var(--font-weight-medium) which evaluates to 500, line 17 is redundant.
Apply this diff to remove the duplicate:
.paginationItem {
- font-weight: 500;
cursor: pointer;
font-size: var(--text-sm);
line-height: var(--tw-leading, var(--text-sm--line-height));
--tw-font-weight: var(--font-weight-medium) /* 500 */;
font-weight: var(--font-weight-medium) /* 500 */;🧰 Tools
🪛 Biome (2.1.2)
[error] 23-23: Duplicate properties can lead to unexpected behavior and may override previous declarations unintentionally.
font-weight is already defined here.
Remove or rename the duplicate property to ensure consistent styling.
(lint/suspicious/noDuplicateProperties)
🤖 Prompt for AI Agents
In styles/index.css around lines 16 to 30, there is a duplicate font-weight
declaration (plain 500 at line 17 and variable-based at line 23); remove the
redundant plain "font-weight: 500;" (the earlier declaration) so only the
variable-based "--tw-font-weight: var(--font-weight-medium)" and "font-weight:
var(--font-weight-medium)" remain, ensuring no duplicate overrides.
| <div className={`card ${className}`} style={style}> | ||
| {title && <div className="card__title">{title}</div>} | ||
| {content && <div className="card__content">{content}</div>} | ||
| {children && <div className="card__content">{children}</div>} | ||
| </div> |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Clarify the intended usage of content vs children.
Both content and children props render to separate <div className="card__content"> elements when provided simultaneously (lines 21-22). This could lead to unexpected styling or layout issues.
Consider one of the following approaches:
- Make
contentandchildrenmutually exclusive (recommended) - Render them in a single container
- Document the intended behavior if this is by design
Option 1 (recommended): Mutually exclusive usage
export const Card: React.FC<CardProps> = ({
title,
content,
style,
className = "",
children,
}) => {
return (
<div className={`card ${className}`} style={style}>
{title && <div className="card__title">{title}</div>}
- {content && <div className="card__content">{content}</div>}
- {children && <div className="card__content">{children}</div>}
+ {(content || children) && (
+ <div className="card__content">{content || children}</div>
+ )}
</div>
);
};Option 2: Single container for both
- {content && <div className="card__content">{content}</div>}
- {children && <div className="card__content">{children}</div>}
+ {(content || children) && (
+ <div className="card__content">
+ {content}
+ {children}
+ </div>
+ )}📝 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.
| <div className={`card ${className}`} style={style}> | |
| {title && <div className="card__title">{title}</div>} | |
| {content && <div className="card__content">{content}</div>} | |
| {children && <div className="card__content">{children}</div>} | |
| </div> | |
| <div className={`card ${className}`} style={style}> | |
| {title && <div className="card__title">{title}</div>} | |
| {(content || children) && ( | |
| <div className="card__content">{content || children}</div> | |
| )} | |
| </div> |
| <div className={`card ${className}`} style={style}> | |
| {title && <div className="card__title">{title}</div>} | |
| {content && <div className="card__content">{content}</div>} | |
| {children && <div className="card__content">{children}</div>} | |
| </div> | |
| <div className={`card ${className}`} style={style}> | |
| {title && <div className="card__title">{title}</div>} | |
| {(content || children) && ( | |
| <div className="card__content"> | |
| {content} | |
| {children} | |
| </div> | |
| )} | |
| </div> |
🤖 Prompt for AI Agents
theme/components/Card/index.tsx lines 19-23: the component currently renders
both `content` and `children` into separate <div className="card__content">
blocks which can cause styling/layout surprises; update the component to make
`content` and `children` mutually exclusive by validating props at the top of
the function (throw or console.error and return) when both are provided, update
the render to choose one source (prefer `children` over `content` or vice versa
per design) and update the prop types/TS definitions and component docs/tests to
reflect the exclusive contract.
| @@ -1,4 +1,4 @@ | |||
| import { useI18n, usePageData } from "@rspress/runtime"; | |||
| import { useI18n } from "@rspress/runtime"; | |||
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check which files use which import paths for useI18n
echo "=== Files using @rspress/core/runtime ==="
rg -n "from ['\"]@rspress/core/runtime['\"]" --type=tsx --type=ts 2>/dev/null || echo "No matches found"
echo ""
echo "=== Files using @rspress/runtime ==="
rg -n "from ['\"]@rspress/runtime['\"]" --type=tsx --type=ts 2>/dev/null || echo "No matches found"
echo ""
echo "=== All @rspress imports ==="
rg -n "from ['\"]@rspress" --type=tsx --type=ts | head -30
echo ""
echo "=== Check package.json for @rspress dependencies ==="
if [ -f "package.json" ]; then
jq '.dependencies, .devDependencies' package.json 2>/dev/null | grep -i rspress || echo "No rspress deps found"
fiRepository: alauda/knowledge
Length of output: 324
🏁 Script executed:
#!/bin/bash
# Search more broadly for the file and imports
echo "=== Looking for HomeBanner file ==="
find . -name "HomeBanner" -o -name "index.tsx" | grep -i homebanner
echo ""
echo "=== Looking for any @rspress imports (no type filter) ==="
rg "@rspress" | head -20
echo ""
echo "=== Check if the file exists ==="
[ -f "theme/components/HomeBanner/index.tsx" ] && echo "File exists" || echo "File does not exist"
echo ""
echo "=== List files in theme/components directory ==="
ls -la theme/components/ 2>/dev/null || echo "Directory does not exist"
echo ""
echo "=== Check package.json content ==="
cat package.json 2>/dev/null | head -50Repository: alauda/knowledge
Length of output: 2982
🏁 Script executed:
#!/bin/bash
# Check the HomeBanner file content
echo "=== Content of theme/components/HomeBanner/index.tsx ==="
cat -n theme/components/HomeBanner/index.tsx
echo ""
echo "=== Search for useI18n imports in codebase ==="
rg "useI18n" --max-count=50
echo ""
echo "=== Check what @rspress exports ==="
rg "import.*from.*@rspress" | head -30Repository: alauda/knowledge
Length of output: 4277
Change import path to match codebase convention.
The import uses @rspress/runtime while all other components in this project (BreadCrumb, SidebarMenu, PostList, HomeContent, Pagination, EditOnGithub) import from @rspress/core/runtime. Update to import { useI18n } from "@rspress/core/runtime"; for consistency.
🤖 Prompt for AI Agents
In theme/components/HomeBanner/index.tsx at line 1 the import path uses
"@rspress/runtime" which is inconsistent with the project's convention; change
the import to "@rspress/core/runtime" so it reads import { useI18n } from
"@rspress/core/runtime"; to match other components and maintain consistent
module resolution.
Summary by CodeRabbit
New Features
Style / Theming
Chores
✏️ Tip: You can customize this high-level summary in your review settings.