-
Notifications
You must be signed in to change notification settings - Fork 273
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
chore(themes): output multiple themes #3020
Conversation
Walkthrough本次变更主要针对构建 CSS 的脚本及相关文档。移除了原有的 Changes
Sequence Diagram(s)sequenceDiagram
participant BuildScript
participant SCSSHandler
participant FileSystem
BuildScript->>SCSSHandler: 调用 generateAllStyles()
alt projectID 存在
SCSSHandler->>FileSystem: 动态导入项目相关变量文件
else projectID 不存在
SCSSHandler->>FileSystem: 导入默认变量文件
end
SCSSHandler->>BuildScript: 返回生成的 SCSS 导入列表
sequenceDiagram
participant BuildScript
participant ThemeCollector
participant FileSystem
BuildScript->>ThemeCollector: 调用 buildThemeCSS()
alt projectID 存在
ThemeCollector->>FileSystem: 收集项目主题文件(默认及暗黑主题)
else projectID 不存在
ThemeCollector->>FileSystem: 收集默认主题文件
end
ThemeCollector->>BuildScript: 返回动态命名的主题文件
Possibly related PRs
Suggested reviewers
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Pro Plan! Simply use the command ✨ Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
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 (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## feat_v3.x #3020 +/- ##
==========================================
Coverage 85.82% 85.82%
==========================================
Files 281 281
Lines 18494 18494
Branches 2803 2803
==========================================
Hits 15873 15873
Misses 2616 2616
Partials 5 5 ☔ View full report in Codecov by Sentry. |
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: 0
🧹 Nitpick comments (4)
scripts/build-taro.mjs (1)
271-289
: 主题配置支持多项目,但需检查扩展性这里通过
glob
搜索theme-*.scss
并根据projectID
映射到default
或dark
。若后续需要支持更多主题名称或维度(例如light
,blue
, 等),建议预留扩展性,或增加错误提示(例如对应文件未找到时提示)。scripts/build.mjs (3)
219-241
: 确认多主题需求的场景,避免丢弃其他主题文件。
当前逻辑仅在不存在projectID
时,全部主题文件都会被编译;当存在projectID
时,则只编译默认主题和暗黑主题(如果文件名匹配<projectID>
和<projectID>-dark
)。这可能会忽略其他并存的主题文件,建议确认这种单一场景下的处理逻辑能否满足全部业务需求,或考虑允许一次生成多个定制主题。
255-255
: 注意输出文件重名冲突风险。
将所有打包出的文件都命名为[name].css
可能在多项目场景下出现文件名冲突,建议在必要时添加哈希或额外前缀后缀以避免覆盖。- assetFileNames: '[name].css', + assetFileNames: '[name].[hash].css',
384-385
: 将 Lottie 资源重复拷贝到 es 与 cjs 目录,可能增加发布包体积。
此操作会导致相同的动画资源在 es 与 cjs 两个目录均占空间,若资源体积较大,建议考虑共用同一路径或单独拆分到可共享的公共目录,再按需引用。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
scripts/build-taro.mjs
(5 hunks)scripts/build.mjs
(6 hunks)scripts/replace-css-var.js
(1 hunks)src/packages/configprovider/doc.en-US.md
(1 hunks)src/packages/configprovider/doc.md
(1 hunks)src/packages/configprovider/doc.taro.md
(1 hunks)src/packages/configprovider/doc.zh-TW.md
(1 hunks)src/styles/theme-dark.scss
(1 hunks)src/styles/theme-default.scss
(1 hunks)
✅ Files skipped from review due to trivial changes (5)
- src/styles/theme-default.scss
- src/packages/configprovider/doc.zh-TW.md
- src/styles/theme-dark.scss
- src/packages/configprovider/doc.md
- src/packages/configprovider/doc.en-US.md
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: test
🔇 Additional comments (7)
scripts/replace-css-var.js (1)
25-25
: 移除字体引入需谨慎验证当前通过两次
replace
将jd-font
的导入语句去除,确保最终编译后的主题内容不包含jd-font
。请确认对应字体已在其他地方正确引入或已不再使用,避免造成可见的样式缺失或字体问题。src/packages/configprovider/doc.taro.md (1)
32-32
: 文档路径更新已匹配最新主题文件格式文档从
.scss
文件改为.css
文件的引用,对使用者而言更直观简洁,且与多主题输出方案保持一致,无需额外处理。scripts/build-taro.mjs (2)
235-251
: 逻辑可行,但需关注文件存在性
generateAllStyles
函数内通过projectID
动态选择variables-${projectID}.scss
。在实际使用时,请验证该文件确实存在,并考虑在文件缺失时给予合理的异常处理或回退至默认变量文件。
298-298
: 多主题并行输出配置正常但需留意命名冲突使用
input: inputFiles
与[name].css
输出,会分别生成多个 CSS 文件。若后续同时存在多个关联主题并使用相似命名,可能产生命名冲突,须确保inputFiles
不会彼此覆盖。Also applies to: 302-302
scripts/build.mjs (3)
251-251
: 动态传入 Rollup 打包入口合理。
将可用主题文件映射给 Rollup 的input
对象能灵活地生成多份主题 CSS,符合新方案的多主题设计需求。
440-440
: 日志标记清晰可读。
console.timeEnd('Copy package.json readme.md')
仅用于标记时间消耗,不存在明显问题。
183-199
:❓ Verification inconclusive
建议对动态引入的 SCSS 文件进行存在性检查,避免打包出错。
如果项目 ID 对应的variables-${projectID}.scss
文件并不存在,Sass 编译阶段可能会报错,导致构建失败。建议在生成 import 语句之前先检查对应文件是否存在,并为不存在的情况提供降级处理或报错提示。下面的脚本可自动检查相关文件是否存在,如未发现指定的项目 ID 变量文件,则提示确认:
🏁 Script executed:
#!/bin/bash # Description: 检查 if styles/variables-${PROJECT_ID}.scss 是否存在 projectID="${VITE_APP_PROJECT_ID}" if [ -n "$projectID" ]; then filePath="${projectID}" echo "Checking existence of styles/variables-${filePath}.scss..." if ! fd "variables-${filePath}.scss" ./src/styles; then echo "Warning: styles/variables-${filePath}.scss could not be found. Please confirm." exit 1 fi fiLength of output: 28
建议对动态引入的 SCSS 文件进行存在性检查及环境变量的配置验证
当前构建逻辑会根据VITE_APP_PROJECT_ID
动态生成对应的变量 SCSS 文件引用,但从验证脚本的输出看,该环境变量为空,导致检查逻辑根本未执行。如果项目预期中会配置VITE_APP_PROJECT_ID
,建议在生成 import 语句之前先确认对应文件(如styles/variables-${VITE_APP_PROJECT_ID}.scss
)是否存在,并在文件缺失时提供备选方案或明确的错误提示。请检查环境变量的设置是否符合预期,并考虑在脚本中增加对默认情况(例如未设置项目 ID 时使用variables.scss
)的处理。
Summary by CodeRabbit