feat: ignore字体集的生成文件,并在编译时自动生成#8264
Conversation
There was a problem hiding this comment.
Hey - I've found 1 issue, and left some high level feedback:
- The
subset-mdi-font.mjsinvocation is duplicated across multiple scripts; consider extracting it into a separate npm script or usingpredev/prebuildlifecycle hooks to keep the scripts DRY and easier to maintain. - Running
subset-mdi-font.mjsbeforevite previewmay be redundant if preview is only serving the build output; you might want to restrict font generation todevand build-related scripts to avoid unnecessary work in preview.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- The `subset-mdi-font.mjs` invocation is duplicated across multiple scripts; consider extracting it into a separate npm script or using `predev`/`prebuild` lifecycle hooks to keep the scripts DRY and easier to maintain.
- Running `subset-mdi-font.mjs` before `vite preview` may be redundant if preview is only serving the build output; you might want to restrict font generation to `dev` and build-related scripts to avoid unnecessary work in preview.
## Individual Comments
### Comment 1
<location path="dashboard/package.json" line_range="7-12" />
<code_context>
+ "dev": "node scripts/subset-mdi-font.mjs && vite --host",
</code_context>
<issue_to_address>
**suggestion:** Avoid duplicating the subset-mdi invocation across multiple scripts by leveraging npm pre-scripts or a shared script.
The `node scripts/subset-mdi-font.mjs` call is now duplicated in several scripts (`dev`, `build*`, `preview`). To keep this DRY and easier to maintain, consider using npm lifecycle hooks (e.g. `prebuild`, `prebuild-stage`, etc.) or a shared script like `"subset-icons"` that other scripts invoke (e.g. `npm run subset-icons && vite ...`). This avoids inconsistent updates if the subset command changes later.
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
| "dev": "node scripts/subset-mdi-font.mjs && vite --host", | ||
| "build:t2i-shiki-runtime": "node scripts/build-t2i-shiki-runtime.mjs", | ||
| "build": "vue-tsc --noEmit && vite build", | ||
| "build-stage": "vue-tsc --noEmit && vite build --base=/vue/free/stage/", | ||
| "build-prod": "vue-tsc --noEmit && vite build --base=/vue/free/", | ||
| "preview": "vite preview --port 5050", | ||
| "build": "node scripts/subset-mdi-font.mjs && vue-tsc --noEmit && vite build", | ||
| "build-stage": "node scripts/subset-mdi-font.mjs && vue-tsc --noEmit && vite build --base=/vue/free/stage/", | ||
| "build-prod": "node scripts/subset-mdi-font.mjs && vue-tsc --noEmit && vite build --base=/vue/free/", | ||
| "preview": "node scripts/subset-mdi-font.mjs && vite preview --port 5050", |
There was a problem hiding this comment.
suggestion: Avoid duplicating the subset-mdi invocation across multiple scripts by leveraging npm pre-scripts or a shared script.
The node scripts/subset-mdi-font.mjs call is now duplicated in several scripts (dev, build*, preview). To keep this DRY and easier to maintain, consider using npm lifecycle hooks (e.g. prebuild, prebuild-stage, etc.) or a shared script like "subset-icons" that other scripts invoke (e.g. npm run subset-icons && vite ...). This avoids inconsistent updates if the subset command changes later.
There was a problem hiding this comment.
Code Review
This pull request automates the Material Design Icons (MDI) subset generation by integrating the generation script into the project's build and development workflows within package.json. It also updates .gitignore to exclude the generated assets. Feedback suggests that the dev script lacks Hot Module Replacement (HMR) for new icons, which may lead to confusion unless a Vite plugin is used or the server is restarted. Additionally, running the subset script during the preview command is identified as redundant since that command serves already built assets from the dist directory.
| "scripts": { | ||
| "dev": "vite --host", | ||
| "subset-icons": "node scripts/subset-mdi-font.mjs", | ||
| "dev": "node scripts/subset-mdi-font.mjs && vite --host", |
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Modifications / 改动点
优化:MDI 图标字体按需裁剪并自动生成
将
subset-mdi-font.mjs脚本从手动触发改为构建流程自动执行,同时排除生成文件避免污染仓库。改动内容:
package.jsondev/build/build-stage/build-prod/preview五个脚本均加入node scripts/subset-mdi-font.mjs前置步骤src/assets/mdi-subset/.gitignoresrc/assets/mdi-subset/规则,忽略脚本生成的 woff2 / woff / css 文件仓库状态
git rm -r --cached src/assets/mdi-subset/将已追踪的字体文件移出暂存区,避免重复提交生成文件效果:
每次
pnpm dev/pnpm build时自动生成最新的图标子集,无需手动运行subset-icons生成文件不进入 git,仓库保持干净
This is NOT a breaking change. / 这不是一个破坏性变更。
Screenshots or Test Results / 运行截图或测试结果
验证步骤:
pnpm dev,观察控制台输出是否包含字体裁剪日志:Checklist / 检查清单
😊 If there are new features added in the PR, I have discussed it with the authors through issues/emails, etc.
/ 如果 PR 中有新加入的功能,已经通过 Issue / 邮件等方式和作者讨论过。
👀 My changes have been well-tested, and "Verification Steps" and "Screenshots" have been provided above.
/ 我的更改经过了良好的测试,并已在上方提供了"验证步骤"和"运行截图"。
🤓 I have ensured that no new dependencies are introduced, OR if new dependencies are introduced, they have been added to the appropriate locations in
requirements.txtandpyproject.toml./ 我确保没有引入新依赖库,或者引入了新依赖库的同时将其添加到
requirements.txt和pyproject.toml文件相应位置。😮 My changes do not introduce malicious code.
/ 我的更改没有引入恶意代码。
Summary by Sourcery
Automate generation of the MDI icon font subset during dashboard dev and build workflows and stop tracking the generated assets in version control.
New Features:
Enhancements:
Chores: