Skip to content

docs: distill Table plugin/hook review patterns into the package reviewer#3798

Merged
cixzhang merged 3 commits into
mainfrom
docs/reviewer-plugin-hook-patterns
Jul 11, 2026
Merged

docs: distill Table plugin/hook review patterns into the package reviewer#3798
cixzhang merged 3 commits into
mainfrom
docs/reviewer-plugin-hook-patterns

Conversation

@cixzhang

@cixzhang cixzhang commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

What

Distills recurring, generalizable patterns from recent review history into the package Copilot reviewer, so it catches them automatically next time. Advisory, as with the rest of the reviewer.

From the Table plugin series (#3756, #3757, #3763)

Plugins & hooks that extend a host component:

  • Mirror the host's API shape, in and out — a Table plugin should accept the same idKey (string-or-getter) and name outputs to match the host's props (idKey, not getRowKey).
  • Semantic values first, arbitrary as escape hatch — a color/status value defaults to system semantics (color: 'accent'), not raw values.
  • Host-level vs plugin-level, deliberately — a11y-affecting config (a startFrom that changes aria-rowindex) likely belongs on the host.

Hook stability & reuse of existing data:

  • Dependency-set stability — flag hooks whose memoized output depends on a churning value and re-hands a new reference each update.
  • Don't re-derive what's already available — a row-index plugin looped over data to recompute what the row already carried as aria-rowindex.

From wider review history (#3665, #3721)

  • No wrapper <div> for styling (Mechanical checklist) — Astryx components extend BaseProps (take xstyle), so apply style directly (<Divider xstyle={...} />) instead of wrapping in a styled div.
  • Silent breaking changes to shared types/context (Judgment) — adding a required field to a shared type/context/API breaks existing consumers; the tell is unrelated tests/examples needing edits. Flag it and ask whether it should be optional with an internal default.

Why

These came up repeatedly across real PRs. Encoding them means the reviewer surfaces API-consistency, hook-stability, wrapper-div, and subtle-breaking-change issues on the next PR instead of relying on a maintainer to re-notice each time.

I deliberately left out one-off nits (test-enumeration asks, "reuse this later" praise, in-progress perf musings) that don't generalize into a rule.

@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Jul 11, 2026
@vercel

vercel Bot commented Jul 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
astryx Ready Ready Preview, Comment Jul 11, 2026 4:23am

Request Review

@thedjpetersen thedjpetersen left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved.

@github-actions

github-actions Bot commented Jul 11, 2026

Copy link
Copy Markdown
Contributor

PR Analysis Report

📚 Storybook Preview

View Storybook for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

🧪 Sandbox Preview

View Sandbox for this PR
GitHub Pages may take up to a minute to hydrate after deploy.

No new or modified components detected.

Bundle Size Summary

Package Size (ESM) Size (CJS) Gzipped
@astryxdesign/core N/A 4.6KB 0B

Accessibility Audit

Status: No accessibility violations detected.


Generated by PR Enrichment workflow | Storybook | Sandbox | View full report

github-actions Bot added a commit that referenced this pull request Jul 11, 2026
github-actions Bot added a commit that referenced this pull request Jul 11, 2026
@cixzhang
cixzhang merged commit 310b83a into main Jul 11, 2026
16 checks passed
@github-actions
github-actions Bot deleted the docs/reviewer-plugin-hook-patterns branch July 12, 2026 06:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CLA Signed This label is managed by the Meta Open Source bot.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants