chore: upgrade alauda doom #89
Conversation
yangxiaolang
temporarily deployed
to
translate
GitHub Actions
Inactive
WalkthroughAdds Card and LinkCard components, a DocLayout and sidebar hook, large CSS/layout additions, download utilities with MDX anchor download gating, TypeScript path/include updates, multiple import/source reorganizations to Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant User as User (click)
participant MDX as Layout / MDX anchor handler
participant Util as download.ts
participant Net as Network (fetch / browser)
note right of MDX: Anchor click intercepted\nby MDX anchor handler
User->>MDX: click anchor (href)
MDX->>Util: getPathname(href)
MDX->>Util: shouldDownload(pathname)?
alt shouldDownload == false
MDX->>User: allow normal navigation
else shouldDownload == true
MDX->>Util: downloadFile(href, filename)
alt same-origin
Util->>Net: fetch(normalized URL)
Net-->>Util: Response (ok/blob)
Util->>User: create blob URL & programmatic click (download)
Util->>Net: revoke blob URL (cleanup)
else cross-origin
Util->>User: create temp anchor href & programmatic click (download)
Util->>User: cleanup temp anchor
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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/core@2.0.0-rc.2 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/core@2.0.0-rc.2 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/core@2.0.0-rc.2 (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.
| @@ -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.
yangxiaolang
force-pushed
the
chore/upgrade-alauda-doom
branch
from
efca7a3 to
9b9f20d
Compare
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
theme/components/HomeContent/index.tsx (1)
126-130: MissingonSearchParamsChangein dependency array.The
onPageChangecallback usesonSearchParamsChangebut has an empty dependency array, which may cause stale closure issues.🔎 Proposed fix
const onPageChange = useCallback( (number: number) => onSearchParamsChange(new URLSearchParams({ page: `${number}` })), - [] + [onSearchParamsChange] );
♻️ Duplicate comments (4)
theme/components/HomeBanner/index.tsx (1)
1-1: Unresolved: Import path inconsistency remains.This import path inconsistency was flagged in a previous review and has not been addressed. All other components in the project import from
@rspress/core/runtime, but this file imports from@rspress/runtime. Update to maintain consistency across the codebase.🔎 Proposed fix
-import { useI18n } from "@rspress/runtime"; +import { useI18n } from "@rspress/core/runtime";theme/components/Card/index.tsx (1)
3-9: Unresolved: Ambiguouscontentvschildrenprop usage.This design issue was flagged in a previous review and remains unaddressed. The component accepts both
contentandchildrenprops and renders them into separate<div className="card__content">containers (lines 21-22), which creates confusing behavior and potential styling conflicts when both are provided.Make
contentandchildrenmutually exclusive to clarify the component's API.🔎 Recommended fix (Option 1: Mutually exclusive)
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> ); };Also applies to: 21-22
styles/index.css (1)
16-30: Remove duplicatefont-weightdeclaration.Line 17 declares
font-weight: 500and line 23 declares it again with the CSS variable. The second declaration overrides the first, making line 17 redundant.theme/components/PostList/index.tsx (1)
49-53: EnsurelastUpdatedTextkey exists in i18n configuration.The code uses
t("lastUpdatedText")but a previous review flagged that this key may be missing from the i18n configuration.
🧹 Nitpick comments (2)
theme/layout/HomeLayout/index.tsx (1)
6-6: Consider removing the redundant React import.With React 17+ and the new JSX transform, explicitly importing React is no longer necessary for JSX usage. This import can be safely removed.
🔎 Proposed cleanup
import { usePageData } from "@rspress/runtime"; import { ReactNode } from "react"; import { HomeBanner } from "../../components/HomeBanner"; import { HomeContent } from "../../components/HomeContent"; -import React from "react";theme/components/HomeContent/index.tsx (1)
21-21: Consider moving React import to the top of the file.The React import is placed after other imports. Conventionally, React imports should be at the top, before other dependencies.
🔎 Suggested reorder
+import React from "react"; import { useI18n, usePageData } from "@rspress/core/runtime"; import { useFullTextSearch } from "@rspress/core/theme-original"; -import { useCallback, useEffect, useMemo, useState } from "react"; +import { useCallback, useEffect, useMemo, useState } from "react"; import { PostInfo, postInfos } from "virtual-post-data"; ... -import React from "react";
📜 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 (22)
package.jsonstyles/index.csstheme/components/BreadCrumb/index.tsxtheme/components/Card/index.tsxtheme/components/Checkbox/index.tsxtheme/components/DocID/index.tsxtheme/components/EditOnGithub/index.tsxtheme/components/Empty/index.tsxtheme/components/HomeBanner/index.tsxtheme/components/HomeContent/index.tsxtheme/components/LinkCard/index.tsxtheme/components/Pagination/index.tsxtheme/components/PostList/index.tsxtheme/components/SidebarMenu/index.tstheme/components/SidebarMenu/useSidebarMenu.tsxtheme/index.tsxtheme/layout/DocLayout/index.tsxtheme/layout/HomeLayout/index.tsxtheme/layout/index.tsxtheme/utils/download.tstsconfig.jsonvirtual-modules.d.ts
💤 Files with no reviewable changes (1)
- theme/components/Checkbox/index.tsx
🚧 Files skipped from review as they are similar to previous changes (9)
- theme/components/LinkCard/index.tsx
- theme/components/BreadCrumb/index.tsx
- tsconfig.json
- theme/layout/DocLayout/index.tsx
- theme/components/SidebarMenu/useSidebarMenu.tsx
- theme/components/Pagination/index.tsx
- theme/components/Empty/index.tsx
- theme/components/SidebarMenu/index.ts
- theme/components/EditOnGithub/index.tsx
🧰 Additional context used
🧬 Code graph analysis (2)
theme/components/Card/index.tsx (1)
theme/index.tsx (1)
Card(10-10)
theme/layout/index.tsx (1)
theme/utils/download.ts (3)
getPathname(16-22)shouldDownload(11-14)downloadFile(24-57)
🪛 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 (10)
theme/components/DocID/index.tsx (1)
11-11: LGTM! Semantic className improves maintainability.The refactoring from utility classes to the semantic
post-metaclassName aligns with the broader styling standardization in this PR and improves code maintainability.theme/components/HomeBanner/index.tsx (1)
12-12: LGTM! Type parameter improves type safety.Adding the type parameter to
useI18nstrengthens type checking and improves developer experience with better autocomplete.theme/layout/HomeLayout/index.tsx (1)
24-28: LGTM! Footer refactoring improves maintainability.The simplified footer structure with semantic class names (
home-footer,home-footer__content) aligns well with the styling standardization throughout this PR.virtual-modules.d.ts (1)
1-25: LGTM!The virtual module type declarations are well-structured with proper exports for
PostInfointerface and the related constants. The typing provides good type safety for consuming components.styles/index.css (2)
57-103: LGTM!The
.cardand.link-cardcomponent styles are well-implemented with consistent use of CSS variables for theming, proper transitions, and hover states that align with the new Card and LinkCard components.
148-194: LGTM!The responsive menu styles are well-structured with appropriate breakpoint handling, consistent theming via CSS variables, and proper interactive states (hover, active).
theme/components/HomeContent/index.tsx (1)
44-72: Dependency array may be incomplete forsearchFull.The
searchFullcallback referencesonKeywordChangeandsearchbut they are not included in the dependency array. This could lead to stale closures if these functions change.const searchFull = useCallback( async (keyword: string) => { // ... }, - [initialized, siteData.base, page.lang] + [initialized, siteData.base, page.lang, onKeywordChange, search] );theme/components/PostList/index.tsx (1)
1-9: LGTM!The import reorganization aligns with the PR's migration to
@rspress/core/theme-originaland properly uses the localLinkCardcomponent.theme/index.tsx (1)
1-11: LGTM!The export structure correctly exposes
DocLayout,Card, andLinkCardfrom local implementations, with the wildcard re-export from@rspress/core/theme-originalproviding base theme exports. Local named exports after the wildcard correctly shadow any conflicting names from the re-export.theme/utils/download.ts (1)
48-56: Cross-origin downloads may silently fail.For cross-origin URLs, the
downloadattribute is often ignored by browsers due to security restrictions, causing a navigation instead of a download. Consider adding a note or warning to users, or opening in a new tab.
package.json
Outdated
| "packageManager": "yarn@4.12.0", | ||
| "devDependencies": { | ||
| "@alauda/doom": "^1.12.7", | ||
| "@alauda/doom": "^1.17.1", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@alauda/doom npm package breaking changes 1.12.7 to 1.17.1
💡 Result:
I can't find an authoritative changelog for @alauda/doom covering 1.12.7 → 1.17.1 with the searches I ran. Could you confirm the package name and that those exact versions exist (or want me to search npm, GitHub repo, and the package's CHANGELOG.md for breaking changes)? If yes I will run a deeper search and report the breaking changes with sources.
🌐 Web query:
@alauda/doom changelog release notes version history
💡 Result:
You can find @alauda/doom's changelog and release history here:
- Package page (versions list / links to source & CHANGELOG): Socket / npm mirror for @alauda/doom. [1]
- Official docs site (Doom) — includes changelog/changes and links to docs: doom.js.org. [2]
- Libraries.io / npm listing (version history and changelog note). [3]
If you want, I can fetch the CHANGELOG.md contents (or list recent release notes) and paste the latest entries here.
🌐 Web query:
@alauda/doom package documentation migration guide
💡 Result:
Short answer — use the official Doom docs (doom.js.org) migration docs and the package README. Key references and quick steps:
References
- Doom docs site (usage & configuration) — shows new config formats, defineConfig import, theme/plugin guidance. [1]
- @alauda/doom npm/package page (README, changelog links). [2]
Quick migration checklist
- Install/update package: npm install @alauda/doom (or the desired version). See package README/changelog. [2]
- Replace old config with doom.config.{yaml,yml,js,ts} and, for JS/TS configs, export via defineConfig from '@alauda/doom/config'. [1][2]
- Move site-wide options into new schema fields (lang, title, logo, base, outDir, api/export/lint/etc.) per Configuration docs. [1]
- If you used a custom theme or plugins, export/compose them via the new theme entry (e.g., theme/index.ts export * from '@alauda/doom/theme') and update plugin references to the new plugin API. [1]
- Update CLI usage/flags as documented (e.g., -R/--edit-repo, -a/--algolia, -S/--site-url) and rebuild. [1]
- Test MDX/MD pipelines and API docs (OpenAPI/CRD) per the API Documentation section; adjust references/export options if needed. [1]
If you want, I can:
- fetch the specific changelog entries for the version you’re migrating from/to, or
- produce a concrete patch (diff) for your current doom config if you paste it.
Sources
- Doom — Configuration & Usage docs. https://doom.js.org/en/usage/configuration.html. [1]
- @alauda/doom package/README (npm / package pages). [2]
Verify migration steps completed for @alauda/doom upgrade.
The upgrade from ^1.12.7 to ^1.17.1 includes breaking changes requiring configuration and setup updates:
- Configuration format change to
doom.config.{yaml,yml,js,ts}withdefineConfigexport - Site-wide options moved to new schema fields (lang, title, logo, base, outDir, api/export/lint, etc.)
- Theme and plugin API updated—custom themes/plugins must be refactored per the new API
- CLI flags may have changed
Confirm that all migration steps from the Doom documentation have been completed.
🤖 Prompt for AI Agents
In package.json around line 5, the dependency was upgraded from @alauda/doom
^1.12.7 to ^1.17.1 but the PR did not confirm migration steps; update and
validate the project to the new Doom v1.17 config and APIs by renaming/replacing
legacy config to doom.config.{yaml,yml,js,ts} and exporting via defineConfig,
migrate site-wide options to the new schema fields (lang, title, logo, base,
outDir, api/export/lint, etc.), refactor any custom themes/plugins to the new
theme/plugin API, adjust CLI flags and npm scripts if changed, run the Doom
migration checklist from the docs, build/lint/test the site to ensure no
runtime/config errors, and commit any transformed config/theme/plugin files and
updated scripts.
| components={{ | ||
| a: (props: any) => { | ||
| const CustomMDXComponent = getCustomMDXComponent(); | ||
| const pathname = getPathname(props.href); | ||
| if (!shouldDownload(pathname)) { | ||
| return <CustomMDXComponent.a {...props}></CustomMDXComponent.a>; | ||
| } | ||
|
|
||
| // <div className="flex justify-between" style={{marginTop:'-1.5rem'}}> | ||
| // <LastUpdated></LastUpdated> | ||
| // <DocID></DocID> | ||
| // </div> | ||
| // </> | ||
| // ) : ( | ||
| // <CustomMDXComponent.h1 {...props} /> | ||
| // ); | ||
| // }, | ||
| // }} | ||
| return ( | ||
| <CustomMDXComponent.a | ||
| {...props} | ||
| href="" | ||
| onClick={(e: Event) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| e.stopImmediatePropagation?.(); | ||
| downloadFile( | ||
| props.href, | ||
| pathname.split("/").pop() || "download" | ||
| ); | ||
| }} | ||
| ></CustomMDXComponent.a> | ||
| ); | ||
| }, | ||
| }} |
There was a problem hiding this comment.
Preserve href for accessibility and UX; use download attribute instead.
Setting href="" removes the link destination, breaking right-click "Copy link" and accessibility features. Consider keeping the original href and using the download attribute, or at minimum preserve the href for context.
Also, the event type should be React.MouseEvent for proper typing.
🔎 Proposed fix
return (
<CustomMDXComponent.a
{...props}
- href=""
- onClick={(e: Event) => {
+ download={pathname.split("/").pop() || "download"}
+ onClick={(e: React.MouseEvent<HTMLAnchorElement>) => {
e.preventDefault();
e.stopPropagation();
- e.stopImmediatePropagation?.();
downloadFile(
props.href,
pathname.split("/").pop() || "download"
);
}}
></CustomMDXComponent.a>
);📝 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.
| components={{ | |
| a: (props: any) => { | |
| const CustomMDXComponent = getCustomMDXComponent(); | |
| const pathname = getPathname(props.href); | |
| if (!shouldDownload(pathname)) { | |
| return <CustomMDXComponent.a {...props}></CustomMDXComponent.a>; | |
| } | |
| // <div className="flex justify-between" style={{marginTop:'-1.5rem'}}> | |
| // <LastUpdated></LastUpdated> | |
| // <DocID></DocID> | |
| // </div> | |
| // </> | |
| // ) : ( | |
| // <CustomMDXComponent.h1 {...props} /> | |
| // ); | |
| // }, | |
| // }} | |
| return ( | |
| <CustomMDXComponent.a | |
| {...props} | |
| href="" | |
| onClick={(e: Event) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| e.stopImmediatePropagation?.(); | |
| downloadFile( | |
| props.href, | |
| pathname.split("/").pop() || "download" | |
| ); | |
| }} | |
| ></CustomMDXComponent.a> | |
| ); | |
| }, | |
| }} | |
| components={{ | |
| a: (props: any) => { | |
| const CustomMDXComponent = getCustomMDXComponent(); | |
| const pathname = getPathname(props.href); | |
| if (!shouldDownload(pathname)) { | |
| return <CustomMDXComponent.a {...props}></CustomMDXComponent.a>; | |
| } | |
| return ( | |
| <CustomMDXComponent.a | |
| {...props} | |
| download={pathname.split("/").pop() || "download"} | |
| onClick={(e: React.MouseEvent<HTMLAnchorElement>) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| downloadFile( | |
| props.href, | |
| pathname.split("/").pop() || "download" | |
| ); | |
| }} | |
| ></CustomMDXComponent.a> | |
| ); | |
| }, | |
| }} |
🤖 Prompt for AI Agents
In theme/layout/index.tsx around lines 69 to 93, the code clears href and uses a
raw Event type which breaks accessibility and typing; preserve the original href
(props.href) on the anchor so right-click "Copy link" and assistive tech still
see the destination, add a download attribute (e.g.,
download={pathname.split("/").pop() || "download"}) to trigger download
behavior, and change the onClick signature to use
React.MouseEvent<HTMLAnchorElement> (or React.MouseEvent) while still calling
e.preventDefault(), e.stopPropagation(), and e.stopImmediatePropagation?.();
call downloadFile(props.href, filename) inside the handler and do not overwrite
href.
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In @package.json:
- Line 5: The package.json dependency entry for "@alauda/doom" uses a
non-existent version "^1.17.6"; update the dependency to a valid version (e.g.,
change "@alauda/doom": "^1.17.6" to the correct published version such as
"1.7.1" or the intended "1.7.6"), or correct the package name if that was the
mistake, then run npm view @alauda/doom versions to confirm the chosen version
exists and npm install to verify resolution.
In @styles/index.css:
- Around line 21-35: .paginationItem declares font-weight twice; remove the
redundant declaration so only one font-weight assignment remains. In the
.paginationItem rule, delete the first font-weight: var(--font-weight-medium)
(the standalone occurrence) and keep the later --tw-font-weight and font-weight
pair (or vice versa) so the class has a single clear font-weight definition.
In @theme/layout/index.tsx:
- Around line 77-93: The anchor currently forces href="#" which can break
UX/accessibility; change the rendered element in the CustomMDXComponent.a block
to use the original props.href (e.g., href={props.href}) instead of "#" so the
link remains valid for screen readers and fallback behavior, keep the existing
preventDefault()/stopPropagation() in the onClick handler and the download prop
and preserve calling downloadFile(props.href, ...), and optionally ensure
aria-label/role is set if the link needs a button-like announcement.
In @theme/utils/download.ts:
- Around line 24-31: getBasePath currently falls back to a hardcoded
"/knowledge/" which can drift from the build config; update getBasePath to
derive its fallback from the project's configuration instead of a literal string
by importing the exported base path (or a BUILD_BASE_PATH env var) from the
build config (e.g., the value exported in doom.config.ts) and using that as the
fallback; ensure getBasePath still attempts the pathname regex first, then
returns the imported/configured base path if the regex fails.
🧹 Nitpick comments (5)
styles/doc-layout.css (1)
63-79: Sidebar usesoverflow: auto scrollwhich may cause unexpected behavior.The
overflow: auto scrolldeclaration setsoverflow-x: autoandoverflow-y: scroll. If the intent is to always show vertical scrollbar while auto-hiding horizontal, this is fine. However,scrollforces a scrollbar even when content fits, which may not be the desired UX.Consider using `overflow-y: auto` if scrollbar should only appear when needed
.rp-doc-layout__sidebar { ... - overflow: auto scroll; + overflow-x: auto; + overflow-y: auto; }styles/index.css (1)
153-165: Potential z-index conflict with doc-layout.css.The
.rp-doc-layout__menuhere usesz-index: 100(line 159), butdoc-layout.cssdefines--rp-z-index-nav-screen: 70as the highest z-index token. This hardcoded value bypasses the z-index hierarchy established in the CSS variables.Consider using the CSS variable system for consistency
.rp-doc-layout__menu { display: none; position: fixed; top: 0; left: 0; right: 0; - z-index: 100; + z-index: var(--rp-z-index-nav-screen); ... }Or define a new token in doc-layout.css if 100 is intentionally higher than nav-screen.
theme/layout/index.tsx (2)
69-95: Performance:getCustomMDXComponent()called on every anchor render.
getCustomMDXComponent()is invoked inside theacomponent, meaning it's called for every anchor element rendered in MDX content. Consider hoisting this call outside the component function or memoizing it.Hoist getCustomMDXComponent call
+const CustomMDXComponent = getCustomMDXComponent(); + export default () => { useEffect(() => { window.parent.postMessage(window.location.href, "*"); }, []); return ( <Layout ... components={{ a: (props: any) => { - const CustomMDXComponent = getCustomMDXComponent(); const pathname = getPathname(props.href); if (!shouldDownload(pathname)) { return <CustomMDXComponent.a {...props}></CustomMDXComponent.a>; } ... }, }} ></Layout> ); };
82-85: UseReact.MouseEventinstead ofEventfor type safety.The event parameter type
Event(line 82) should beReact.MouseEvent<HTMLAnchorElement>for proper TypeScript type checking in React. Additionally,stopImmediatePropagationis not a standard React synthetic event method.Fix event type
- onClick={(e: Event) => { + onClick={(e: React.MouseEvent<HTMLAnchorElement>) => { e.preventDefault(); e.stopPropagation(); - e.stopImmediatePropagation?.(); downloadFile( props.href, pathname.split("/").pop() || "download" ); - return false; }}theme/utils/download.ts (1)
47-51: Empty catch block silently swallows URL parsing errors.The empty catch block provides no feedback when URL parsing fails. Consider logging a warning for debugging purposes.
Add minimal logging
try { path = new URL(url).pathname; } catch { + // URL parsing failed, use url as-is }
📜 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 (8)
package.jsonstyles/doc-layout.cssstyles/index.csstheme/components/Card/index.tsxtheme/components/Checkbox/index.tsxtheme/layout/DocLayout/index.tsxtheme/layout/index.tsxtheme/utils/download.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- theme/components/Card/index.tsx
- theme/components/Checkbox/index.tsx
- theme/layout/DocLayout/index.tsx
🧰 Additional context used
🧬 Code graph analysis (1)
theme/layout/index.tsx (1)
theme/utils/download.ts (3)
getPathname(16-22)shouldDownload(11-14)downloadFile(39-81)
🪛 Biome (2.1.2)
styles/index.css
[error] 28-28: 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 (6)
styles/doc-layout.css (3)
1-28: Well-structured CSS variables for layout tokens.The root CSS variables establish a clear z-index hierarchy and dimension tokens. The calculated margins for sidebar and outline using
max()andcalc()are a good approach for centering content.
179-205: Mobile sidebar transition and visibility handling looks good.The mobile breakpoint (≤768px) properly handles the sidebar with opacity, transform, and fixed positioning for a slide-in effect. The
--openmodifier correctly overrides visibility and positioning.
30-34: Media query syntax is widely supported—no changes needed.The
width >= 1280pxandwidth <= 1279pxsyntax uses CSS Media Queries Level 4 range syntax, which is fully supported across all modern browsers (Chrome/Chromium 104+, Edge 104+, Firefox, Safari 16.4+, and iOS Safari 16.4+). The boundary definitions correctly avoid overlap.styles/index.css (1)
62-109: Card and LinkCard styles are well-defined.The component styles follow consistent patterns with proper use of CSS variables for theming and appropriate transition properties for interactive states.
theme/utils/download.ts (2)
1-9: Extension list may need expansion for common file types.The
DOWNLOAD_EXTENSIONSlist includes code/data files but omits common document types like.doc,.xlsx,.csv. Verify if this is intentional based on the documentation content.
39-71: Same-origin download flow is well-implemented.The blob-based download approach for same-origin files correctly handles base path normalization, proper cleanup of blob URLs and DOM elements, and includes error handling with a console warning.
| "packageManager": "yarn@4.12.0", | ||
| "devDependencies": { | ||
| "@alauda/doom": "^1.12.7", | ||
| "@alauda/doom": "^1.17.6", |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
@alauda/doom npm package versions security
💡 Result:
Quick summary for @alauda/doom:
- No public npm security advisories or CVEs specifically for @alauda/doom were found in my search. [1][2]
- Package appears actively published/maintained (recent versions listed). [1]
- Recommended action: run npm audit (or Snyk/Dependabot) against your project and pin to a specific safe version; review the package's changelog/CHANGELOG.md and repository before upgrading. (Tools: npm audit, Snyk, GitHub Dependabot).
Sources:
- Socket package page for @alauda/doom (versions, publish info). [1]
- Libraries.io / npm package listing for @alauda/doom. [2]
Would you like me to run an npm audit on your project or check a specific version?
🌐 Web query:
@alauda/doom v1.17.6 changelog breaking changes
💡 Result:
Do you want me to look up the official changelog for @alauda/doom v1.17.6 and list its breaking changes? (I can search the repo, npm, and CHANGELOG.md.)
🌐 Web query:
@alauda/doom GitHub repository changelog releases
💡 Result:
I couldn't find a public GitHub "Releases" page for an alauda/doom repository. The project appears published on npm and documented at doom.js.org; its changelog is maintained in the repository's CHANGELOG.md (referenced from the package/docs), not GitHub releases. See: (socket.dev)
Sources:
- npm / package info mentioning CHANGELOG.md. (socket.dev)
- Official docs/site (doom.js.org) which links project/docs. (doom.js.org)
If you want, I can search further (e.g., for a specific repo URL, GitHub user/org, or the CHANGELOG.md contents).
Citations:
- 1: https://socket.dev/npm/package/%40alauda/doom?utm_source=openai
- 2: https://socket.dev/npm/package/%40alauda/doom?utm_source=openai
- 3: https://doom.js.org/en/?utm_source=openai
🌐 Web query:
site:github.com @alauda doom package
💡 Result:
I couldn't find a GitHub repo named "doom" under the Alauda organization. Instead I found an npm package @alauda/doom and its docs site (Doom — a documentation tool by Alauda):
- @alauda/doom (npm/package info). [1]
- Official Doom docs / site (doom.js.org). [2]
- Package metadata listing (libraries.io). [3]
If you want, I can search more broadly (other org/user names) or look for a specific GitHub repo file—tell me which.
🌐 Web query:
@alauda/doom npm registry version 1.17.6
💡 Result:
There is no @alauda/doom version 1.17.6 on the npm registry. The package's published versions (latest ~1.7.1 / historical list) are shown on npm and package trackers — 1.17.6 does not appear. [1][2]
Sources:
[1] npm package page for @alauda/doom.
[2] Package/version list for @alauda/doom (Socket / Libraries.io).
🌐 Web query:
"@alauda/doom" repository github alauda-io
💡 Result:
I found the @alauda/doom package (Doom docs tool) with official docs at doom.js.org and registry entries — the docs reference a GitHub repo path alauda/doom (e.g., editRepoBaseUrl: alauda/doom/tree/main/docs), but I couldn't locate a distinct public repository under an "alauda-io" org. Sources:
- @alauda/doom package metadata (Libraries.io / npm summary). [1]
- Official Doom documentation site (doom.js.org) which references the GitHub path alauda/doom. [2]
- Package security/registry page showing versions and maintainers. [3]
Do you want me to (a) try to open github.com/alauda/doom directly, (b) search for a repo under a different Alauda org/name, or (c) fetch the package's npm registry page?
Version ^1.17.6 does not exist on the npm registry for @alauda/doom.
The latest available version is approximately 1.7.1. This will cause npm install to fail. Verify the intended version—it may be a typo (e.g., 1.7.6 or 1.7.1), or the package name may be incorrect.
🤖 Prompt for AI Agents
In @package.json at line 5, The package.json dependency entry for "@alauda/doom"
uses a non-existent version "^1.17.6"; update the dependency to a valid version
(e.g., change "@alauda/doom": "^1.17.6" to the correct published version such as
"1.7.1" or the intended "1.7.6"), or correct the package name if that was the
mistake, then run npm view @alauda/doom versions to confirm the chosen version
exists and npm install to verify resolution.
| .paginationItem { | ||
| font-weight: var(--font-weight-medium); | ||
| 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 property.
Static analysis correctly identified that font-weight is declared twice (lines 22-23 and 27-28). This is redundant and could cause confusion.
Proposed fix
.paginationItem {
- font-weight: var(--font-weight-medium);
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;
}📝 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.
| .paginationItem { | |
| font-weight: var(--font-weight-medium); | |
| 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; | |
| } | |
| .paginationItem { | |
| 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; | |
| } |
🧰 Tools
🪛 Biome (2.1.2)
[error] 28-28: 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 21 - 35, .paginationItem declares font-weight
twice; remove the redundant declaration so only one font-weight assignment
remains. In the .paginationItem rule, delete the first font-weight:
var(--font-weight-medium) (the standalone occurrence) and keep the later
--tw-font-weight and font-weight pair (or vice versa) so the class has a single
clear font-weight definition.
| return ( | ||
| <CustomMDXComponent.a | ||
| {...props} | ||
| href="#" | ||
| download={pathname.split("/").pop() || "download"} | ||
| onClick={(e: Event) => { | ||
| e.preventDefault(); | ||
| e.stopPropagation(); | ||
| e.stopImmediatePropagation?.(); | ||
| downloadFile( | ||
| props.href, | ||
| pathname.split("/").pop() || "download" | ||
| ); | ||
| return false; | ||
| }} | ||
| ></CustomMDXComponent.a> | ||
| ); |
There was a problem hiding this comment.
Setting href="#" may cause accessibility and UX issues.
For download links, href="#" (line 80) can cause the page to scroll to top if the click handler fails, and screen readers may announce it incorrectly. Consider keeping the original href while relying on preventDefault().
Proposed fix
return (
<CustomMDXComponent.a
{...props}
- href="#"
+ href={props.href}
download={pathname.split("/").pop() || "download"}
onClick={(e: React.MouseEvent<HTMLAnchorElement>) => {
e.preventDefault();
- e.stopPropagation();
- e.stopImmediatePropagation?.();
downloadFile(
props.href,
pathname.split("/").pop() || "download"
);
- return false;
}}
></CustomMDXComponent.a>
);📝 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.
| return ( | |
| <CustomMDXComponent.a | |
| {...props} | |
| href="#" | |
| download={pathname.split("/").pop() || "download"} | |
| onClick={(e: Event) => { | |
| e.preventDefault(); | |
| e.stopPropagation(); | |
| e.stopImmediatePropagation?.(); | |
| downloadFile( | |
| props.href, | |
| pathname.split("/").pop() || "download" | |
| ); | |
| return false; | |
| }} | |
| ></CustomMDXComponent.a> | |
| ); | |
| return ( | |
| <CustomMDXComponent.a | |
| {...props} | |
| href={props.href} | |
| download={pathname.split("/").pop() || "download"} | |
| onClick={(e: React.MouseEvent<HTMLAnchorElement>) => { | |
| e.preventDefault(); | |
| downloadFile( | |
| props.href, | |
| pathname.split("/").pop() || "download" | |
| ); | |
| }} | |
| ></CustomMDXComponent.a> | |
| ); |
🤖 Prompt for AI Agents
In @theme/layout/index.tsx around lines 77 - 93, The anchor currently forces
href="#" which can break UX/accessibility; change the rendered element in the
CustomMDXComponent.a block to use the original props.href (e.g.,
href={props.href}) instead of "#" so the link remains valid for screen readers
and fallback behavior, keep the existing preventDefault()/stopPropagation() in
the onClick handler and the download prop and preserve calling
downloadFile(props.href, ...), and optionally ensure aria-label/role is set if
the link needs a button-like announcement.
| const getBasePath = (): string => { | ||
| const pathname = window.location.pathname; | ||
| const match = pathname.match(/^(\/[^/]+\/)/); | ||
| if (match) { | ||
| return match[1]; | ||
| } | ||
| return "/knowledge/"; | ||
| }; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Search for "knowledge" references to understand its usage
rg -n "knowledge" --type-list | grep -E "json|ts|js|config|env" || true
rg -n "knowledge" --type=ts --type=js --type=json -g '!node_modules' | head -50Repository: alauda/knowledge
Length of output: 1366
🏁 Script executed:
#!/bin/bash
# Check the context of getBasePath function usage
rg -n "getBasePath" -A 3 -B 3 --type=ts --type=js -g '!node_modules'Repository: alauda/knowledge
Length of output: 813
🏁 Script executed:
#!/bin/bash
# Look for base path, route prefix, or environment-related configuration
rg -n "basePath|BASE_PATH|baseUrl|BASE_URL|ROUTE|prefix" --type=ts --type=js --type=json -g '!node_modules' | head -40Repository: alauda/knowledge
Length of output: 335
Consider deriving the fallback base path from project configuration.
The getBasePath() function attempts to dynamically extract the base path from the URL pathname, but falls back to the hardcoded "/knowledge/". While this fallback aligns with the current project configuration in doom.config.ts, it creates a maintenance risk: if the base path is changed in the build configuration, the fallback becomes stale and must be updated separately. Consider exporting the base path from build configuration or using an environment variable for the fallback to keep them synchronized.
🤖 Prompt for AI Agents
In @theme/utils/download.ts around lines 24 - 31, getBasePath currently falls
back to a hardcoded "/knowledge/" which can drift from the build config; update
getBasePath to derive its fallback from the project's configuration instead of a
literal string by importing the exported base path (or a BUILD_BASE_PATH env
var) from the build config (e.g., the value exported in doom.config.ts) and
using that as the fallback; ensure getBasePath still attempts the pathname regex
first, then returns the imported/configured base path if the regex fails.
| } else { | ||
| const a = document.createElement("a"); | ||
| a.href = url; | ||
| a.download = filename; | ||
| a.style.display = "none"; | ||
| document.body.appendChild(a); | ||
| a.click(); | ||
| setTimeout(() => document.body.removeChild(a), 100); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
HTML download attribute cross-origin same-origin policy behavior
💡 Result:
Short answer
- The HTML anchor download attribute is only honored for same-origin, blob: and data: URLs; browsers ignore it for cross-origin http(s) URLs. [1][2]
Why
- Browsers block cross-origin use of download to prevent user-mediated cross-origin info leaks / spoofing. Chromium removed support in Chrome 65; Firefox likewise only honors same-origin. [2][1]
Practical workarounds
- Serve the file with a Content-Disposition: attachment; filename="..." header from the resource origin (or add a server-side ?dl-style endpoint that sets that header). This forces download and can control the filename. [3][4]
- Proxy the file through your origin so the link is same-origin.
- Fetch the resource via XHR/fetch (CORS-enabled response), create a Blob, then create an object URL and set that on an to trigger a client-side download.
Sources
- MDN: element — download behavior and same-origin note. [1]
- Chromium change / rationale (Chrome 65) and discussion. [2]
- Guidance to use Content-Disposition header to force download. [3]
- Example provider solution (append ?dl to force CDN to return Content-Disposition). [4]
🏁 Script executed:
cat -n theme/utils/download.ts | head -100Repository: alauda/knowledge
Length of output: 2938
Cross-origin downloads will fail silently.
The download attribute is ignored by browsers for cross-origin URLs (Chrome 65+, Firefox). In the else branch (lines 72-80), the file will open in a new tab instead of downloading, and the filename will not be controlled. The same-origin branch correctly uses a blob URL to work around this, but the cross-origin case lacks any fallback or user-facing warning.
Consider adding a warning to users about cross-origin files, or fetch and proxy the file through your origin to enable proper downloads.
Summary by CodeRabbit
New Features
Style / Theming
Chores
✏️ Tip: You can customize this high-level summary in your review settings.