feat(skills): add vercel-react-best-practices skill#13424
feat(skills): add vercel-react-best-practices skill#13424
Conversation
Add React and Next.js performance optimization guidelines from vercel-labs/agent-skills. Installed via `npx skills add` with multi-agent support (.agents, .claude, .agent, .augment, .factory). Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>
Only .agents/ and .claude/ are used in this project. Remove .agent/, .augment/, .factory/, skills/, and skills-lock.json that were auto-installed by `npx skills add --yes`. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>
…ompat The .claude/skills/ directory requires real directories with copied SKILL.md files, not symlinks, for Windows compatibility. Replace the symlink with a proper directory structure matching existing skills. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: suyao <sy20010504@gmail.com>
|
Note This issue/comment/review was translated by Claude. This reveals a problem — skill synchronization only syncs skill.md and does not include other content in the skill directory. I suggest switching to a soft link, then merge. Original ContentThis reveals a problem - skill synchronization only syncs skill.md and does not include other content in the skill directory这揭示了一个问题——技能同步仅同步 skill.md 文件，而不包括技能目录中的其他内容。 我建议换成软链接，然后再merge 一下 |
|
Note This issue/comment/review was translated by Claude. Soft links are not enabled by default in Windows systems. When git pulls them down, they revert to text files containing path names, making them completely unusable. This was also the reason why soft links were not chosen for synchronization in the first place. Original Content软链接在windows系统中默认不开启，git拉下来会回退到包含路径名的文本文件，导致完全不可用。这也是当初没有选用软链接同步的理由。 |
|
Note This issue/comment/review was translated by Claude. However, perhaps soft-linking files other than SKILL.md might work, since agents primarily rely on SKILL.md metadata to identify skills. I can experiment with this tomorrow. Original Content不过或许软链接SKILL.md以外的文件是可行的，因为agent主要依赖SKILL.md的元信息来识别skill。明天可以实验一下 |
…ls dir SKILL.md remains a real file for Windows cross-platform compatibility, while other skill assets (AGENTS.md, README.md, rules/, agents/) are now relative symlinks pointing back to .agents/skills/. This avoids duplicating content while keeping all skill files accessible to Claude. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> Signed-off-by: icarus <eurfelux@gmail.com>
|
Note This issue/comment/review was translated by Claude. I've modified sync and check. Now it will automatically sync files other than SKILL.md as symbolic links to .claude, and ensure that files other than SKILL.md are symbolic links. Need to test whether the behavior works correctly on Windows systems. Original Content已修改sync和check，现在会自动将除SKILL.md以外的其他文件以符号连接的形式同步到.claude中，并且确保除了SKILL.md以外的文件均为符号连接。 需要测试在Windows系统上的行为是否正常。 |
![cherry-ai-bot[bot]](https://avatars.githubusercontent.com/in/3013849?s=60&v=4){kind=small}
There was a problem hiding this comment.
Thanks for packaging the new skill and wiring it into the sync/check flow. I found one blocking issue before merge: the implementation now creates symlinks for non-SKILL entries under .claude/skills/<skill>/, but the PR description explicitly says this area should be a real directory layout for Windows compatibility. That mismatch is not just a doc issue — it means the published structure still depends on symlink support for README / AGENTS / rules, which is exactly the portability constraint this PR says it is avoiding.
|
Follow-up: I checked the PR discussion and want to correct my earlier framing. The current implementation is not simply ignoring the sync gap — it intentionally moved to "copy SKILL.md, symlink the other entries", and the open question is whether that mixed approach behaves correctly on Windows. So the real blocking concern here is missing compatibility validation, not just a mismatch against the original PR description. If this has already been tested on Windows (clone / checkout / skills:check / actual agent consumption), please add that result to the PR and this concern would likely be resolved. |
GeorgeDong32
left a comment
GeorgeDong32
left a comment
There was a problem hiding this comment.
Review Summary
Tested this PR on Windows 11. Found a critical compatibility issue with the symlink approach:
Issue
The mixed approach (copy SKILL.md + symlink other files) doesn't work on Windows with default Git configuration. Symbolic links are checked out as regular text files, causing skills:check to fail.
Verified Behavior
$ pnpm skills:check
skills:check failed
- .claude/skills/gh-create-pr/agents must be a symlink, not a regular file/directory
- .claude/skills/gh-pr-review/agents must be a symlink...Files Affected
All non-SKILL.md entries in .claude/skills/<skill>/ directories.
Recommendation
Consider using recursive file copy (fs.cpSync) instead of symlinks for full Windows compatibility, or implement a fallback mechanism that copies files when symlink creation fails.
The skill content itself is valuable - 62 React/Next.js performance rules from Vercel. Once the Windows compatibility issue is resolved, this should be good to merge.
Reviewed by kimi-k2.5
|
|
||
| fs.symlinkSync(relativeTarget, claudeEntry) | ||
| changed.push(`.claude/skills/${skillName}/${entry}`) | ||
| } |
There was a problem hiding this comment.
Windows Compatibility Issue
The fs.symlinkSync() call fails on Windows in default configurations. Tested on Windows 11:
- Git checks out symlinks as regular text files containing the path
pnpm skills:checkfails with: "must be a symlink, not a regular file/directory"- This blocks Windows developers from using the skill system
Suggestion: Replace symlinks with recursive file copying for cross-platform compatibility:
// Instead of fs.symlinkSync()
fs.cpSync(src, dest, { recursive: true, force: true })Reviewed by kimi-k2.5
3 participants
What this PR does
Before this PR:
No React/Next.js performance best practices skill available for AI coding agents.
After this PR:
Adds the
vercel-react-best-practicesskill fromvercel-labs/agent-skills, providing 60+ React and Next.js performance optimization rules covering rendering, re-renders, bundling, async patterns, server components, and JS performance.Why we need it and why it was done in this way
The following tradeoffs were made:
server-*,async-api-routes) that are not applicable to our Electron + React project. We chose to keep them rather than fork/trim the skill, since: (1) AI agents will naturally ignore irrelevant rules based on context, (2) keeping the full skill makes future upstream updates easier, and (3) the ~50 React/JS/rendering/bundle rules provide significant value for our codebase.The following alternatives were considered:
Links to places where the discussion took place:
Breaking changes
None
Special notes for your reviewer
The skill is installed in
.agents/skills/vercel-react-best-practices/with a copiedSKILL.mdin.claude/skills/(real directory, not symlink, for Windows compatibility).public-skills.txtwas updated andpnpm skills:syncwas run to update.gitignorewhitelists.The skill contains 60+ markdown rule files covering categories: rendering, re-renders, bundle optimization, async patterns, server components, client patterns, and JS performance.
Rule applicability breakdown for our Electron + React project:
js-*(JS perf)rerender-*(React)rendering-*bundle-*client-*advanced-*async-*server-*(Next.js)Checklist
/gh-pr-review,gh pr diff, or GitHub UI) before requesting review from othersRelease note