fix: check when expression correctly
#69
Conversation
🦋 Changeset detectedLatest commit: d7bcd12 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 |
WalkthroughA patch changeset was added for the "@alauda/doom" package to document a fix involving the Changes
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This PR addresses the incorrect handling of the when expression in the CLI command and makes updates to dependency management in package.json.
- Update null check logic for the
whenexpression in the CLI command (src/cli/new.ts). - Adjust the version range for @cspell/eslint-plugin in package.json.
- Move the TypeScript dependency from devDependencies to dependencies in package.json.
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/cli/new.ts | Refines the when condition to first ensure a non-null value before evaluation. |
| package.json | Updates dependency version ranges and adjusts the placement of the TypeScript dependency. |
Comments suppressed due to low confidence (2)
package.json:101
- Verify that moving the 'typescript' dependency from devDependencies to dependencies is intentional, to ensure it is included only when appropriate.
"typescript": "^5.8.3",
package.json:60
- Confirm that the updated version range for '@cspell/eslint-plugin' meets your compatibility requirements and does not introduce regressions.
"@cspell/eslint-plugin": "^8.19.4 || ^9.0.2",
commit: |
Signed-off-by: JounQin <admin@1stg.me>
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
📜 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 (3)
.changeset/proud-masks-kiss.md(1 hunks)package.json(2 hunks)src/cli/new.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/cli/new.ts (1)
src/shared/constants.ts (1)
JS_STR_FALSY_VALUES(17-25)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: Build and Lint with Node.js 24 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 22 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 18 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 20 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 18 on ubuntu-latest
🔇 Additional comments (3)
package.json (1)
60-60:Details
✅ Verification successful
Verify the version range broadening for @cspell/eslint-plugin.
The dependency now accepts both
^8.19.4and^9.0.2, which is a significant version range. Ensure this change doesn't introduce compatibility issues between the different major versions.
🏁 Script executed:
#!/bin/bash # Check for breaking changes between @cspell/eslint-plugin versions 8.x and 9.x curl -s https://registry.npmjs.org/@cspell/eslint-plugin | jq '.versions | keys[]' | grep -E '^"(8\.|9\.)'Length of output: 681
To verify whether v9 introduced any breaking changes, let’s pull the changelog for the 9.0.0 release:
🏁 Script executed:
#!/bin/bash set -e # 1. Get the package repo URL repo=$(curl -s https://registry.npmjs.org/@cspell/eslint-plugin \ | jq -r '.repository.url' \ | sed -e 's/^git+//' -e 's/\.git$//') # 2. Try both common default branches for branch in main master; do raw_url=$(echo "$repo" \ | sed 's#github.com#raw.githubusercontent.com#')"/$branch/CHANGELOG.md" echo "Fetching from $raw_url…" >&2 if data=$(curl -fsS "$raw_url"); then echo "$data" \ | sed -n '/^## \[9\.0\.0\]/,/^## /p' exit 0 fi done echo "⚠️ Could not fetch 9.0.0 changelog – please check the repository’s default branch or file path." >&2 exit 1Length of output: 66222
@cspell/eslint-plugin v8 → v9 Compatibility Confirmed
I checked the v9.0.0 release notes and it only includes a version bump for the monorepo (no API or behavior changes), so widening the range to^8.19.4 ‖ ^9.0.2poses no compatibility risk.src/cli/new.ts (1)
224-226: Excellent fix for thewhenexpression logic!This change correctly addresses a logical bug in the condition evaluation. The previous logic
when && JS_STR_FALSY_VALUES.has(when)had a flaw: it would only check the falsy values set ifwhenwas truthy, but values likenull,undefined, and''are both falsy AND present in theJS_STR_FALSY_VALUESset.The new logic
when != null && JS_STR_FALSY_VALUES.has(when)properly:
- Treats
null/undefinedas "no condition specified" (process the layout)- Only skips processing when
whenhas a defined value that evaluates to falsy (like'false','0','', etc.)This ensures that scaffolding layouts are processed correctly based on their conditional expressions.
.changeset/proud-masks-kiss.md (1)
1-6: Well-documented changeset for the fix.The changeset appropriately documents this as a patch-level fix with a clear description of what was corrected.
Summary by CodeRabbit
Bug Fixes
Chores