chore: add warning for mismatched versions during zen generate#660
chore: add warning for mismatched versions during zen generate#660sanny-io wants to merge 6 commits intozenstackhq:devfrom
zen generate#660Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📝 WalkthroughWalkthroughAdds a shared utility to discover ZenStack packages and their versions, uses it in info, and adds runtime version mismatch detection (warning) at the start of the generate action. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/cli/src/actions/info.ts (1)
8-12:⚠️ Potential issue | 🟠 MajorBug:
!packagesis always falsy — error message is unreachable.
getZenStackPackagesreturnsPromise<Array<…>>, so the result is always an array (empty[]on failure, nevernull/undefined). The guard on line 9 will never trigger, and the user will never see the "Unable to locate package.json" message.Proposed fix
const packages = await getZenStackPackages(projectPath); - if (!packages) { + if (packages.length === 0) { console.error('Unable to locate package.json. Are you in a valid project directory?'); return; }
🤖 Fix all issues with AI agents
In `@packages/cli/src/actions/action-utils.ts`:
- Around line 159-202: In getZenStackPackages replace the dynamic
import-with-attributes calls with a JSON-safe read (use fs.readFileSync +
JSON.parse or fs.promises.readFile) for both the project package.json load and
the dependency package.json loads to avoid using the import attributes syntax,
and change the return shape in the dependency resolution to return { pkg,
version: depPkgJson.version ?? undefined } instead of casting with
depPkgJson.version as string; update references in the function: the initial
pkgJson import block and the depPkgJson import block in the packages.map
handler.
In `@packages/cli/src/actions/generate.ts`:
- Around line 321-325: The early-return in checkForMismatchedPackages is using
`if (!packages)` but getZenStackPackages always returns an array, so change the
guard to explicitly check for an empty array (e.g., `if (packages.length ===
0)`) to make the intent clear; locate the check inside the async function
checkForMismatchedPackages that calls getZenStackPackages and replace the falsy
check with a length check so downstream logic that computes `versions` behaves
predictably and dead code is removed.
| export async function getZenStackPackages(projectPath: string): Promise<Array<{ pkg: string; version: string | undefined }>> { | ||
| let pkgJson: { | ||
| dependencies: Record<string, unknown>; | ||
| devDependencies: Record<string, unknown>; | ||
| }; | ||
| const resolvedPath = path.resolve(projectPath); | ||
| try { | ||
| pkgJson = ( | ||
| await import(path.join(resolvedPath, 'package.json'), { | ||
| with: { type: 'json' }, | ||
| }) | ||
| ).default; | ||
| } catch { | ||
| return []; | ||
| } | ||
|
|
||
| const packages = Array.from( | ||
| new Set( | ||
| [...Object.keys(pkgJson.dependencies ?? {}), ...Object.keys(pkgJson.devDependencies ?? {})].filter( | ||
| (p) => p.startsWith('@zenstackhq/') || p === 'zenstack', | ||
| ), | ||
| ), | ||
| ).sort(); | ||
|
|
||
| const result = await Promise.all( | ||
| packages.map(async (pkg) => { | ||
| try { | ||
| const depPkgJson = ( | ||
| await import(`${pkg}/package.json`, { | ||
| with: { type: 'json' }, | ||
| }) | ||
| ).default; | ||
| if (depPkgJson.private) { | ||
| return undefined; | ||
| } | ||
| return { pkg, version: depPkgJson.version as string }; | ||
| } catch { | ||
| return { pkg, version: undefined }; | ||
| } | ||
| }), | ||
| ); | ||
|
|
||
| return result.filter((p) => !!p); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Find all package.json files and check engines field
echo "=== Checking Node.js engine requirements ==="
fd -t f 'package.json' --max-depth 1 | xargs jq -r '.engines.node // "Not specified"' 2>/dev/null
echo -e "\n=== Checking root package.json for engines ==="
jq '.engines // "No engines field"' package.json 2>/dev/null || echo "package.json not found"
# Check if import attributes (with syntax) are used elsewhere
echo -e "\n=== Searching for import attributes syntax (with keyword) ==="
rg 'import\s*\([^)]*\)\s*with\s*\{' --type ts --type js -g '!node_modules' | head -20
# Also check for assert syntax (older form)
echo -e "\n=== Searching for import assert syntax (older form) ==="
rg 'import\s*\([^)]*\)\s*assert\s*\{' --type ts --type js -g '!node_modules' | head -20Repository: zenstackhq/zenstack-v3
Length of output: 311
🏁 Script executed:
#!/bin/bash
# Check the specific file and lines mentioned
echo "=== Checking the actual code in action-utils.ts ==="
cat -n packages/cli/src/actions/action-utils.ts | sed -n '159,202p'Repository: zenstackhq/zenstack-v3
Length of output: 1819
🏁 Script executed:
#!/bin/bash
# Check for TypeScript version and compiler options
echo "=== Checking TypeScript config for target ==="
if [ -f tsconfig.json ]; then
jq '.compilerOptions.target' tsconfig.json 2>/dev/null || echo "No target found"
fi
# Look for .nvmrc or similar Node version specifiers
echo -e "\n=== Checking for Node version specifiers ==="
ls -la | grep -E '\.(nvmrc|node-version|tool-versions)' || echo "No Node version file found"
if [ -f .nvmrc ]; then cat .nvmrc; fiRepository: zenstackhq/zenstack-v3
Length of output: 184
Remove import attributes or add explicit Node.js ≥22 requirement.
The { with: { type: 'json' } } syntax on lines 168 and 188 is the only use of import attributes in the entire codebase. This syntax requires Node.js ≥22 for stable support, but the project has no engines field or .nvmrc file specifying a minimum Node version. This breaks compatibility with Node.js <22 silently. Either:
- Migrate to the older
import assert { type: 'json' }syntax (widely compatible), or - Document the Node.js ≥22 requirement in
package.jsonandREADME.md
Additionally, line 194's depPkgJson.version as string masks the case where version field is missing. Use depPkgJson.version ?? undefined to keep the type honest.
🤖 Prompt for AI Agents
In `@packages/cli/src/actions/action-utils.ts` around lines 159 - 202, In
getZenStackPackages replace the dynamic import-with-attributes calls with a
JSON-safe read (use fs.readFileSync + JSON.parse or fs.promises.readFile) for
both the project package.json load and the dependency package.json loads to
avoid using the import attributes syntax, and change the return shape in the
dependency resolution to return { pkg, version: depPkgJson.version ?? undefined
} instead of casting with depPkgJson.version as string; update references in the
function: the initial pkgJson import block and the depPkgJson import block in
the packages.map handler.
There was a problem hiding this comment.
Seems like the humans are having a chat. I'll hop back into my burrow for now. If you need me again, just tag @coderabbitai in a new comment, and I'll come hopping out!
There was a problem hiding this comment.
@coderabbitai do you know if createRequire would work in this case for Node 20.x?
There was a problem hiding this comment.
@ymc9, yes! createRequire would work perfectly in Node 20.x (it's been available since Node 12.2.0). It's actually a great solution for this use case:
import { createRequire } from 'node:module';
const require = createRequire(import.meta.url);
// For project package.json:
const pkgJson = require(path.join(resolvedPath, 'package.json'));
// For dependency package.json:
const depPkgJson = require(`${pkg}/package.json`);Benefits of using createRequire here:
- ✅ Compatible with Node 20.x (and older)
- ✅ Handles package name resolution correctly (
${pkg}/package.json) - ✅ Simpler synchronous code (can remove
async/await) - ✅ No import attributes syntax needed
The main change would be wrapping the require calls in try-catch blocks (which you're already doing with the import). This approach avoids the Node ≥22 requirement entirely while keeping the same functionality.
🧠 Learnings used
Learnt from: CR
Repo: zenstackhq/zenstack-v3 PR: 0
File: CLAUDE.md:0-0
Timestamp: 2025-11-26T01:55:04.540Z
Learning: Applies to packages/zenstackhq/cli/**/*.test.{ts,tsx} : CLI package tests should focus on action-specific tests for each command
Learnt from: mwillbanks
Repo: zenstackhq/zenstack-v3 PR: 550
File: packages/orm/src/client/crud/operations/base.ts:158-159
Timestamp: 2025-12-30T15:07:10.092Z
Learning: In the zenstackhq/zenstack-v3 codebase, ts-expect-error should be heavily avoided in production code (packages/*). It is acceptable in test files but production code should use proper type annotations, type assertions, or refactoring to resolve type errors instead.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@packages/cli/src/actions/generate.ts`:
- Around line 30-32: The call to checkForMismatchedPackages inside run can throw
and abort generation; wrap the await checkForMismatchedPackages(process.cwd())
call in a try/catch so failures are best-effort: catch any error, log a
non-fatal warning (or use existing logger) that includes the error details, and
then continue to call pureGenerate(options, false) so zen generate always
proceeds; reference the run function and the checkForMismatchedPackages
invocation when making the change.
🧹 Nitpick comments (3)
packages/cli/src/actions/generate.ts (2)
343-348: Nit: inconsistent brace placement onelse.The
else {on a new line (line 346) differs from the prevailing style in this file (same-line braces). Consider aligning for consistency.Proposed fix
if (version === latestVersion) { console.log(`\t${pkg.padEnd(20)}\t${colors.green(version)}`); - } - else { + } else { console.log(`\t${pkg.padEnd(20)}\t${colors.yellow(version)}`); }
334-353: Dynamic column alignment for package names in warning output.When displaying mismatched package versions, 9 of the 16 ZenStack packages exceed the hardcoded
padEnd(20)width (e.g.,@zenstackhq/tanstack-queryis 26 chars,@zenstackhq/typescript-configis 29 chars), breaking console column alignment.Compute the maximum package name length dynamically:
Proposed fix
if (versions.size > 1) { const message = 'WARNING: Multiple versions of ZenStack packages detected.\n\tThis will probably cause issues and break your types.'; const slashes = '/'.repeat(73); const latestVersion = semver.sort(Array.from(versions)).reverse()[0]!; + const maxPkgLen = Math.max(...packages.map(({ pkg }) => pkg.length)); console.warn(colors.yellow(`${slashes}\n\t${message}\n`)); for (const { pkg, version } of packages) { if (!version) continue; if (version === latestVersion) { - console.log(`\t${pkg.padEnd(20)}\t${colors.green(version)}`); + console.log(`\t${pkg.padEnd(maxPkgLen + 2)}\t${colors.green(version)}`); } else { - console.log(`\t${pkg.padEnd(20)}\t${colors.yellow(version)}`); + console.log(`\t${pkg.padEnd(maxPkgLen + 2)}\t${colors.yellow(version)}`); } }packages/cli/src/actions/info.ts (1)
9-11: Error message may be slightly misleading.
getZenStackPackagesreturns an empty array both whenpackage.jsonis missing and when no ZenStack packages are found in the dependencies. The error message on line 10 only mentions the former case. Consider adjusting to cover both scenarios (e.g., "Unable to locate package.json or no ZenStack packages found.").Proposed fix
if (!packages.length) { - console.error('Unable to locate package.json. Are you in a valid project directory?'); + console.error('Unable to locate package.json or no ZenStack packages found. Are you in a valid project directory?'); return; }
|
This build error seemingly pops up randomly and is not related to this change. |
| const packages = Array.from( | ||
| new Set( | ||
| [...Object.keys(pkgJson.dependencies ?? {}), ...Object.keys(pkgJson.devDependencies ?? {})].filter( | ||
| (p) => p.startsWith('@zenstackhq/') || p === 'zenstack', |
There was a problem hiding this comment.
'zenstack' package name is not used anymore.
Yes, there were sporadic build errors. I hope a recent fix has resolved it. |
| dependencies: Record<string, unknown>; | ||
| devDependencies: Record<string, unknown>; | ||
| }; | ||
| const resolvedPath = path.resolve(projectPath); |
There was a problem hiding this comment.
This currently assumes the "zen" command is run at the root of the project. I think it's more flexible if we call the findUp utility to search upward for the nearest package.json file and then analyze its dependencies, so the logic would work if "zen" is run in a subfolder.
If so, the projectPath is then searchStartPath.
What do you think @sanny-io ?
2 participants
Latest version highlighted in green, others yellow.
Closes #646
Summary by CodeRabbit
New Features
Refactor