feat: support fixed language for edit repo #178
Conversation
close IDP-1307
🦋 Changeset detectedLatest commit: 1fe6845 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughAdds language-aware edit-repo option and config handling, restructures theme exports via a new barrel, introduces EditLink and a conditional Algolia-backed Search, updates documentation (EN/ZH), adjusts edit-page base URL in config, bumps dependencies, and points theme export to a new index entry. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant CLI as CLI (-R/--edit-repo)
participant Loader as load-config:getCommonConfig
participant Site as Runtime Theme
User->>CLI: doom ... -R [boolean|url|lang]
CLI-->>Loader: editRepo option value
Loader->>Loader: if value contains "/" -> use as base URL
Loader->>Loader: else if base exists -> append value as segment
Loader-->>Site: resolved editRepoBaseUrl (+ optional fixed lang)
Site->>Site: useEditLink builds final edit link (lang-aware)
Site-->>User: Edit link rendered (external)
sequenceDiagram
autonumber
participant Env as Env Vars
participant Theme as Theme/Search
participant Algolia as AlgoliaSearch
participant CoreSearch as Core Search
Env-->>Theme: ALGOLIA_* present?
alt All present
Theme->>Theme: useMemo(docSearchProps with facetFilters: lang)
Theme->>Algolia: render with props + locales
else Missing any
Theme->>CoreSearch: render fallback search
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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 |
Signed-off-by: JounQin <admin@1stg.me>
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for fixed language configuration in edit repository URLs, allowing documentation to be edited from a repository that has language-specific subdirectories.
- Adds new EditLink component with
useEditLinkhook to handle language-aware edit URLs - Restructures theme exports by moving Search component to separate file
- Updates CLI help text to reflect new language parameter support for edit-repo option
Reviewed Changes
Copilot reviewed 14 out of 16 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| packages/doom/styles/edit-link.module.scss | Adds CSS styles for the new EditLink component |
| packages/doom/src/theme/useEditLink.ts | Creates hook to generate edit URLs with fixed language support |
| packages/doom/src/theme/index.ts | Updates theme exports to include new components |
| packages/doom/src/theme/Search.tsx | Extracts Search component from Layout.tsx |
| packages/doom/src/theme/Layout.tsx | Refactors to remove Search component and update imports |
| packages/doom/src/theme/EditLink.tsx | Implements new EditLink component using the useEditLink hook |
| packages/doom/src/cli/load-config.ts | Adds logic to handle language-based edit repository configuration |
| packages/doom/src/cli/index.ts | Updates CLI help text for edit-repo parameter |
| packages/doom/package.json | Updates package exports path and dependency versions |
| package.json | Updates development dependency versions |
| doom.config.yml | Changes edit repository URL from tree to blob format |
| docs/zh/start.mdx | Updates CLI help documentation in Chinese |
| docs/en/start.mdx | Updates CLI help documentation and improves wording |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
commit: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (3)
packages/doom/src/cli/load-config.ts (1)
171-178: Ensure robust slash handling when appending repo/lang segmentThe current logic can yield inconsistent trailing slashes and is brittle if the base already ends with a different segment. Normalize with leading/trailing slash helpers and avoid double-appending.
Apply:
- if (editRepo.includes('/')) { - editRepoBaseUrl = editRepo - } else if (editRepoBaseUrl) { - editRepoBaseUrl = addTrailingSlash(editRepoBaseUrl) - if (!editRepoBaseUrl.endsWith(`/${editRepo}/`)) { - editRepoBaseUrl += editRepo - } - } + if (editRepo.includes('/')) { + editRepoBaseUrl = editRepo + } else if (editRepoBaseUrl) { + const base = addTrailingSlash(editRepoBaseUrl) + const segment = removeLeadingSlash(editRepo) + editRepoBaseUrl = base.endsWith(`/${segment}/`) + ? base + : addTrailingSlash(base + segment) + }#!/bin/bash # Quick sanity checks for combinations node - <<'JS' function addTrailingSlash(s){return s.endsWith('/')?s:s+'/'} function removeLeadingSlash(s){return s.startsWith('/')?s.slice(1):s} function build(base, seg){ const b=addTrailingSlash(base); const s=removeLeadingSlash(seg); return (b.endsWith(`/${s}/`) ? b : addTrailingSlash(b+s)) } console.log(build('alauda/doom/blob/main/docs','en')) // .../docs/en/ console.log(build('alauda/doom/blob/main/docs/','en')) // .../docs/en/ console.log(build('alauda/doom/blob/main/docs/en','en')) // .../docs/en/ console.log(build('alauda/doom/blob/main/docs/en/','en')) // .../docs/en/ JSpackages/doom/src/theme/useEditLink.ts (2)
20-21: Avoid.at(-2); validate and handle short URLsMore explicit and compatible with older runtimes. Also guards empty segments.
- const lastSegment = docRepoBaseUrl.split('/').at(-2) + const segments = docRepoBaseUrl.split('/').filter(Boolean) + const lastSegment = segments.length ? segments[segments.length - 1] : undefined
27-27: Improve readability by extracting the final page pathThe current ternary inside template string hurts clarity.
- const link = `${docRepoBaseUrl}${fixedLang ? relativePagePath.split('/').slice(1).join('/') : relativePagePath}` + const finalPagePath = fixedLang + ? relativePagePath.split('/').slice(1).join('/') + : relativePagePath + const link = `${docRepoBaseUrl}${finalPagePath}`
🧹 Nitpick comments (20)
doom.config.yml (1)
28-28: Blob vs. Edit route mismatch with CTAIf the UI text says “Edit this page on GitHub”, consider using the
/edit/<branch>/route instead of/blob/to land users directly on the editor. Please confirm this aligns withuseEditLinkbehavior; otherwise switch to/edit/.Would you like me to adjust the base to
alauda/doom/edit/main/docsand verify end-to-end?packages/doom/styles/edit-link.module.scss (1)
1-11: Improve a11y and avoid transitioning all properties
- Add visible focus styles and restore underline on hover/focus for link discoverability.
- Limit transition to
colorto reduce layout/repaint work.Apply:
.editLink { - color: var(--rp-c-brand); - text-decoration: none; + color: var(--rp-c-brand); + text-decoration: none; font-size: 15px; font-weight: 500; margin: 20px 0; - transition: all 0.2s ease-in-out; + transition: color 0.2s ease-in-out; &:hover { - color: var(--rp-c-brand-dark); + color: var(--rp-c-brand-dark); + text-decoration: underline; } + &:focus-visible { + outline: 2px solid var(--rp-c-brand); + outline-offset: 2px; + text-decoration: underline; + } }packages/doom/src/cli/load-config.ts (1)
181-189: CLI help promises ‘lang’ variant; ensure UX when base missingIf users pass
-R <lang>withouteditRepoBaseUrlin config, we log an error and do nothing. Consider upgrading the message to hint thateditRepoBaseUrlis required for the lang variant.packages/doom/src/cli/index.ts (1)
99-101: Includealauda-ruin help text
load-config.tssupports'alauda-ru'; reflect it here for consistency:[boolean|alauda|alauda-ru].Apply:
- .option( - '-R, --edit-repo [boolean|url|lang]', - 'Whether to enable or override the `editRepoBaseUrl` config feature, `https://github.com/` prefix could be omitted; if a language code is provided, all other languages will use the same repo but with the language code as sub-path', + .option( + '-R, --edit-repo [boolean|url|lang]', + 'Enable or override `editRepoBaseUrl`. `https://github.com/` prefix may be omitted; if a language code is provided, all languages use the same repo with that code as a sub-path', parseBooleanOrString, false, )Also consider noting that the
langvariant requireseditRepoBaseUrlin config.docs/zh/start.mdx (1)
99-101: 文档与实现不一致：Algolia 还支持alauda-ruCLI 实现支持
alauda-ru预设，帮助文案仅写了alauda。建议更新为[boolean|alauda|alauda-ru]。Apply:
- -a, --algolia [boolean|alauda] Whether to enable or use the alauda (docs.alauda.io) preset for Algolia search (default: false) + -a, --algolia [boolean|alauda|alauda-ru] Whether to enable or use the alauda (docs.alauda.io) or alauda-ru preset for Algolia search (default: false)packages/doom/package.json (1)
103-107: type-fest major‑range expansion: double‑check TS compatibilityAllowing
^5.xis probably fine (you’re on TS ^5.9), but it’s a types-only package with occasional breaking changes. Make sure CI type checks still pass across consumers.Suggestion: if downstream repos compile against this package, run a full
tsc -bthere or add an integration job to catch cross‑package type regressions.docs/en/start.mdx (2)
103-104: Clarify the new-R/--edit-repo [boolean|url|lang]with a concrete exampleGreat addition. Consider adding one example for the
langcase to remove ambiguity.For example:
doom dev -R zh-> all languages share the same repo, usingzhas a fixed sub-path.doom dev -R https://github.com/org/repo/tree/main/docs/zh/
191-191: Nit: “Specially” → “Specifically”Minor wording tweak for standard phrasing.
- Specially, if you use `-g '*'` for full translation, ... + Specifically, if you use `-g '*'` for full translation, ...packages/doom/src/theme/index.ts (1)
1-6: Scope the ESLint disable to the star re‑exportPrefer limiting suppression to the offending line to avoid masking future issues.
-/* eslint-disable import-x/export */ - -export * from '@rspress/core/theme' +// eslint-disable-next-line import-x/export +export * from '@rspress/core/theme' export { EditLink } from './EditLink.tsx' export { Layout } from './Layout.tsx' export { Search } from './Search.tsx'packages/doom/src/theme/EditLink.tsx (1)
16-18: Addrel="noopener noreferrer"for external link securityOpening a new tab should include
relto prevent reverse‑tabnabbing.- <a href={link} target="_blank" className={classes.editLink}> + <a + href={link} + target="_blank" + rel="noopener noreferrer" + className={classes.editLink} + > {text} </a>packages/doom/src/theme/Layout.tsx (4)
10-10: Style import consistencyElsewhere you import CSS via the package subpath (
@alauda/doom/styles/...). Consider aligning here for consistency, or keep it consistently relative—either way, pick one convention.
51-57: Guard optionalflattenScopeto avoid runtime errors
flattenScope!will throw if undefined. Use optional chaining.- if (exportItem.flattenScope!.includes(sidebar._fileKey)) { + if (exportItem.flattenScope?.includes(sidebar._fileKey)) { matched = { sidebar, exportItem, depth, } }
109-116: Handle missingthemeConfig.localessafelyAccessing
.lengthon possibly undefined can crash. Use optional chaining and provide a sane fallback.- if (themeConfig.locales.length) { + if (themeConfig.locales?.length) { let found: MatchedSidebar | undefined - - for (const { lang, sidebar } of themeConfig.locales) { - const sidebarItems = sidebar[ - siteLang === lang ? '/' : `/${lang}` - ] as DoomSidebar[] + for (const { lang, sidebar } of themeConfig.locales) { + const base = siteLang === lang ? '/' : `/${lang}` + const sidebarItems = (sidebar[base] ?? sidebar['/']) as DoomSidebar[] found ??= getClosestSidebar(sidebarItems, pathname) if (found) { return found } }
129-134: Safer PDF filenameSidebar text may contain spaces or symbols. Normalize before composing the filename to avoid broken links on some hosts.
- const pdfLink = useMemo( - () => - found && - withBase(`${found.exportItem.name ?? found.sidebar.text}-${lang}.pdf`), + const pdfLink = useMemo(() => { + if (!found) return undefined + const base = (found.exportItem.name ?? found.sidebar.text).trim() + const safe = base.replace(/[^\w.-]+/g, '-') + return withBase(`${safe}-${lang}.pdf`) + }, [found, lang], )packages/doom/src/theme/useEditLink.ts (2)
26-28: Harden page path access to avoid runtime crash
page._relativePathmay be undefined on some routes; the cast masks this and.replacewill throw.Apply:
- const relativePagePath = (page._relativePath as string).replace(/\\/g, '/') + if (!page?._relativePath) return null + const relativePagePath = page._relativePath.replace(/\\/g, '/')
3-3: Add an explicit return type for the hookClarifies the public API and prevents accidental type drift.
-export function useEditLink() { +export function useEditLink(): { text: string; link: string } | null {packages/doom/src/theme/Search.tsx (4)
27-28: Scope locales to current language and normalize facet langUnconditionally passing
ZH_LOCALESshows Chinese UI for non‑ZH users. Also, Algolia facets often use base language (e.g.,zhvszh-CN).- const docSearchProps = useMemo( - () => ({ + const docSearchProps = useMemo( + () => ({ appId: process.env.ALGOLIA_APP_ID!, apiKey: process.env.ALGOLIA_API_KEY!, indexName: process.env.ALGOLIA_INDEX_NAME!, searchParameters: { - facetFilters: [`lang:${lang}`], + facetFilters: [`lang:${lang.split('-')[0]}`], }, }), [lang], ) return ( - <AlgoliaSearch docSearchProps={docSearchProps} locales={ZH_LOCALES} /> + <AlgoliaSearch + docSearchProps={docSearchProps} + locales={lang.startsWith('zh') ? ZH_LOCALES : undefined} + /> )Also applies to: 14-25
1-1: Annotate component type and compute a clear feature flagHelps typing and readability; no behavioral change.
-import { useLang } from '@rspress/core/runtime' +import { useLang } from '@rspress/core/runtime' +import type { FC } from 'react' @@ -export const Search = +const HAS_ALGOLIA = process.env.ALGOLIA_APP_ID && process.env.ALGOLIA_API_KEY && process.env.ALGOLIA_INDEX_NAME - ? () => { +export const Search: FC = HAS_ALGOLIA + ? () => { @@ - : OriginalSearch + : OriginalSearchAlso applies to: 9-13
3-6: Optional: avoid bundling Algolia code when disabledStatic imports can keep Algolia in the bundle even when env vars are missing. Consider lazy loading to trim baseline size.
I can provide a
lazy(() => import(...))variant behindHAS_ALGOLIAif you want to pursue this optimization.Also applies to: 9-13
1-1: Use local typeduseLangpackages/doom/src/runtime/hooks/useTranslation.ts exports a typed
useLang— import that in the theme to keepLanguageconsistent.Change in packages/doom/src/theme/Search.tsx:
- from:
import { useLang } from '@rspress/core/runtime'- to:
import { useLang } from '../runtime/hooks/useTranslation'
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (15)
.changeset/hungry-suits-play.md(1 hunks)docs/en/start.mdx(9 hunks)docs/zh/start.mdx(1 hunks)doom.config.yml(1 hunks)package.json(1 hunks)packages/doom/package.json(4 hunks)packages/doom/src/cli/index.ts(1 hunks)packages/doom/src/cli/load-config.ts(1 hunks)packages/doom/src/theme/EditLink.tsx(1 hunks)packages/doom/src/theme/Layout.tsx(1 hunks)packages/doom/src/theme/Search.tsx(1 hunks)packages/doom/src/theme/index.ts(1 hunks)packages/doom/src/theme/useEditLink.ts(1 hunks)packages/doom/styles/edit-link.module.scss(1 hunks)tsconfig.json(1 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2025-05-26T08:59:41.491Z
Learnt from: JounQin
PR: alauda/doom#30
File: src/runtime/components/K8sCrd.tsx:5-5
Timestamp: 2025-05-26T08:59:41.491Z
Learning: In rspress/core v2.0.0-beta.7, the '/theme' export is available in the package exports field as `"./theme": { "default": "./theme.ts" }`, so imports like `import { Badge, Button } from 'rspress/core/theme'` are valid even if ESLint shows resolution errors.
Applied to files:
packages/doom/src/theme/index.tstsconfig.jsonpackages/doom/package.json
📚 Learning: 2025-05-26T09:09:21.339Z
Learnt from: JounQin
PR: alauda/doom#30
File: src/runtime/components/K8sCrd.tsx:5-5
Timestamp: 2025-05-26T09:09:21.339Z
Learning: The alauda/doom project uses yarn v4 for dependency management, which handles package exports and module resolution correctly, so imports like `rspress/core/theme` work without any ESLint errors.
Applied to files:
packages/doom/src/theme/index.tstsconfig.jsonpackages/doom/package.json
📚 Learning: 2025-05-29T16:25:28.086Z
Learnt from: JounQin
PR: alauda/doom#40
File: src/plugins/sitemap/index.ts:7-7
Timestamp: 2025-05-29T16:25:28.086Z
Learning: In rspress/shared v2.0.0-beta.8, the '/logger' export is available in the package exports field, so imports like `import { logger } from 'rspress/shared/logger'` are valid even if ESLint shows resolution errors. This is used throughout the codebase in files like src/cli/translate.ts, src/cli/load-config.ts, src/utils/git.ts, and src/plugins/sitemap/index.ts.
Applied to files:
packages/doom/src/theme/index.ts
📚 Learning: 2025-06-06T07:08:55.881Z
Learnt from: JounQin
PR: alauda/doom#69
File: package.json:101-101
Timestamp: 2025-06-06T07:08:55.881Z
Learning: When a package has peer dependencies that require TypeScript (like `eslint-react/eslint-plugin`), TypeScript should be moved from devDependencies to dependencies to satisfy the peer dependency requirement.
Applied to files:
package.jsonpackages/doom/package.json
📚 Learning: 2025-05-26T09:09:21.339Z
Learnt from: JounQin
PR: alauda/doom#30
File: src/runtime/components/K8sCrd.tsx:5-5
Timestamp: 2025-05-26T09:09:21.339Z
Learning: JounQin has mentioned multiple times that the alauda/doom project uses yarn v4, and there are no ESLint import resolution errors for rspress/core subpath imports.
Applied to files:
packages/doom/package.json
🧬 Code graph analysis (2)
packages/doom/src/theme/EditLink.tsx (2)
packages/doom/src/theme/index.ts (1)
EditLink(4-4)packages/doom/src/theme/useEditLink.ts (1)
useEditLink(3-33)
packages/doom/src/theme/Search.tsx (2)
packages/doom/src/theme/index.ts (1)
Search(6-6)packages/doom/src/runtime/hooks/useTranslation.ts (1)
useLang(7-7)
🔇 Additional comments (9)
.changeset/hungry-suits-play.md (1)
1-6: Changeset metadata is correctMinor bump for the new edit-repo language feature is appropriate.
package.json (1)
35-37: Dev dep bumps look good — verify dedupe & lockfileIncremental bumps look fine; yarn-berry-deduplicate is unavailable in this environment (command not found). Ensure the lockfile is updated and run locally: yarn-berry-deduplicate --list && yarn-berry-deduplicate — confirm no duplicate ranges and commit the updated lockfile.
Applies to package.json lines 35–37 (also 42, 45, 49).packages/doom/src/cli/load-config.ts (1)
238-244: Help string mismatch — include 'alauda-ru' in CLI helppackages/doom/src/cli/load-config.ts (lines 238–244) accepts 'alauda-ru', but the CLI help only documents 'alauda'. Update the CLI help text to mention 'alauda-ru' for discoverability.
tsconfig.json (1)
8-8: Approve — tsconfig alias and package export are consistenttsconfig.json maps "@alauda/doom/theme" → ./packages/doom/src/theme/index.ts and packages/doom/package.json exports "./theme" → "./lib/theme/index.js" — confirmed. packages/doom/lib is not checked into the repo (build artifact); ensure lib/theme/index.js is produced and included when publishing.
docs/en/start.mdx (1)
88-118: Docs polish looks goodHelp text and command sections read clearly, options are consistent with the CLI. Nice work.
packages/doom/src/theme/index.ts (1)
3-6: Re‑export precedence: confirm no ambiguous export warnings
export *plus named re‑exports is fine; your namedLayout/Searchwill take precedence over the core ones. Just confirm bundler/ESLint doesn’t flag ambiguous exports.Given the project’s Yarn v4 + subpath‑exports setup (per our past learnings), this pattern should resolve cleanly.
packages/doom/src/theme/Layout.tsx (1)
137-153: PDF link injection LGTMGood UX: conditional render, MDX component wrapper, and proper
relon the anchor.packages/doom/package.json (2)
53-58: Approve — @RsPress packages aligned; no stragglers found.
ripgrep shows only packages/doom/package.json (lines 53–57) containing @RsPress deps and all are 2.0.0-beta.32.
24-26: Exports: verify barrel build output and export types for TS consumerstsconfig.base.json sets "moduleResolution": "NodeNext" (confirmed); "allowImportingTsExtensions" not found. Ensure the build emits lib/theme/index.js and lib/theme/index.d.ts. To surface types via subpath exports, consider:
"exports": { - "./theme": "./lib/theme/index.js", + "./theme": { + "types": "./lib/theme/index.d.ts", + "default": "./lib/theme/index.js" + }, "./types": "./lib/types.js", "./package.json": "./package.json" },
close IDP-1307
Summary by CodeRabbit
New Features
Documentation
Refactor
Chores