fix(packages/ui,packages/cli): fix FileTree build failure and add --ref to diff#58
fix(packages/ui,packages/cli): fix FileTree build failure and add --ref to diff#58zrosenbauer merged 6 commits intomainfrom
Conversation
…ef to diff Bug 1: Pin rspress-plugin-file-tree to 1.0.3 (1.0.4 shipped without dist/) and copy its component files into @zpress/ui dist so Rspress webpack can resolve them at build time. Bug 2: Add --ref option to zpress diff that uses git diff instead of git status, enabling use as a Vercel ignoreCommand. When --ref is set and changes are detected, exits 1 to signal "proceed with build". Also upgrades all dependencies to latest (except mermaid, pinned to v10). Co-Authored-By: Claude <noreply@anthropic.com>
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
🦋 Changeset detectedLatest commit: feb37be The changes in this PR will be included in the next version bump. This PR includes changesets to release 3 packages
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 Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds a Changes
Sequence Diagram(s)sequenceDiagram
participant User as User/Invoker
participant CLI as zpress CLI
participant Git as Git
participant Node as Node Process
rect rgba(100,150,240,0.5)
User->>CLI: run `zpress diff --ref <ref>` or `zpress diff`
end
CLI->>Git: if --ref -> `git diff --name-only <ref> HEAD` \n else -> `git status --short`
Git-->>CLI: stdout (list of changed files or empty)
CLI->>CLI: filter files by configured directories
alt changes found
CLI->>User: print changed files (pretty or space-separated)
CLI->>Node: if --ref -> process.exit(1)
else no changes
CLI->>User: print "no changes"
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
Co-Authored-By: Claude <noreply@anthropic.com>
Replace forbidden ternary expression with ts-pattern match() to select between gitDiffFiles and gitChangedFiles. Also disable prefer-destructuring lint rule for raw-copied MermaidRenderer component. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/cli/src/commands/diff.ts (1)
315-318: Consider sharing file-list parsing logic between git helpers.
gitChangedFilesandgitDiffFilesboth split/filter lines; extracting a small shared parser would reduce drift risk.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/diff.ts` around lines 315 - 318, gitChangedFiles and gitDiffFiles both parse git output by splitting on '\n' and filtering empty lines; extract this into a small shared helper (e.g., parseGitOutputLines or parseFileList) and use it in both functions to avoid duplication and drift. Update the implementations of gitChangedFiles and gitDiffFiles to call the new helper instead of repeating .split('\n').filter(...) and ensure the helper returns the same array shape currently returned so the callers' return tuple ([null, files]) behavior remains unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/cli/src/commands/diff.ts`:
- Around line 85-87: The ternary used to assign [gitErr, changed] causes an
ESLint "no-ternary" failure; replace it with an if/else assignment: declare let
gitErr, changed; then if (ref) call gitDiffFiles({ repoRoot: paths.repoRoot,
dirs, ref }) and assign its result to [gitErr, changed], else call
gitChangedFiles({ repoRoot: paths.repoRoot, dirs }) and assign to [gitErr,
changed]; keep the same variable names (gitErr, changed) and same call sites
(gitDiffFiles, gitChangedFiles) so behavior is unchanged.
---
Nitpick comments:
In `@packages/cli/src/commands/diff.ts`:
- Around line 315-318: gitChangedFiles and gitDiffFiles both parse git output by
splitting on '\n' and filtering empty lines; extract this into a small shared
helper (e.g., parseGitOutputLines or parseFileList) and use it in both functions
to avoid duplication and drift. Update the implementations of gitChangedFiles
and gitDiffFiles to call the new helper instead of repeating
.split('\n').filter(...) and ensure the helper returns the same array shape
currently returned so the callers' return tuple ([null, files]) behavior remains
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9796fa35-200b-45ac-aea8-d427543a6a9a
📒 Files selected for processing (1)
packages/cli/src/commands/diff.ts
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/src/commands/diff.ts (1)
310-314: Consider protecting against refs that look like git options.If a user passes a ref starting with
-(e.g., a branch named-featureor accidentally--help), git will interpret it as an option since it appears before the--delimiter. While rare and low-severity (the user controls their own input), this could cause unexpected behavior.🛡️ Optional: prefix ambiguous refs
function gitDiffFiles(params: { readonly repoRoot: string readonly dirs: readonly string[] readonly ref: string }): Result<readonly string[]> { + // Prefix refs starting with '-' to prevent git option interpretation + const safeRef = params.ref.startsWith('-') ? `refs/heads/${params.ref}` : params.ref const [err, output] = execSilent({ file: 'git', - args: ['diff', '--name-only', params.ref, 'HEAD', '--', ...params.dirs], + args: ['diff', '--name-only', safeRef, 'HEAD', '--', ...params.dirs], cwd: params.repoRoot, })Alternatively, validate and reject refs starting with
-at the option parsing level with a zod refinement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/commands/diff.ts` around lines 310 - 314, The git ref passed in params.ref can start with '-' and be misinterpreted as a git option; before calling execSilent (the call building args ['diff','--name-only', params.ref, 'HEAD','--', ...params.dirs]), validate params.ref and reject or normalize any ref that begins with '-' (e.g., throw/return a clear error asking for an explicit ref like refs/heads/<name> or require a non-leading-dash ref), or add a zod refinement at input parsing to enforce this; update the check near the execSilent invocation in diff.ts so the args array is never constructed with an ambiguous params.ref.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/cli/src/commands/diff.ts`:
- Around line 310-314: The git ref passed in params.ref can start with '-' and
be misinterpreted as a git option; before calling execSilent (the call building
args ['diff','--name-only', params.ref, 'HEAD','--', ...params.dirs]), validate
params.ref and reject or normalize any ref that begins with '-' (e.g.,
throw/return a clear error asking for an explicit ref like refs/heads/<name> or
require a non-leading-dash ref), or add a zod refinement at input parsing to
enforce this; update the check near the execSilent invocation in diff.ts so the
args array is never constructed with an ambiguous params.ref.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 74ace88d-3cfa-4460-ad2b-8974f5fabcfc
📒 Files selected for processing (2)
.oxlintrc.jsonpackages/cli/src/commands/diff.ts
✅ Files skipped from review due to trivial changes (1)
- .oxlintrc.json
When --verbose is passed, the build check skips output capture so Rspress/Rspack errors are printed directly to stdout/stderr. This makes it easier to diagnose build failures on CI. Co-Authored-By: Claude <noreply@anthropic.com>
There was a problem hiding this comment.
🧹 Nitpick comments (1)
packages/cli/src/lib/check.ts (1)
86-96: Consider updating JSDoc to mention verbose behavior.The JSDoc documents the output-capture behavior but doesn't mention that when
verboseis true, the capture is skipped and deadlink parsing won't occur. This is a minor documentation improvement.📝 Suggested JSDoc update
/** * Run a silent Rspress build to detect deadlinks. * * Rspress's `remarkLink` plugin checks internal links during build. In * production mode it logs colored error messages per file via `logger.error()` * then throws `Error("Dead link found")`. We capture stderr to parse the * diagnostics and present them in our own clean format. * + * When `verbose` is true, output capture is skipped and Rspress/Rspack + * writes directly to stdout/stderr. Deadlink parsing is not performed in + * this mode — the result will be `passed` or a generic `error`. + * * `@param` params - Config and paths for the build * `@returns` A `BuildCheckResult` with pass/fail status and any deadlinks found */🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/cli/src/lib/check.ts` around lines 86 - 96, Update the JSDoc for the function that performs the silent Rspress build (the one returning BuildCheckResult and described as "Run a silent Rspress build to detect deadlinks") to explicitly state that when the verbose parameter is true the stderr/stdout capture is skipped and deadlink parsing is not performed; mention the verbose flag behavior and its effect on diagnostics so callers know parsing only occurs when verbose is false.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@packages/cli/src/lib/check.ts`:
- Around line 86-96: Update the JSDoc for the function that performs the silent
Rspress build (the one returning BuildCheckResult and described as "Run a silent
Rspress build to detect deadlinks") to explicitly state that when the verbose
parameter is true the stderr/stdout capture is skipped and deadlink parsing is
not performed; mention the verbose flag behavior and its effect on diagnostics
so callers know parsing only occurs when verbose is false.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 38dccd2c-899f-4f8a-aa27-eb9016531630
📒 Files selected for processing (2)
packages/cli/src/commands/build.tspackages/cli/src/lib/check.ts
The 1.0.4 package does ship dist/ — the earlier missing files were from a stale pnpm store. The copy rule in rslib.config.ts is the actual fix. Co-Authored-By: Claude <noreply@anthropic.com>
Copy rspress-plugin-file-tree chunk files (0~*.js) to dist root so FileTree.js relative imports resolve correctly. Exclude index.* and components/** to avoid overwriting our own build output. Co-Authored-By: Claude <noreply@anthropic.com>
Summary
zpress buildfails becauserspress-plugin-file-tree@1.0.4shipped without itsdist/directory. Pin to1.0.3and copy FileTree component files into@zpress/ui/dist/components/so Rspress webpack resolves them at build time.zpress diffusesgit statuswhich always returns empty on clean CI checkouts (e.g. Vercel). Add--refoption that usesgit diff --name-only <ref> HEADinstead, with exit code 1 when changes are detected (Vercel ignoreCommand convention).Changes
packages/ui/package.json— pinrspress-plugin-file-treeto1.0.3, upgrade depspackages/ui/rslib.config.ts— copy FileTree component files from plugin into distpackages/cli/src/commands/diff.ts— add--refoption withgit diffand exit code semanticspnpm-workspace.yaml— catalog version bumps (typescript, vitest, tsgo)@zpress/uipatch,@zpress/climinorTest plan
pnpm buildpasses (verifies FileTree files are copied)npm pack --dry-runon@zpress/uishowsdist/components/FileTree/fileszpress diff --ref HEAD^detects changes and exits 1zpress diff --ref HEADwith no changes exits 0zpress diff(no --ref) still works as before withgit statusSummary by CodeRabbit
New Features
Bug Fixes
Chores