Skip to content

fix: rename skill dirs to match frontmatter, surface React Compiler caveat, fix listener/guidelines examples - #315

Open
SomSamantray wants to merge 6 commits into
vercel-labs:mainfrom
SomSamantray:fix/top-4-verified-bugs
Open

fix: rename skill dirs to match frontmatter, surface React Compiler caveat, fix listener/guidelines examples#315
SomSamantray wants to merge 6 commits into
vercel-labs:mainfrom
SomSamantray:fix/top-4-verified-bugs

Conversation

@SomSamantray

Copy link
Copy Markdown

Summary

Four independently-verified, still-open bugs, bundled since each is small and none depend on the others except through the directory rename:

  • react-best-practices directory name doesn't match frontmatter name #80skills/react-best-practices, composition-patterns, react-native-skills, and react-view-transitions don't match their SKILL.md frontmatter name: (agentskills.io spec requires directory name == frontmatter name). The frontmatter names are already the canonical cross-reference ID used throughout vercel-optimize's docs/tests/scanner and skills.sh.json, so the 4 directories are renamed to match (not the reverse). Also fixes packages/react-best-practices-build's config/scripts, the CI workflow's path filters, a stale directory-name reference in vercel-react-view-transitions/README.md, and the top-level README.md's skill-listing headers.
  • useMemo was suggested in spite of my config that I'm using react-compiler #87 — The React Compiler caveat ("manual memo()/useMemo() isn't necessary when React Compiler is enabled") was buried in a trailing note on one rule file. Promoted to a "Pre-flight Checks" section in SKILL.md plus a prominent "Skip if" callout on the 3 rules it actually affects (the other 12 rerender-* rules are unrelated to React Compiler and untouched).
  • useSWRSubscription example for deduplicating global DOM event listeners may be misleading #68client-event-listeners.md's example misused useSWRSubscription() (a data-subscription hook) for a DOM side-effect (a keyboard listener). Replaced with a plain module-level singleton: attaches once on first consumer, detaches once the last consumer's key is removed.
  • Use pinned version for web-design-guidelines skill instead of fetching from main branch #30 (and its sibling Potential security risk in skill fetching pattern #91) — web-design-guidelines fetched its instructions from a GitHub raw URL on every invocation — an unpinned supply-chain/reproducibility risk. Vendored the current upstream content as references/guidelines.md, pinned to a specific commit, with a documented refresh procedure.

Notes

  • The two pre-built .zip archives under skills/ (react-best-practices.zip, react-view-transitions.zip) will reference stale paths after the rename, and web-design-guidelines.zip will ship the old live-fetch behavior, until whatever external process (not visible in this repo) regenerates them.
  • Code review caught and fixed a subtle bug in the rewritten singleton-listener example: the effect depended on callback identity, so an inline arrow (new identity every render) would tear down/re-attach the real DOM listener on every render — exactly the churn the pattern teaches how to avoid. Fixed by stabilizing the callback via a ref, mirroring the existing pattern in advanced-event-handler-refs.md.

Test plan

  • pnpm build && pnpm validate passes (from packages/react-best-practices-build/)
  • Repo-wide grep confirms no remaining reference to the 4 old bare directory names outside the .zip archives
  • Scratch-tested the singleton listener's attach/detach lifecycle (multi-consumer, same-key, re-mount-after-teardown, and re-render-with-inline-callback cases)
  • Multi-persona code review (correctness, project-standards, agent-native) — 4 findings surfaced and fixed

Fixes #80, #87, #68, #30.

SomSamantray added 6 commits July 30, 2026 17:42
Four skills' directory names don't match their SKILL.md frontmatter
name: field (agentskills.io spec violation, vercel-labs#80). The frontmatter
names are already the canonical identity used throughout
vercel-optimize's docs/tests/scanner and skills.sh.json, so rename
the directories to match rather than the reverse.

Also fixes a stale directory-name reference in the
vercel-react-view-transitions README and the top-level README's
skill-listing headers.
…t check

The React Compiler caveat was buried in a trailing note on one rule
file (vercel-labs#87), easy to miss. Add a "Pre-flight Checks" section near the
top of SKILL.md with detection guidance, and promote the caveat to a
prominent "Skip if" callout on the 3 rules it actually affects
(rerender-memo, rerender-memo-with-default-value,
rerender-simple-expression-in-memo). The other 12 rerender-* rules
are unrelated to React Compiler and are untouched.
…ain singleton

useSWRSubscription() is a data-subscription hook being misused here for
a DOM side-effect (a keyboard listener), not genuine data streaming
(vercel-labs#68). Replace it with a plain module-level singleton: the listener
attaches once on first consumer and detaches once the last consumer's
key is removed from the callback Map. Preserves the "N instances = 1
listener" guarantee and the existing keyCallbacks Map bookkeeping.
…e-fetching

The skill live-fetched instructions from raw.githubusercontent.com on
every invocation, an unpinned supply-chain/reproducibility risk (vercel-labs#30,
vercel-labs#91). Vendor the current upstream content as guidelines.md, pinned to
commit d0a657bfe87e86dd3a4753d7ec28c7e7dd7a88fe (2026-04-06), and
update SKILL.md's How It Works / Guidelines Source / Usage sections
to read the local file. Add an "Updating the pinned guidelines"
section documenting the refresh procedure.
Build artifact regeneration to reflect the U2/U3 rule-file content
changes (React Compiler pre-flight callouts, singleton listener
example).
- client-event-listeners.md: stabilize the callback via a ref so the
  listener effect doesn't depend on `callback` identity -- an inline
  arrow (a new identity every render) was tearing down and
  re-attaching the singleton DOM listener on every render, the exact
  churn the pattern exists to avoid. Mirrors the ref-stabilization
  pattern in advanced-event-handler-refs.md.
- web-design-guidelines: move guidelines.md into references/, per
  this repo's documented directory structure for on-demand supporting
  docs (AGENTS.md).
- guidelines.md: strip the vendored file's dead $ARGUMENTS
  slash-command placeholder and inert nested frontmatter, which only
  made sense when the file was invoked as a command rather than read
  as reference data.
- SKILL.md: fix the refresh-pin instructions to give the actual
  fetchable raw-content URL template instead of the repo homepage.
@vercel

vercel Bot commented Jul 30, 2026

Copy link
Copy Markdown

Someone is attempting to deploy a commit to the Vercel Team on Vercel.

A member of the Team first needs to authorize it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

react-best-practices directory name doesn't match frontmatter name

1 participant