-
Notifications
You must be signed in to change notification settings - Fork 9
feat: 支持快手小程序 #114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: 支持快手小程序 #114
Conversation
WalkthroughThe pull request adds support for a new 'ks' mode across CLI shared utilities. The Mode type union is extended to include 'ks', and the SUPPORT_MODE array and MODE_CONFIG_FILES_MAP are updated to register this new mode with its associated configuration file. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/cli-shared-utils/lib/index.js(1 hunks)packages/cli-shared-utils/lib/target.js(2 hunks)
🔇 Additional comments (1)
packages/cli-shared-utils/lib/target.js (1)
11-11: LGTM – 'ks' mode support correctly added.The addition of 'ks' (Kuaishou) to SUPPORT_MODE and its configuration file mapping to 'project.config.json' is consistent with other mini-program platforms (wx, qq, tt, dd). The placement maintains logical grouping of platform types.
Also applies to: 23-23
|
|
||
| /** | ||
| * @typedef { 'wx' | 'ali' | 'swan' | 'qq' | 'tt' | 'dd' | 'web' | 'android' | 'ios' } Mode | ||
| * @typedef { 'wx' | 'ali' | 'swan' | 'qq' | 'tt' | 'dd' | 'ks' | 'web' | 'android' | 'ios' } Mode |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion | 🟠 Major
Typedef is incomplete – add missing modes for consistency.
While adding 'ks' is correct, the Mode typedef is missing 'tenon' and 'harmony' which are present in SUPPORT_MODE (target.js line 11). This can cause type mismatches in TypeScript-aware tooling.
Apply this diff to complete the typedef:
-/**
- * @typedef { 'wx' | 'ali' | 'swan' | 'qq' | 'tt' | 'dd' | 'ks' | 'web' | 'android' | 'ios' } Mode
- * @typedef { { mode: Mode, env: 'development' | 'production' } } Target
- */
+/**
+ * @typedef { 'wx' | 'ali' | 'swan' | 'qq' | 'tt' | 'dd' | 'ks' | 'web' | 'tenon' | 'android' | 'ios' | 'harmony' } Mode
+ * @typedef { { mode: Mode, env: 'development' | 'production' } } Target
+ */Committable suggestion skipped: line range outside the PR's diff.
🤖 Prompt for AI Agents
In packages/cli-shared-utils/lib/index.js around line 3, the JSDoc typedef for
Mode is missing the 'tenon' and 'harmony' values that exist in SUPPORT_MODE
(target.js line 11); update the typedef to include 'tenon' and 'harmony' so it
reads with all supported modes (wx, ali, swan, qq, tt, dd, ks, tenon, harmony,
web, android, ios) ensuring TypeScript-aware tooling and type checks see the
full set.
Summary by CodeRabbit
Release Notes