diff --git a/.agents/skills/assign-prs/README.md b/.agents/skills/assign-prs/README.md new file mode 100644 index 00000000000000..41f9c6a8f892a3 --- /dev/null +++ b/.agents/skills/assign-prs/README.md @@ -0,0 +1,66 @@ +# Reviewer configuration + +`reviewers.json` decides who receives review requests from the `/assign-prs` skill. Edit that file to +change assignment; never edit `SKILL.md`, which deliberately contains no names. + +## It is an overlay, not a roster + +Team membership is resolved at run time from the GitHub team API. This file only declares which of +those members are eligible for automatic assignment, and which areas each one owns. Both halves are +required: the API says who exists, the file says who to ask. + +The `team` field names the team the roster is drawn from. It is deliberately independent of the +`--team` queues the skill reads: this repository grooms two queues but draws every reviewer from one +team, so a build-queue PR can be routed to a component reviewer. + +The two are reconciled on every run and drift is reported in both directions: + +- **In the file but no longer on the team** — a stale entry. They never receive a request. +- **On the team but absent from the file** — unconfigured. Under the default `exclude-and-report` + policy they receive nothing, and the run names them. + +Every member of the team is listed, **including ineligible ones**, so that nobody is silently absent. +Removing an entry is not the same as marking it ineligible: an unlisted member is reported as +unconfigured on every run until someone decides. + +## Fields + +| Field | Required | Meaning | +| ------------------ | -------- | ---------------------------------------------------------------- | +| `login` | yes | GitHub login, matched against the live team roster | +| `eligible` | yes | Whether this person may receive automatic review requests | +| `primary` | yes | Areas this person owns; selection prefers them here | +| `secondary` | yes | Areas this person can cover when no primary owner is available | +| `fallbackEligible` | no | Defaults to `true`. Set `false` to opt out of unclassifiable PRs | + +`fallbackEligible: false` means "only ever pick me for an area I actually declare." It exists because +a narrow specialist carries little load by definition, so lowest-load selection would otherwise hand +them every PR that matched no area — the opposite of what declaring a specialty means. + +Read it with an explicit presence check, never with jq's `//` operator: jq treats `false` as empty, so +`.fallbackEligible // true` reads every opt-out back as opt-in. + +## Areas + +The `areas` map defines each area's `paths` and conventional-commit `scopes`. A PR's area is detected +from its changed file paths first, falling back to the commit scope in the title. + +Every value in `primary` and `secondary` must be a key of `areas`. JSON Schema cannot express that +cross-reference, so it is checked at run time — a typo such as `ai-skill` for `ai-skills` is not a +validation error, it simply never matches anything. + +## This file is public + +`microsoft/fluentui` is a public repository and this config is committed to it. An entry carries a +login, an eligibility flag and areas — deliberately no free-text notes about anyone. The schema sets +`additionalProperties: false` so a commentary field cannot be reintroduced by accident. + +When eligibility changes, record the reasoning in the pull request that changes the flag. Review +statistics and comparisons between people belong in that discussion, not in a permanent public file. + +## Validating a change + +```bash +jq empty reviewers.json +npx ajv-cli validate -s reviewers.schema.json -d reviewers.json --spec=draft2020 +``` diff --git a/.agents/skills/assign-prs/SKILL.md b/.agents/skills/assign-prs/SKILL.md new file mode 100644 index 00000000000000..900ef3eb58aecb --- /dev/null +++ b/.agents/skills/assign-prs/SKILL.md @@ -0,0 +1,359 @@ +--- +name: assign-prs +description: >- + Assign reviewers to the open pull requests awaiting review from the Fluent UI team queues, by default microsoft/cxe-prg and microsoft/fluentui-react-build. Routes each PR that lacks coverage to a reviewer by area ownership and then by lowest current review load, requesting only the shortfall. Always presents a dry-run plan and requires explicit approval before requesting a review. +disable-model-invocation: true +argument-hint: '[--repo owner/repo] [--team slug] [--account login] [--reviewers count] [--stale-days days]' +allowed-tools: Bash Read Grep Glob +--- + +# Assign PRs + +Groom the team review queues by getting a human reviewer onto every PR that needs one. Route by area ownership, balance by current review load, and request only the shortfall. The default operation is read-only. Never request a review until the user explicitly approves the plan. + +This skill does one thing. It does not approve, merge, close, or comment on anything — for Dependabot updates use `/dependabot-rollup`, which combines compatible updates into a single reviewable branch. + +## Defaults + +| Argument | Default | Purpose | +| -------------- | ----------------------------------------------------- | --------------------------------------------------- | +| `--repo` | `microsoft/fluentui` | Repository containing the pull requests | +| `--team` | `microsoft/cxe-prg`, `microsoft/fluentui-react-build` | Team queue to read; repeat the flag to pass several | +| `--account` | Active `gh` account | GitHub account the run acts as | +| `--reviewers` | `1` | Reviewers to assign per PR, minimum 1 | +| `--stale-days` | `90` | Age past which a PR is reported but never assigned | + +Parse overrides from `$ARGUMENTS`. Reject an invalid repository name, a `--reviewers` value below 1, a non-integer `--stale-days`, or unknown arguments instead of guessing. + +`--team` is repeatable and replaces the defaults entirely when given: passing it once reads exactly one queue rather than adding a third. Collect the validated values into `TEAMS`, each as `org/slug`. Note that this selects which queues to read, not who may be assigned — the reviewer roster comes from the team declared in `reviewers.json`, as Step 4 explains. + +## Step 1 - Check prerequisites + +Every command runs as `ACCOUNT`. Several accounts are commonly authenticated at once, and `gh` applies whichever one is active, so a run can silently request reviews as the wrong identity. Resolve the active account before anything else: + +```bash +gh auth status --active +``` + +`ACCOUNT` defaults to whatever that returns. **Name the resolved login in the plan and have the user confirm it before any mutation.** This is a confirmation, not an assertion — the skill has no expected identity to check against, so it cannot detect a wrong-account run on its own. The user is the only thing standing between a stale `gh auth switch` and a batch of review requests sent from the wrong person. + +Pass `--account` to state the expected login explicitly and get the stronger check back: stop if the active login is not the one given. Report it and offer the switch rather than performing it silently, because switching changes the identity for every other `gh` process on the machine: + +```bash +gh auth switch --user "$ACCOUNT" +``` + +Also confirm the token carries `read:org`, which Step 4 needs to resolve the team roster. + +The account must then have `push` or `triage` permission on the repository. Requesting a reviewer fails on a read-only token, and the failure message is misleading: GitHub reports a `ReplaceActorsForAssignable` permission error rather than an authentication error. + +```bash +gh api "repos/${REPO}" -q '.permissions' +``` + +Stop and report if `push` and `triage` are both `false`. + +## Step 2 - Fetch the queues + +Use the search API directly so `draft:false` is honored. The `gh pr list` and `gh search prs` shortcuts do not reliably apply the draft filter and will pull draft PRs into the queue: + +```bash +for TEAM in "${TEAMS[@]}"; do + gh api --method GET search/issues \ + -f q="is:open is:pr repo:${REPO} team-review-requested:${TEAM} draft:false" \ + -f sort=updated -f order=desc -f per_page=100 \ + --jq '.items[] | {number, title, user: .user.login, updated_at}' +done +``` + +**Build the qualifier string from the validated `REPO` and `TEAM` values, and never from a literal.** A single-quoted query with the repository baked in ignores `--repo` entirely, so discovery reads one repository while the mutations in the final step, which do honour `--repo`, write to another. The queue then holds PR numbers from the wrong project and every number resolves to an unrelated PR that happens to share it. Use `--method GET` with `-f q="..."` as above so `gh` form-encodes the qualifiers; hand-glued `+` separators in a query string silently mis-encode any value containing a space or a slash. + +**Deduplicate by PR number across the queues.** The queues overlap substantially; a PR requesting review from more than one team must be considered once and assigned once. Record which queue or queues each PR came from so the report can show it. + +### What these queries do and do not cover + +`team-review-requested:` matches only PRs with an _outstanding_ review request for that team. Two consequences worth stating in the report: + +- **The queue is a point-in-time snapshot.** PRs are opened while the plan is being reviewed, and a long approval pause means the batch no longer matches reality. Re-fetch every queue immediately before applying, and assign to anything that appeared in between rather than reusing a stale list. Timestamp the snapshot in the report so the gap is visible. +- **A PR that never requested one of these teams is invisible here**, however badly it needs a reviewer. The repository carries several times more open PRs than these queues contain; the rest belong to other teams and are out of scope. Report the queue size against the repository's total open non-draft PR count so the boundary is explicit and nobody reads a clean queue as a clean repository. + +## Step 3 - Partition the queue + +Split the deduplicated set into three buckets. A PR belongs to exactly one: + +1. **Dependabot** - author is `dependabot[bot]` or `app/dependabot`. Reported, never assigned. +2. **Stale** - last updated more than `STALE_DAYS` ago. Reported, never assigned. +3. **Assignable** - everything else. + +Only the third bucket is acted on. The other two are **exclusions, not workstreams** — this skill neither merges nor closes anything, and the report says how many PRs each bucket held so the queue's real shape stays visible. + +Both exclusions earn their place: + +- **Dependabot PRs do not need a human reviewer each.** These queues are mostly dependency bumps, so assigning them would hand a small pool dozens of review requests that no human judgement improves. Point the reader at `/dependabot-rollup`, which combines compatible updates into one reviewable branch. +- **Stale PRs do not benefit from another reviewer.** Piling a request onto a PR idle for months creates noise, not throughput; these queues contain PRs several years old. Report them so someone can decide their fate, and leave that decision to a human. + +## Step 4 - Plan reviewer assignments + +Resolve team members at run time rather than hardcoding a roster, because membership changes. The roster comes from the team declared in `reviewers.json`, not from `--team`: + +```bash +TEAM_ORG="$(jq -r '.team.org' "$CONFIG")" +TEAM_SLUG="$(jq -r '.team.slug' "$CONFIG")" +gh api "orgs/${TEAM_ORG}/teams/${TEAM_SLUG}/members" --paginate -q '.[].login' +``` + +**`--team` and the config's `team` answer different questions.** `--team` selects which queues to read — whose PRs need attention. The config's `team` selects who may be assigned. They are deliberately allowed to differ: this repository grooms two queues but draws every reviewer from one team, so a build-queue PR can be routed to a component reviewer. Resolving the roster from the union of the queue teams instead would report every member of the second team as unconfigured on every run, which is noise, not drift. + +This needs only `read:org`. If it fails, stop and report; do not fall back to a stale hardcoded list. A roster that resolves to zero members is a fault, not an empty pool. + +### Reviewer configuration + +Everything about _who_ reviews lives in `reviewers.json`, beside this file, and `README.md` documents its fields. This document deliberately contains no names — to change who receives assignments, edit that file, never this one. + +Validate the config before reading anything out of it: + +```bash +CONFIG="$(dirname "$SKILL_PATH")/reviewers.json" +SCHEMA="$(dirname "$SKILL_PATH")/reviewers.schema.json" + +jq empty "$CONFIG" || { echo "reviewers.json is unparseable"; } + +jq -e ' + (.areas | keys) as $valid + | [.reviewers[] | (.primary + .secondary)[]] + | map(select(. as $a | $valid | index($a) | not)) + | if length == 0 then true else ("undeclared areas: " + join(", ") | error) end +' "$CONFIG" +``` + +`reviewers.schema.json` covers the structure, and its `additionalProperties: false` is what stops a free-text field about a person being reintroduced. It cannot express that every entry in `primary` and `secondary` must be a key of `areas` — JSON Schema has no keyref — so that cross-reference is the check above. It matters because a misspelled area is not an error at run time: it simply never matches, and the owner silently stops receiving their own area's PRs. + +The config is an **overlay, not a roster**. Membership still comes from the API call above; the file only declares which of those members are eligible and what each one knows well. Both halves are required: the API says who exists, the file says who to ask. + +Read the eligible pool from it: + +```bash +jq -r '[.reviewers[] | select(.eligible) | .login] | join("\n")' "$CONFIG" +``` + +**If `reviewers.json` is missing, unparseable, fails schema validation, declares an undeclared area, or yields zero eligible reviewers, stop and report the specific fault.** Never fall back to an inline list — that reintroduces the hardcoding this design removes. Assigning to a guessed reviewer is worse than assigning to nobody, because it looks like the config was consulted. + +Report the effective pool size at the top of the plan, because shrinking the roster concentrates load: at `REVIEWERS=1` a 19-PR queue still lands roughly 5 new reviews on each of 4 eligible people, and raising `REVIEWERS` multiplies that directly. + +### Reconcile the config against the live roster + +The two sources drift as people join and leave. Compare them every run and report both directions: + +- **In the config but no longer on the team** — a stale entry. Never request a review from them; report so the entry can be removed. +- **On the team but absent from the config** — unconfigured. Under the default `unknownMemberPolicy` of `exclude-and-report` they receive nothing, and the run says so by name. + +The second case is the one that matters. Treating an unknown member as eligible would silently route human review work to an account the config knows nothing about — it may be a manager, a service account, or someone who joined for an unrelated reason. Excluding them is safe but only stays visible if the report names them, so it must never be folded into a summary count. + +### Eligibility means "reviews", not "is on the team" + +Before marking anyone `eligible`, check that they actually review. An account can accumulate hundreds of review requests through CODEOWNERS while submitting almost none: + +```bash +gh api "search/issues?q=repo:${REPO}+reviewed-by:${LOGIN}&per_page=1" --jq .total_count +gh api "search/issues?q=repo:${REPO}+review-requested:${LOGIN}&per_page=1" --jq .total_count +``` + +A ratio near zero matters because an eligible member counts toward `existing_coverage` on any PR that already requests them **in an area they serve**. Marking a non-reviewing account eligible therefore suppresses the real assignment those PRs needed, inside that area. Coverage is area-scoped precisely to bound this, so an account that CODEOWNERS requests across the whole repository only distorts the areas it actually claims — but the bound is only as tight as its area list. Keep such an account's `primary` and `secondary` narrow, and set `fallbackEligible` to `false` so it never absorbs unclassified work either. + +Where an area's declared owner does not review in practice, give the area to someone who does as well. `reviewers.json` holds no free-text field, so name the substitution in the run's report — read cold, the config shows both as owners with nothing to indicate one is standing in for the other. + +**Report those ratios in the run, never try to record them in `reviewers.json`.** This repository is public and the config is committed, so it deliberately gives you nowhere to put them. A measurement that justifies a decision in conversation becomes a permanent public statement about a named person once committed. Change the `eligible` flag, and explain the change in the pull request that makes it. + +A typo'd login in the config surfaces here too: it appears as a stale entry on one side and an unconfigured member on the other. + +When diffing the two lists with `comm`, sort both sides with the same collation — `jq` sorts ASCII (uppercase first) while shell `sort` follows the locale, so mixed-case logins land in different positions and `comm` reports every name as drifted in both directions at once. That symptom is diagnostic: identical non-empty lists under both headings means the sort, not the roster, is wrong. + +```bash +gh api "orgs/${TEAM_ORG}/teams/${TEAM_SLUG}/members" --paginate -q '.[].login' | LC_ALL=C sort > live.txt +jq -r '.reviewers[].login' "$CONFIG" | LC_ALL=C sort > cfg.txt +LC_ALL=C comm -13 live.txt cfg.txt # stale entries +LC_ALL=C comm -23 live.txt cfg.txt # unconfigured members +``` + +### Route by area, then by load + +Each reviewer declares `primary` and `secondary` areas; each area declares the `paths` and conventional-commit `scopes` that identify it. + +Detect a PR's area from **changed file paths first**, falling back to the conventional-commit scope in the title (`feat(react-button): …` → `react-button`). Paths are authoritative because a title scope is free text and is occasionally wrong or absent. A PR matching nothing has no area. + +**Score each file against the most specific pattern that matches it, then total per area — never count a file toward every area whose pattern matches.** Area paths nest: `packages/react-components/react-headless-components-preview/**` sits inside `packages/react-components/**`, so every headless file also matches `components`. Counting matches per area therefore guarantees the broader area wins any nested case by construction, and a purely headless PR routes to `components`. + +Assign each file to exactly one area — the one whose matching pattern has the longest literal prefix — and only then take the area holding the most files. Ties go to the more specific area. + +Take a pattern's literal prefix as everything before its **first** `*`, not merely by stripping a trailing `/**`. Patterns carry mid-string globs — `packages/react-components/react-motion*/**` is one — and trimming only the tail leaves a `*` inside the prefix, so it matches nothing and the area silently never fires. + +Ignore `change/**` beachball files when counting. Every fluentui PR carries them, they encode the package name in the filename rather than the path, and including them dilutes the signal without ever identifying an area. + +Expect path and scope to disagree, and trust the path. A GitHub Action bump is titled `chore(deps)` but edits `.github/workflows/**`, so it is build work wearing a dependencies label; an npm bump with the same title touches only `yarn.lock` and really is a dependencies change. The disagreement is information, not noise. + +**Resolve overlapping patterns by specificity, never by declaration order.** Areas overlap heavily: `react-headless-components-preview` matches both the `headless` area exactly _and_ the `components` area's `react-*` glob, and `react-motion-components-preview` matches both `react-motion*` and `react-*`. Rank candidates so that an exact match beats a glob, and a longer glob prefix beats a shorter one: + +``` +score(scope, pattern) = 1000 + len(pattern) if pattern == scope + len(pattern) if pattern is a prefix glob that matches + no match otherwise +``` + +Taking the first match in file order instead produces silently wrong routing that looks correct on the common cases — `react-motion` lands in `components`, and `headless` only wins by the accident of being declared first. Apply the same specificity rule to path patterns. + +Build the tier for selection: + +``` +pool = (live roster ∩ config eligible) − author − already requested − already reviewing +tier = pool ∩ reviewers whose PRIMARY areas include area + else pool ∩ reviewers whose SECONDARY areas include area + else pool ∩ reviewers where fallbackEligible is not false +``` + +A reviewer may set `"fallbackEligible": false` to opt out of that last tier. They are then only ever selected for an area they actually declare, and never absorb work that matched nothing. This exists because a narrow specialist carries little load by definition, so lowest-load selection would otherwise hand them every unclassifiable PR — the opposite of what declaring a specialty means. The field is optional and defaults to `true`. + +Read that flag with an explicit presence check, never with jq's `//` operator: + +```bash +jq -r '.reviewers[] | select(.eligible) + | select((has("fallbackEligible") | not) or .fallbackEligible) + | .login' "$CONFIG" +``` + +`.fallbackEligible // true` is wrong and fails silently: jq treats `false` as empty, so the alternative fires and every opted-out reviewer reads back as opted-in. Note the parentheses around `has(...) | not` as well — without them the pipe binds first, `.fallbackEligible` is applied to a boolean, and jq aborts with `Cannot index boolean`. The same trap applies to any boolean in this file that defaults to true. + +The final fallback tier is what makes `areaMatch: "preference"` different from `"hard"`. The eligible pool is small, and some areas are owned by other teams entirely, so at any moment an area may have no declared owner among the people available. A strict filter would report those PRs under-covered while a perfectly capable reviewer sat idle; preference mode degrades to the pool instead. Under `"hard"`, an empty tier is left under-covered rather than filled. + +An area that repeatedly reaches the fallback tier is a gap in `reviewers.json`, not a property of the work — give it an owner rather than letting selection default. + +Then pick from the tier by **lowest current open-review load**, breaking ties at random. + +Load-first selection within the tier, rather than a uniform draw over it, is deliberate. A blind random draw over a four-person pool skews badly and self-reinforces: it repeatedly lands on whoever is already busiest, and the excess trains the team to ignore review notifications. Report the load column alongside the number of requests this batch adds so a bad draw is still visible before approval. + +For each assignable PR, build the eligible pool by removing: + +- **The PR author.** GitHub rejects a review request for the author's own PR, and this is the most common cause of a failed batch. Team members author many of the PRs in these queues. This applies to **any** calculation of coverage, including a quick pre-flight check before a mutation — an author who has self-reviewed still counts for nothing, and a guard that forgets this will skip a PR that genuinely needs a reviewer. +- **Existing requested reviewers**, individual or team, so the same person is not asked twice. +- **Anyone who has already submitted a review** on that PR. + +### Assign only the shortfall + +Never assign a fixed `REVIEWERS` per PR. Count what the PR already has, and request only what is missing: + +``` +serves(area) = eligible reviewers whose primary or secondary areas include area, + or — when no eligible reviewer declares that area — every eligible + reviewer whose fallbackEligible is not false +existing_coverage = reviewers in serves(area) already requested on the PR or already + reviewing it +shortfall = max(0, REVIEWERS - existing_coverage) +``` + +Select `shortfall` logins from the tier described above — area match first, then lowest load. When `shortfall` is `0`, **request nobody** and report the PR as already covered. + +This is the single most important rule in this step. A PR that already has a team member on it does not need a second, and assigning one anyway is the default failure mode of this skill: most PRs in these queues already carry a reviewer, so a naive fixed-size assignment inflates a batch several times over and dumps the excess on a small pool. Excess requests are worse than useless — they train the team to ignore review notifications. + +**Coverage is area-scoped: being requested on a PR only counts if the person serves that PR's area.** Note that `serves(area)` does not cascade the way selection does — selection prefers a primary owner over a secondary one, but for coverage either counts, because either would be a legitimate review. Without this scoping, one account that CODEOWNERS requests across the whole repository would mark nearly every PR "already covered" and silently suppress the assignments they needed, in areas that account never works in. Area ownership is already declared in `reviewers.json`, so use it on both sides of the calculation rather than treating any request as coverage. + +`existing_coverage` counts only members of the eligible pool defined by `reviewers.json`. A review request aimed at the whole team is what put the PR in this queue, so it never counts toward coverage; neither does a bot review, nor a reviewer outside the team, nor an eligible member requested on a PR outside the areas they serve, nor the author's own review of their own PR. + +### Judge coverage, not pool size + +A small or empty pool usually means the PR is already well covered, not that it needs attention. Compute **effective coverage** for each PR: + +``` +effective_coverage = (reviewers in serves(area) already requested or already reviewing) + + (newly selected reviewers) +``` + +- Report a PR as **under-covered** only when `effective_coverage < REVIEWERS`. That is the condition a human needs to act on, and it means the pool ran dry before the shortfall was filled. +- When `shortfall` is `0`, report the PR as **already covered** and make no request. This is a normal, healthy outcome, not a failure, and it is expected to be the majority of a mature queue. +- When the pool is smaller than `shortfall`, select everyone available. Never pad the selection with an ineligible login. + +Flagging on pool size alone produces false warnings on exactly the PRs that are in the best shape. + +Compute each member's current open-review load across the deduplicated queue and include it in the report, alongside the number of requests this batch would add. Selection is area-then-load rather than a blind draw, so a lopsided batch is now a signal that something is wrong rather than ordinary variance — check whether one area is absorbing the whole queue, or whether the shortfall rule is being ignored. If the batch total looks large relative to the number of under-covered PRs, that is a symptom of ignoring the shortfall rule — recheck it before presenting the plan. + +## Step 5 - Present the dry-run plan + +Show the whole plan before touching anything: + +```markdown +## Reviewer assignment plan + +- Repository: owner/repo +- Queues: cxe-prg (25), fluentui-react-build (51), 62 unique after dedupe +- Eligible reviewers after exclusions: 6 of 8 +- Config: reviewers.json in sync with the live roster +- Assignable: 18 | Dependabot (not assigned): 40 | Stale (not assigned): 4 + +### Reviewer assignments + +| PR | Author | Queue | Area | Reviewers to add | Match | Coverage | +| ---- | ------ | ------- | ---------- | ----------------- | --------- | -------- | +| #123 | alice | cxe-prg | headless | bob | primary | 1 | +| #124 | dave | both | motion | carol | fell back | 1 | +| #125 | erin | cxe-prg | components | (already covered) | - | 2 | + +### Not assigned + +| Bucket | Count | Note | +| ---------- | ----- | ---------------------------------------------- | +| Dependabot | 40 | use `/dependabot-rollup` | +| Stale | 4 | idle more than 90 days; needs a human decision | +``` + +Show the config-reconciliation line even when it is clean, so a silent drift is never mistaken for an absent check. When it is not clean, replace it with the detail and list the affected logins by name: + +```markdown +- Config: 1 stale entry (`oldperson` left the team), 1 unconfigured member (`newperson` — receiving nothing until added to reviewers.json) +``` + +The `Match` column records whether the chosen reviewer owned the area as `primary`, as `secondary`, or whether selection `fell back` to the whole pool because nobody owned it. A column full of fallbacks means the area map in `reviewers.json` no longer reflects what the team actually works on, and is the signal to update it. + +The `Coverage` column counts only reviewers who serve the PR's area, so it can read lower than the reviewer list GitHub shows. When a PR is assigned despite already carrying an eligible reviewer, say which reviewer was discounted and for which area — otherwise the row looks like the shortfall rule misfiring, and the natural correction is to suppress exactly the assignment that was needed. + +Report the two excluded buckets as counts rather than dropping them. A run that assigns 3 reviewers out of a 62-PR queue looks broken until the report shows that 40 were Dependabot and 4 were stale. + +Then ask the user to approve. Accept `apply all`, a subset such as `assign 36476` or `skip 36430`, or `cancel`. Treat invoking the skill as a request for the plan, never as approval to mutate. + +## Step 6 - Apply approved assignments + +Act on approved items only, one PR at a time, printing a one-line result for each. Do not retry a failure blindly; report it and continue with the remaining items. + +Request reviewers: + +```bash +gh pr edit "$PR" --repo "$REPO" --add-reviewer "$LOGIN" +``` + +Pass `--repo "$REPO"` on every call, using the same value discovery ran under. A mutation aimed at a different repository than the queue was read from will still succeed whenever that number happens to exist there, and it will act on an unrelated PR. + +If the request is rejected for a missing permission or because the login cannot be requested on that PR, report it and move on. Never retry by substituting a different reviewer without saying so. + +## Step 7 - Report + +Print assigned, skipped and failed counts, each failure with its specific reason, plus already-covered PRs and any genuinely under-covered PRs still needing a human. Restate the Dependabot and stale counts that were excluded from assignment. Name the next action for anything left unresolved. + +Verify rather than trusting exit codes: re-read the affected PRs and confirm the state actually changed — every assigned PR should sit at exactly `REVIEWERS` eligible reviewers who serve its area. A command that returns zero has not necessarily produced the state you intended. + +If a correction is needed, remove a request with `gh pr edit "$PR" --repo "$REPO" --remove-reviewer "$LOGIN"`, but guard each removal: skip when that person has already submitted a review, when the request is already gone, or when removing would drop coverage to zero. Only ever remove requests this run created — a reviewer who predates the run is not yours to unassign. + +A run almost always leaves something a human has to finish: a PR parked on another team's CODEOWNERS approval, an area with no eligible owner, a stale PR somebody has to decide about. Nothing persists between runs, so list each of these explicitly at the end of the report, with a link and the specific reason it needs a person — "no eligible reviewer serves `charting`, and the PR has sat 40 days" is actionable, "needs review" is not. Anything left only as an implication of a table is lost when the conversation ends. + +## Guardrails + +- Always dry-run and obtain explicit approval before requesting a review. +- Never act as an account other than the one confirmed in Step 1; stop and ask rather than switching accounts unprompted. +- Never approve, merge, close, or comment on a pull request. This skill only requests reviewers; `/dependabot-rollup` owns dependency updates. +- Never build a search query from a literal repository or team name; derive every qualifier from the validated `REPO` and `TEAMS` values, and mutate only PRs discovered under that same `REPO`. +- Never assign the PR author as a reviewer of their own PR. +- Never add a reviewer to a PR that already has `REVIEWERS` eligible team members requested or reviewing who serve that PR's area; request only the shortfall. A request to someone outside the area they serve is not coverage, and the author's own review never counts toward that total. +- Never assign a reviewer to a Dependabot PR or to a PR idle longer than `STALE_DAYS`; report both as counts instead. +- Never hardcode the team roster or reviewer names in `SKILL.md`; resolve membership from the API at run time and read eligibility and areas from `reviewers.json`. Neither source is sufficient alone. +- Never request a review from a login that is absent from `reviewers.json`, or present but not `eligible`; report the omission by name instead. +- Never fall back to an inline reviewer list when `reviewers.json` is missing, malformed, or fails schema validation; stop and report the specific fault. +- Never add review statistics, performance comparisons, or any other free-text assessment of a person to `reviewers.json`; it is a committed file in a public repository. Report those figures in the run, and explain eligibility changes in the pull request that makes them. +- Never request or print a GitHub token; use the user's existing `gh` authentication. +- Never run on a schedule or add a GitHub Actions workflow. +- Never remove a reviewer this run did not add. diff --git a/.agents/skills/assign-prs/reviewers.json b/.agents/skills/assign-prs/reviewers.json new file mode 100644 index 00000000000000..cf934db3caf8f0 --- /dev/null +++ b/.agents/skills/assign-prs/reviewers.json @@ -0,0 +1,134 @@ +{ + "$schema": "./reviewers.schema.json", + "version": 1, + + "team": { + "org": "microsoft", + "slug": "cxe-prg" + }, + + "settings": { + "reviewers": 1, + "areaMatch": "preference", + "unknownMemberPolicy": "exclude-and-report" + }, + + "areas": { + "headless": { + "label": "Headless components", + "scopes": ["react-headless-components-preview", "headless-*"], + "paths": ["packages/react-components/react-headless-components-preview/**", "apps/public-docsite-v9-headless/**"] + }, + "components": { + "label": "v9 React components", + "scopes": ["react", "react-*"], + "paths": ["packages/react-components/**"] + }, + "web-components": { + "label": "Web components", + "scopes": ["web-components"], + "paths": ["packages/web-components/**", "apps/vr-tests-web-components/**"] + }, + "motion": { + "label": "Motion", + "scopes": ["react-motion*"], + "paths": ["packages/react-components/react-motion*/**"] + }, + "build": { + "label": "Build, tooling and release", + "scopes": [ + "eslint-*", + "workspace-plugin", + "tools", + "scripts*", + "ci", + "release", + "bundle-size", + "monosize", + "babel-preset*", + "react-conformance" + ], + "paths": [ + "tools/**", + "scripts/**", + ".github/**", + ".devops/**", + "packages/eslint-plugin/**", + "packages/react-conformance/**" + ] + }, + "dependencies": { + "label": "Dependencies", + "scopes": ["deps", "deps-dev", "dependabot"], + "paths": ["yarn.lock", "package.json", "**/package.json"] + }, + "docs": { + "label": "Docs and docsite", + "scopes": ["docsite*", "api-docs", "react-docsite-components", "docs"], + "paths": ["apps/public-docsite*/**", "packages/api-docs/**"] + }, + "a11y": { + "label": "Accessibility", + "scopes": ["a11y*", "axe"], + "paths": ["packages/a11y-testing/**"] + }, + "ai-skills": { + "label": "Agentic Skills", + "scopes": ["skills"], + "paths": [".agents/skills/**", ".claude/skills/**", "AGENTS.md"] + } + }, + + "reviewers": [ + { + "login": "Hotell", + "eligible": true, + "primary": ["build"], + "secondary": ["ai-skills", "dependencies", "docs", "headless"] + }, + { + "login": "dmytrokirpa", + "eligible": true, + "primary": ["components", "headless"], + "secondary": ["a11y", "ai-skills", "build", "dependencies", "motion", "web-components"] + }, + { + "login": "mainframev", + "eligible": true, + "primary": ["components", "headless"], + "secondary": ["a11y", "ai-skills", "build", "dependencies", "motion", "web-components"] + }, + { + "login": "PaulGMardling", + "eligible": true, + "primary": ["components", "headless"], + "secondary": ["a11y", "ai-skills", "build", "dependencies", "motion", "web-components"] + }, + { + "login": "tudorpopams", + "eligible": true, + "fallbackEligible": false, + "primary": ["ai-skills"], + "secondary": [] + }, + { + "login": "ValentinaKozlova", + "eligible": false, + "primary": [], + "secondary": [] + }, + { + "login": "paolo-aliprandi", + "eligible": true, + "fallbackEligible": false, + "primary": ["ai-skills"], + "secondary": [] + }, + { + "login": "gouttierre", + "eligible": false, + "primary": [], + "secondary": [] + } + ] +} diff --git a/.agents/skills/assign-prs/reviewers.schema.json b/.agents/skills/assign-prs/reviewers.schema.json new file mode 100644 index 00000000000000..d1ff1d97fc3a7c --- /dev/null +++ b/.agents/skills/assign-prs/reviewers.schema.json @@ -0,0 +1,115 @@ +{ + "$schema": "https://json-schema.org/draft/2020-12/schema", + "$id": "https://github.com/microsoft/fluentui/.agents/skills/assign-prs/reviewers.schema.json", + "title": "assign-prs reviewer configuration", + "description": "Overlay declaring which members of the live GitHub team roster are eligible for automatic review assignment, and which areas each one owns. See README.md.", + "type": "object", + "additionalProperties": false, + "required": ["version", "team", "settings", "areas", "reviewers"], + "properties": { + "$schema": { + "type": "string", + "description": "Path to this schema, for editor validation." + }, + "version": { + "type": "integer", + "minimum": 1, + "description": "Config format version." + }, + "team": { + "type": "object", + "additionalProperties": false, + "required": ["org", "slug"], + "description": "GitHub team whose live membership is reconciled against this file.", + "properties": { + "org": { "type": "string", "minLength": 1 }, + "slug": { "type": "string", "minLength": 1 } + } + }, + "settings": { + "type": "object", + "additionalProperties": false, + "required": ["reviewers", "areaMatch", "unknownMemberPolicy"], + "properties": { + "reviewers": { + "type": "integer", + "minimum": 1, + "description": "Reviewers each PR should end up with. The skill requests only the shortfall." + }, + "areaMatch": { + "enum": ["preference", "hard"], + "description": "preference degrades to the whole eligible pool when nobody owns an area; hard leaves the PR under-covered instead." + }, + "unknownMemberPolicy": { + "enum": ["exclude-and-report"], + "description": "How to treat a live team member with no entry in this file." + } + } + }, + "areas": { + "type": "object", + "minProperties": 1, + "description": "Area definitions. Every value in a reviewer's primary or secondary list must be a key of this map; that cross-reference is checked at run time, not by this schema.", + "additionalProperties": { + "type": "object", + "additionalProperties": false, + "required": ["label", "scopes", "paths"], + "properties": { + "label": { + "type": "string", + "minLength": 1, + "description": "Human-readable area name used in reports." + }, + "scopes": { + "type": "array", + "items": { "type": "string", "minLength": 1 }, + "description": "Conventional-commit scopes that identify this area, used only when paths are inconclusive." + }, + "paths": { + "type": "array", + "minItems": 1, + "items": { "type": "string", "minLength": 1 }, + "description": "Glob patterns identifying this area. Paths are authoritative over scopes." + } + } + } + }, + "reviewers": { + "type": "array", + "minItems": 1, + "description": "Every member of the team, including ineligible ones, so nobody is silently absent.", + "items": { + "type": "object", + "additionalProperties": false, + "required": ["login", "eligible", "primary", "secondary"], + "properties": { + "login": { + "type": "string", + "pattern": "^[A-Za-z0-9](?:[A-Za-z0-9]|-(?=[A-Za-z0-9])){0,38}$", + "description": "GitHub login, matched against the live team roster." + }, + "eligible": { + "type": "boolean", + "description": "Whether this person may receive automatic review requests." + }, + "fallbackEligible": { + "type": "boolean", + "description": "Defaults to true. Set false to be selected only for declared areas, never for PRs that matched no area." + }, + "primary": { + "type": "array", + "uniqueItems": true, + "items": { "type": "string", "minLength": 1 }, + "description": "Areas this person owns. Selection prefers a primary owner." + }, + "secondary": { + "type": "array", + "uniqueItems": true, + "items": { "type": "string", "minLength": 1 }, + "description": "Areas this person can cover when no primary owner is available." + } + } + } + } + } +} diff --git a/.claude/skills/assign-prs/SKILL.md b/.claude/skills/assign-prs/SKILL.md new file mode 100644 index 00000000000000..3cca4d7da7e429 --- /dev/null +++ b/.claude/skills/assign-prs/SKILL.md @@ -0,0 +1 @@ +@../../../.agents/skills/assign-prs/SKILL.md diff --git a/AGENTS.md b/AGENTS.md index 43421874f15fc7..d89ee236fd14f8 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -103,6 +103,7 @@ state.root.className = mergeClasses( | `review-pr` | `/review-pr #123` | Review a PR with confidence scoring and category checks | | `triage-issues` | `/triage-issues` | Walk the Needs-Triage queue and recommend labels/assignee | | `dependabot-rollup` | `/dependabot-rollup` | Dry-run and optionally roll up at most 11 Dependabot patch/minor PRs | +| `assign-prs` | `/assign-prs` | Assign reviewers to the team review queues by area and current load | ## Package Layout diff --git a/change/@fluentui-react-headless-components-preview-2a6fb260-36cb-47d2-8495-c719a4952a0d.json b/change/@fluentui-react-headless-components-preview-2a6fb260-36cb-47d2-8495-c719a4952a0d.json new file mode 100644 index 00000000000000..491a522284761e --- /dev/null +++ b/change/@fluentui-react-headless-components-preview-2a6fb260-36cb-47d2-8495-c719a4952a0d.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "feat: export toDataAttributeValue from the utils entry point", + "packageName": "@fluentui/react-headless-components-preview", + "email": "dmytrokirpa@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json index 66d5e19222b520..3042e4e6862544 100644 --- a/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json +++ b/packages/react-components/react-headless-components-preview/library/bundle-isolation.config.json @@ -3,5 +3,7 @@ "fixturesRoot": "./bundle-size", "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], - "allowedViolations": {} + "fixtures": { + "AllComponents.fixture.js": {} + } } diff --git a/packages/react-components/react-headless-components-preview/library/etc/utils.api.md b/packages/react-components/react-headless-components-preview/library/etc/utils.api.md new file mode 100644 index 00000000000000..71fa91e7c2abe8 --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/etc/utils.api.md @@ -0,0 +1,12 @@ +## API Report File for "@fluentui/react-headless-components-preview" + +> Do not edit this file. It is a report generated by [API Extractor](https://api-extractor.com/). + +```ts + +// @public +export function toDataAttributeValue(value: boolean | string | number | undefined): string | undefined; + +// (No @packageDocumentation comment for this package) + +``` diff --git a/packages/react-components/react-headless-components-preview/library/package.json b/packages/react-components/react-headless-components-preview/library/package.json index af905dc9380b40..ae979fed08df40 100644 --- a/packages/react-components/react-headless-components-preview/library/package.json +++ b/packages/react-components/react-headless-components-preview/library/package.json @@ -587,6 +587,16 @@ "default": "./lib-commonjs/tooltip.cjs" } }, + "./utils": { + "import": { + "types": "./dist/utils.d.ts", + "default": "./lib/utils.js" + }, + "require": { + "types": "./dist/utils.d.cts", + "default": "./lib-commonjs/utils.cjs" + } + }, "./package.json": "./package.json" }, "beachball": { diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Accordion/AccordionHeader/useAccordionHeader.ts b/packages/react-components/react-headless-components-preview/library/src/components/Accordion/AccordionHeader/useAccordionHeader.ts index 41ac3a04cdca5a..8448cb4e2abe2d 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Accordion/AccordionHeader/useAccordionHeader.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Accordion/AccordionHeader/useAccordionHeader.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useAccordionHeaderBase_unstable, useAccordionHeaderContextValues_unstable } from '@fluentui/react-accordion'; import type { AccordionHeaderProps, AccordionHeaderState, AccordionHeaderContextValues } from './AccordionHeader.types'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; /** * Returns the state for an AccordionHeader component, given its props and ref. @@ -15,9 +15,9 @@ export const useAccordionHeader = (props: AccordionHeaderProps, ref: React.Ref) // Set data attributes for collapsible and multiple states to simplify styling of these states. // eslint-disable-next-line react-hooks/immutability - state.root['data-collapsible'] = stringifyDataAttribute(state.collapsible); + state.root['data-collapsible'] = toDataAttributeValue(state.collapsible); // eslint-disable-next-line react-hooks/immutability - state.root['data-multiple'] = stringifyDataAttribute(state.multiple); + state.root['data-multiple'] = toDataAttributeValue(state.multiple); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Breadcrumb/BreadcrumbButton/useBreadcrumbButton.ts b/packages/react-components/react-headless-components-preview/library/src/components/Breadcrumb/BreadcrumbButton/useBreadcrumbButton.ts index 9aadcb4716b512..311ddf7349d5f3 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Breadcrumb/BreadcrumbButton/useBreadcrumbButton.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Breadcrumb/BreadcrumbButton/useBreadcrumbButton.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useBreadcrumbButtonBase_unstable } from '@fluentui/react-breadcrumb'; import type { BreadcrumbButtonProps, BreadcrumbButtonState } from './BreadcrumbButton.types'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; /** * Returns the state for a BreadcrumbButton component, given its props and ref. @@ -18,7 +18,7 @@ export const useBreadcrumbButton = ( // Set data attribute for current state to simplify styling of the active breadcrumb item. // eslint-disable-next-line react-hooks/immutability - state.root['data-current'] = stringifyDataAttribute(state.current); + state.root['data-current'] = toDataAttributeValue(state.current); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Button/useButton.ts b/packages/react-components/react-headless-components-preview/library/src/components/Button/useButton.ts index 61b6c4bb22707b..978f634860f2a7 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Button/useButton.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Button/useButton.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useButtonBase_unstable } from '@fluentui/react-button'; import type { ButtonProps, ButtonState } from './Button.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Returns the state for a Button component, given its props and ref. @@ -15,11 +15,11 @@ export const useButton = (props: ButtonProps, ref: React.Ref): CardS const state: CardState = useCardBase_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability - state.root['data-selected'] = stringifyDataAttribute(state.selected); + state.root['data-selected'] = toDataAttributeValue(state.selected); // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Checkbox/useCheckbox.ts b/packages/react-components/react-headless-components-preview/library/src/components/Checkbox/useCheckbox.ts index 04d1b8790ec545..8c8707fab6d929 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Checkbox/useCheckbox.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Checkbox/useCheckbox.ts @@ -3,7 +3,7 @@ import type * as React from 'react'; import { useCheckboxBase_unstable } from '@fluentui/react-checkbox'; import type { CheckboxProps, CheckboxState } from './Checkbox.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Create the state required to render Checkbox. @@ -19,9 +19,9 @@ export const useCheckbox = (props: CheckboxProps, ref: React.Ref): AlphaSliderState => { const state: AlphaSliderState = useAlphaSliderBase_unstable(props, ref); @@ -13,7 +13,7 @@ export const useAlphaSlider = (props: AlphaSliderProps, ref: React.Ref): ComboboxState => { @@ -51,11 +51,11 @@ export const useCombobox = (props: ComboboxProps, ref: React.Ref): DrawerFooterState => { const state: DrawerFooterState = useDrawerFooter_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability - state.root['data-scroll-state'] = stringifyDataAttribute(state.scrollState); + state.root['data-scroll-state'] = toDataAttributeValue(state.scrollState); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Drawer/DrawerHeader/useDrawerHeader.ts b/packages/react-components/react-headless-components-preview/library/src/components/Drawer/DrawerHeader/useDrawerHeader.ts index 4a2a612b78f8d6..310706d442fc1d 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Drawer/DrawerHeader/useDrawerHeader.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Drawer/DrawerHeader/useDrawerHeader.ts @@ -3,7 +3,7 @@ import type * as React from 'react'; import { useDrawerHeader_unstable } from '@fluentui/react-drawer'; import type { DrawerHeaderProps, DrawerHeaderState } from './DrawerHeader.types'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; /** * Returns the state for a DrawerHeader component, given its props and ref. @@ -11,7 +11,7 @@ import { stringifyDataAttribute } from '../../../utils'; export const useDrawerHeader = (props: DrawerHeaderProps, ref: React.Ref): DrawerHeaderState => { const state: DrawerHeaderState = useDrawerHeader_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability - state.root['data-scroll-state'] = stringifyDataAttribute(state.scrollState); + state.root['data-scroll-state'] = toDataAttributeValue(state.scrollState); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Drawer/InlineDrawer/useInlineDrawer.ts b/packages/react-components/react-headless-components-preview/library/src/components/Drawer/InlineDrawer/useInlineDrawer.ts index b811a82bc1c84d..e83ad25da200be 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Drawer/InlineDrawer/useInlineDrawer.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Drawer/InlineDrawer/useInlineDrawer.ts @@ -3,7 +3,7 @@ import type * as React from 'react'; import { useInlineDrawerBase_unstable } from '@fluentui/react-drawer'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; import type { InlineDrawerProps, InlineDrawerState } from './InlineDrawer.types'; /** @@ -13,7 +13,7 @@ export const useInlineDrawer = (props: InlineDrawerProps, ref: React.Ref): Opti ...baseState, root: { ...baseState.root, - 'data-disabled': stringifyDataAttribute(baseState.disabled), - 'data-selected': stringifyDataAttribute(baseState.selected), - 'data-multiselect': stringifyDataAttribute(baseState.multiselect), + 'data-disabled': toDataAttributeValue(baseState.disabled), + 'data-selected': toDataAttributeValue(baseState.selected), + 'data-multiselect': toDataAttributeValue(baseState.multiselect), }, }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Dropdown/useDropdown.ts b/packages/react-components/react-headless-components-preview/library/src/components/Dropdown/useDropdown.ts index 7c65d667739575..86c2b1d8038ef5 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Dropdown/useDropdown.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Dropdown/useDropdown.ts @@ -6,7 +6,7 @@ import { mergeCallbacks, slot, useEventCallback, useMergedRefs } from '@fluentui import type { DropdownProps, DropdownState } from './Dropdown.types'; import { useButtonTriggerSlot } from '@fluentui/react-combobox'; import { Listbox } from './Listbox'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; import { useListboxPopupState } from './useListboxPopupState'; /** @@ -63,11 +63,11 @@ export const useDropdown = (props: DropdownProps, ref: React.Ref setPopoverOpen(data.open)); const infoRef = useMergedRefs(state.info.ref); diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Input/useInput.ts b/packages/react-components/react-headless-components-preview/library/src/components/Input/useInput.ts index 97ea19896f3852..317893e0649d74 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Input/useInput.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Input/useInput.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useInputBase_unstable } from '@fluentui/react-input'; import type { InputProps, InputState } from './Input.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Returns the state for an Input component, given its props and ref. @@ -15,11 +15,9 @@ export const useInput = (props: InputProps, ref: React.Ref): I // Set data attribute for disabled state to simplify styling. // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.input.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.input.disabled); // eslint-disable-next-line react-hooks/immutability - state.root['data-invalid'] = stringifyDataAttribute( - state.input['aria-invalid'] === true || state.input['aria-invalid'] === 'true', - ); + state.root['data-invalid'] = toDataAttributeValue(state.input['aria-invalid']); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/InteractionTagPrimary/useInteractionTagPrimary.ts b/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/InteractionTagPrimary/useInteractionTagPrimary.ts index 9f75f97d897437..61ab994ef2d4f9 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/InteractionTagPrimary/useInteractionTagPrimary.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/InteractionTagPrimary/useInteractionTagPrimary.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useInteractionTagPrimaryBase_unstable } from '@fluentui/react-tags'; import type { InteractionTagPrimaryProps, InteractionTagPrimaryState } from './InteractionTagPrimary.types'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; /** * Returns the state for an InteractionTagPrimary component, given its props and ref. @@ -17,9 +17,9 @@ export const useInteractionTagPrimary = ( const state: InteractionTagPrimaryState = useInteractionTagPrimaryBase_unstable(props, ref); /* eslint-disable react-hooks/immutability -- intentional: decorate base state with data-* attrs for styling */ - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); - state.root['data-selected'] = stringifyDataAttribute(state.selected); - state.root['data-has-secondary-action'] = stringifyDataAttribute(state.hasSecondaryAction); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); + state.root['data-selected'] = toDataAttributeValue(state.selected); + state.root['data-has-secondary-action'] = toDataAttributeValue(state.hasSecondaryAction); /* eslint-enable react-hooks/immutability */ return state; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/InteractionTagSecondary/useInteractionTagSecondary.ts b/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/InteractionTagSecondary/useInteractionTagSecondary.ts index c5c19fd39c63b2..77704c8215aff7 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/InteractionTagSecondary/useInteractionTagSecondary.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/InteractionTagSecondary/useInteractionTagSecondary.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useInteractionTagSecondaryBase_unstable } from '@fluentui/react-tags'; import type { InteractionTagSecondaryProps, InteractionTagSecondaryState } from './InteractionTagSecondary.types'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; /** * Returns the state for an InteractionTagSecondary component, given its props and ref. @@ -17,8 +17,8 @@ export const useInteractionTagSecondary = ( const state: InteractionTagSecondaryState = useInteractionTagSecondaryBase_unstable(props, ref); /* eslint-disable react-hooks/immutability -- intentional: decorate base state with data-* attrs for styling */ - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); - state.root['data-selected'] = stringifyDataAttribute(state.selected); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); + state.root['data-selected'] = toDataAttributeValue(state.selected); /* eslint-enable react-hooks/immutability */ return state; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/useInteractionTag.ts b/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/useInteractionTag.ts index 9f28391454e253..81b7e7065e8c40 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/useInteractionTag.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/InteractionTag/useInteractionTag.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useInteractionTagBase_unstable } from '@fluentui/react-tags'; import type { InteractionTagProps, InteractionTagState } from './InteractionTag.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Returns the state for an InteractionTag component, given its props and ref. @@ -14,8 +14,8 @@ export const useInteractionTag = (props: InteractionTagProps, ref: React.Ref): L // Set data attribute for disabled state to simplify styling. // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); // eslint-disable-next-line react-hooks/immutability - state.root['data-required'] = stringifyDataAttribute(Boolean(state.required)); + state.root['data-required'] = toDataAttributeValue(Boolean(state.required)); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Link/useLink.ts b/packages/react-components/react-headless-components-preview/library/src/components/Link/useLink.ts index 5cf12d344afcda..ef5a68648e5c82 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Link/useLink.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Link/useLink.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useLinkBase_unstable } from '@fluentui/react-link'; import type { LinkProps, LinkState } from './Link.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Returns the state for a Link component, given its props and ref. @@ -14,9 +14,9 @@ export const useLink = (props: LinkProps, ref: React.Ref): LinkStat const state: LinkState = useLinkBase_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled-focusable'] = stringifyDataAttribute(state.disabledFocusable); + state.root['data-disabled-focusable'] = toDataAttributeValue(state.disabledFocusable); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItem/useMenuItem.ts b/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItem/useMenuItem.ts index 96b6ddac35ebd1..d92d392dc8b6cf 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItem/useMenuItem.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItem/useMenuItem.ts @@ -5,7 +5,7 @@ import { useMenuItemBase_unstable } from '@fluentui/react-menu'; import type { ARIAButtonElement } from '@fluentui/react-aria'; import type { MenuItemProps, MenuItemState } from './MenuItem.types'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; /** Returns the state for a MenuItem; adds `focusgroupstart` so the focusgroup polyfill anchors the initial tab stop on the first item. */ export const useMenuItem = (props: MenuItemProps, ref: React.Ref>): MenuItemState => { @@ -14,11 +14,11 @@ export const useMenuItem = (props: MenuItemProps, ref: React.Ref): MenuItemLinkState => { const state: MenuItemLinkState = useMenuItemLinkBase_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(props.disabled); + state.root['data-disabled'] = toDataAttributeValue(props.disabled); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItemRadio/useMenuItemRadio.ts b/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItemRadio/useMenuItemRadio.ts index 0170739e191132..93b9f1e911363c 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItemRadio/useMenuItemRadio.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItemRadio/useMenuItemRadio.ts @@ -3,7 +3,7 @@ import type * as React from 'react'; import type { ARIAButtonElement } from '@fluentui/react-aria'; import { useMenuItemRadioBase_unstable } from '@fluentui/react-menu'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; import type { MenuItemRadioProps, MenuItemRadioState } from './MenuItemRadio.types'; export const useMenuItemRadio = ( @@ -13,13 +13,13 @@ export const useMenuItemRadio = ( const state: MenuItemRadioState = useMenuItemRadioBase_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); // eslint-disable-next-line react-hooks/immutability - state.root['data-has-submenu'] = stringifyDataAttribute(state.hasSubmenu); + state.root['data-has-submenu'] = toDataAttributeValue(state.hasSubmenu); // eslint-disable-next-line react-hooks/immutability - state.root['data-submenu-open'] = stringifyDataAttribute(state.submenuOpen); + state.root['data-submenu-open'] = toDataAttributeValue(state.submenuOpen); // eslint-disable-next-line react-hooks/immutability - state.root['data-checked'] = stringifyDataAttribute(state.checked); + state.root['data-checked'] = toDataAttributeValue(state.checked); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItemSwitch/useMenuItemSwitch.ts b/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItemSwitch/useMenuItemSwitch.ts index 4bbcc0d966c7f0..43188c506a7356 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItemSwitch/useMenuItemSwitch.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuItemSwitch/useMenuItemSwitch.ts @@ -2,16 +2,16 @@ import type * as React from 'react'; import { useMenuItemSwitchBase_unstable } from '@fluentui/react-menu'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; import type { MenuItemSwitchProps, MenuItemSwitchState } from './MenuItemSwitch.types'; export const useMenuItemSwitch = (props: MenuItemSwitchProps, ref: React.Ref): MenuItemSwitchState => { const state: MenuItemSwitchState = useMenuItemSwitchBase_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); // eslint-disable-next-line react-hooks/immutability - state.root['data-checked'] = stringifyDataAttribute(state.checked); + state.root['data-checked'] = toDataAttributeValue(state.checked); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuTrigger/useMenuTrigger.ts b/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuTrigger/useMenuTrigger.ts index ff568785d8574a..1ad6a1c1c9f70d 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuTrigger/useMenuTrigger.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Menu/MenuTrigger/useMenuTrigger.ts @@ -5,7 +5,7 @@ import { useMenuTriggerBase_unstable } from '@fluentui/react-menu'; import { useFluent_unstable as useFluent } from '@fluentui/react-shared-contexts'; import { isHTMLElement } from '@fluentui/react-utilities'; import { useMenuContext } from '../menuContext'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; import type { MenuTriggerProps, MenuTriggerState } from '@fluentui/react-menu'; type TriggerChildProps = { @@ -50,7 +50,7 @@ export const useMenuTrigger = (props: MenuTriggerProps): MenuTriggerState => { const child = baseState.children as React.ReactElement; const overrideProps: TriggerChildProps = { - 'data-open': stringifyDataAttribute(open), + 'data-open': toDataAttributeValue(open), }; if (openOnContext && targetDocument) { diff --git a/packages/react-components/react-headless-components-preview/library/src/components/MenuButton/useMenuButton.ts b/packages/react-components/react-headless-components-preview/library/src/components/MenuButton/useMenuButton.ts index be7d6028d29ffe..073fb0230ddb20 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/MenuButton/useMenuButton.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/MenuButton/useMenuButton.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useMenuButtonBase_unstable } from '@fluentui/react-button'; import type { MenuButtonProps, MenuButtonState } from './MenuButton.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Returns the state for a MenuButton component, given its props and ref. @@ -18,13 +18,13 @@ export const useMenuButton = ( // Set data attributes for disabled, disabledFocusable, and iconOnly states to simplify styling. // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled-focusable'] = stringifyDataAttribute(state.disabledFocusable); + state.root['data-disabled-focusable'] = toDataAttributeValue(state.disabledFocusable); // eslint-disable-next-line react-hooks/immutability - state.root['data-icon-only'] = stringifyDataAttribute(state.iconOnly); + state.root['data-icon-only'] = toDataAttributeValue(state.iconOnly); // eslint-disable-next-line react-hooks/immutability - state.root['data-open'] = stringifyDataAttribute( + state.root['data-open'] = toDataAttributeValue( state.root['aria-expanded'] === true || state.root['aria-expanded'] === 'true', ); diff --git a/packages/react-components/react-headless-components-preview/library/src/components/MessageBar/MessageBarActions/useMessageBarActions.ts b/packages/react-components/react-headless-components-preview/library/src/components/MessageBar/MessageBarActions/useMessageBarActions.ts index 296fb128a3c277..26d26982805bd7 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/MessageBar/MessageBarActions/useMessageBarActions.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/MessageBar/MessageBarActions/useMessageBarActions.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useMessageBarActions_unstable } from '@fluentui/react-message-bar'; import type { MessageBarActionsProps, MessageBarActionsState } from './MessageBarActions.types'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; /** * Returns the state for a MessageBarActions component, given its props and ref. @@ -19,7 +19,7 @@ export const useMessageBarActions = ( // eslint-disable-next-line react-hooks/immutability state.root['data-layout'] = state.layout; // eslint-disable-next-line react-hooks/immutability - state.root['data-has-actions'] = stringifyDataAttribute(state.hasActions); + state.root['data-has-actions'] = toDataAttributeValue(state.hasActions); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/MessageBar/useMessageBar.ts b/packages/react-components/react-headless-components-preview/library/src/components/MessageBar/useMessageBar.ts index a2eb2547918c27..dfb276a5abba47 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/MessageBar/useMessageBar.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/MessageBar/useMessageBar.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useMessageBarBase_unstable, useMessageBarContextValue_unstable } from '@fluentui/react-message-bar'; import type { MessageBarProps, MessageBarState, MessageBarContextValues } from './MessageBar.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Returns the state for a MessageBar component, given its props and ref. @@ -16,7 +16,7 @@ export const useMessageBar = (props: MessageBarProps, ref: React.Ref): R // Set data attribute for disabled state to simplify styling. // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.input.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.input.disabled); // eslint-disable-next-line react-hooks/immutability state.root['data-label-position'] = state.labelPosition; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/SearchBox/useSearchBox.ts b/packages/react-components/react-headless-components-preview/library/src/components/SearchBox/useSearchBox.ts index 5ce1f5ca64007e..a5a2df5e2546ff 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/SearchBox/useSearchBox.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/SearchBox/useSearchBox.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useSearchBoxBase_unstable } from '@fluentui/react-search'; import type { SearchBoxProps, SearchBoxState } from './Search.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Returns the state for a SearchBox component, given its props and ref. @@ -15,9 +15,9 @@ export const useSearchBox = (props: SearchBoxProps, ref: React.Ref) // Set data attribute for disabled state to simplify styling. // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.select.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.select.disabled); // eslint-disable-next-line react-hooks/immutability - state.root['data-invalid'] = stringifyDataAttribute( - state.select['aria-invalid'] === true || state.select['aria-invalid'] === 'true', - ); + state.root['data-invalid'] = toDataAttributeValue(state.select['aria-invalid']); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Slider/useSlider.ts b/packages/react-components/react-headless-components-preview/library/src/components/Slider/useSlider.ts index d26d8c6ab2ba53..a8cbc0e69279e6 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Slider/useSlider.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Slider/useSlider.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useSliderBase_unstable } from '@fluentui/react-slider'; import type { SliderProps, SliderState } from './Slider.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Returns the state for a Slider component, given its props and ref. @@ -15,9 +15,9 @@ export const useSlider = (props: SliderProps, ref: React.Ref): // Set data attributes for disabled and vertical states to simplify styling of these states. // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); // eslint-disable-next-line react-hooks/immutability - state.root['data-vertical'] = stringifyDataAttribute(state.vertical); + state.root['data-vertical'] = toDataAttributeValue(state.vertical); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/SpinButton/useSpinButton.ts b/packages/react-components/react-headless-components-preview/library/src/components/SpinButton/useSpinButton.ts index 02e475fcd44c24..e48d2e699ec12f 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/SpinButton/useSpinButton.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/SpinButton/useSpinButton.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useSpinButtonBase_unstable } from '@fluentui/react-spinbutton'; import type { SpinButtonProps, SpinButtonState } from './SpinButton.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Returns the state for a SpinButton component, given its props and ref. @@ -15,15 +15,13 @@ export const useSpinButton = (props: SpinButtonProps, ref: React.Ref): ColorSwatchState => { const state: ColorSwatchState = useColorSwatchBase_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability - state.root['data-selected'] = stringifyDataAttribute(state.selected); + state.root['data-selected'] = toDataAttributeValue(state.selected); // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/SwatchPicker/EmptySwatch/useEmptySwatch.ts b/packages/react-components/react-headless-components-preview/library/src/components/SwatchPicker/EmptySwatch/useEmptySwatch.ts index 20c685ba560a72..3428d22d51cb1a 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/SwatchPicker/EmptySwatch/useEmptySwatch.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/SwatchPicker/EmptySwatch/useEmptySwatch.ts @@ -2,16 +2,16 @@ import type * as React from 'react'; import { useEmptySwatchBase_unstable } from '@fluentui/react-swatch-picker'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; import type { EmptySwatchProps, EmptySwatchState } from './EmptySwatch.types'; export const useEmptySwatch = (props: EmptySwatchProps, ref: React.Ref): EmptySwatchState => { const state: EmptySwatchState = useEmptySwatchBase_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability - state.root['data-selected'] = stringifyDataAttribute(state.root['aria-checked']); + state.root['data-selected'] = toDataAttributeValue(state.root['aria-checked']); // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.root.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.root.disabled); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/SwatchPicker/ImageSwatch/useImageSwatch.ts b/packages/react-components/react-headless-components-preview/library/src/components/SwatchPicker/ImageSwatch/useImageSwatch.ts index e73fc3309cfe28..80c3ac24c79cad 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/SwatchPicker/ImageSwatch/useImageSwatch.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/SwatchPicker/ImageSwatch/useImageSwatch.ts @@ -2,14 +2,14 @@ import type * as React from 'react'; import { useImageSwatchBase_unstable } from '@fluentui/react-swatch-picker'; -import { stringifyDataAttribute } from '../../../utils/stringifyDataAttribute'; +import { toDataAttributeValue } from '../../../utils/toDataAttributeValue'; import type { ImageSwatchProps, ImageSwatchState } from './ImageSwatch.types'; export const useImageSwatch = (props: ImageSwatchProps, ref: React.Ref): ImageSwatchState => { const state: ImageSwatchState = useImageSwatchBase_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability - state.root['data-selected'] = stringifyDataAttribute(state.selected); + state.root['data-selected'] = toDataAttributeValue(state.selected); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Switch/useSwitch.ts b/packages/react-components/react-headless-components-preview/library/src/components/Switch/useSwitch.ts index 8f5d1b58f00db9..6ac5b0ab805f77 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Switch/useSwitch.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Switch/useSwitch.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useSwitchBase_unstable } from '@fluentui/react-switch'; import type { SwitchProps, SwitchState } from './Switch.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Returns the state for a Switch component, given its props and ref. @@ -15,11 +15,11 @@ export const useSwitch = (props: SwitchProps, ref: React.Ref): // Set data attributes for disabled, disabledFocusable, and checked states to simplify styling. // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.input.disabled || state.disabledFocusable); + state.root['data-disabled'] = toDataAttributeValue(state.input.disabled || state.disabledFocusable); // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled-focusable'] = stringifyDataAttribute(state.disabledFocusable); + state.root['data-disabled-focusable'] = toDataAttributeValue(state.disabledFocusable); // eslint-disable-next-line react-hooks/immutability - state.root['data-checked'] = stringifyDataAttribute(state.input.checked); + state.root['data-checked'] = toDataAttributeValue(state.input.checked); // eslint-disable-next-line react-hooks/immutability state.root['data-label-position'] = state.labelPosition; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/TabList/Tab/useTab.ts b/packages/react-components/react-headless-components-preview/library/src/components/TabList/Tab/useTab.ts index 8ee23959743c38..23248e7d6cf8f2 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/TabList/Tab/useTab.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/TabList/Tab/useTab.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useTabBase_unstable } from '@fluentui/react-tabs'; import type { TabProps, TabState } from './Tab.types'; -import { stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; /** * Returns the state for a Tab component, given its props and ref. @@ -14,13 +14,13 @@ export const useTab = (props: TabProps, ref: React.Ref): TabState = const state: TabState = useTabBase_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability - state.root.focusgroupstart = stringifyDataAttribute(state.selected); + state.root.focusgroupstart = toDataAttributeValue(state.selected); // eslint-disable-next-line react-hooks/immutability - state.root['data-icon-only'] = stringifyDataAttribute(state.iconOnly); + state.root['data-icon-only'] = toDataAttributeValue(state.iconOnly); // eslint-disable-next-line react-hooks/immutability - state.root['data-selected'] = stringifyDataAttribute(state.selected); + state.root['data-selected'] = toDataAttributeValue(state.selected); // eslint-disable-next-line react-hooks/immutability - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Tag/useTag.ts b/packages/react-components/react-headless-components-preview/library/src/components/Tag/useTag.ts index b9f336ce7dc47d..6497bf725b28bf 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Tag/useTag.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Tag/useTag.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useTagBase_unstable } from '@fluentui/react-tags'; import type { TagProps, TagState } from './Tag.types'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Returns the state for a Tag component, given its props and ref. @@ -14,9 +14,9 @@ export const useTag = (props: TagProps, ref: React.Ref /* eslint-disable react-hooks/immutability -- intentional: decorate base state with focusgroup + data-* attrs */ state.root.focusgroup = 'toolbar inline wrap'; - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); - state.root['data-dismissible'] = stringifyDataAttribute(state.dismissible); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); + state.root['data-dismissible'] = toDataAttributeValue(state.dismissible); /* eslint-enable react-hooks/immutability */ return state; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerButton/useTagPickerButton.ts b/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerButton/useTagPickerButton.ts index 9324eb4d5c2818..22bfcc74f5f5a5 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerButton/useTagPickerButton.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerButton/useTagPickerButton.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useTagPickerButtonBase_unstable } from '@fluentui/react-tag-picker'; import type { TagPickerButtonProps, TagPickerButtonState } from './TagPickerButton.types'; -import { stringifyDataAttribute } from '../../../utils/stringifyDataAttribute'; +import { toDataAttributeValue } from '../../../utils/toDataAttributeValue'; /** * Returns the state for a headless TagPickerButton. @@ -16,7 +16,7 @@ export const useTagPickerButton = ( const state: TagPickerButtonState = useTagPickerButtonBase_unstable(props, ref); // eslint-disable-next-line react-hooks/immutability -- decorate base state with data-* attribute - state.root['data-disabled'] = stringifyDataAttribute(state.root.disabled); + state.root['data-disabled'] = toDataAttributeValue(state.root.disabled); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerControl/useTagPickerControl.ts b/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerControl/useTagPickerControl.ts index 9913a01175e0a2..f7e26f03617a0d 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerControl/useTagPickerControl.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerControl/useTagPickerControl.ts @@ -4,7 +4,7 @@ import type * as React from 'react'; import { useTagPickerControlBase_unstable } from '@fluentui/react-tag-picker'; import type { TagPickerControlProps, TagPickerControlState } from './TagPickerControl.types'; -import { stringifyDataAttribute } from '../../../utils/stringifyDataAttribute'; +import { toDataAttributeValue } from '../../../utils/toDataAttributeValue'; /** * Returns the state for a headless TagPickerControl. @@ -19,8 +19,8 @@ export const useTagPickerControl = ( const state: TagPickerControlState = useTagPickerControlBase_unstable(props, ref); /* eslint-disable react-hooks/immutability -- decorate base state with data-* attributes */ - state.root['data-disabled'] = stringifyDataAttribute(state.disabled); - state.root['data-invalid'] = stringifyDataAttribute(state.invalid); + state.root['data-disabled'] = toDataAttributeValue(state.disabled); + state.root['data-invalid'] = toDataAttributeValue(state.invalid); /* eslint-enable react-hooks/immutability */ return state; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerGroup/useTagPickerGroup.ts b/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerGroup/useTagPickerGroup.ts index 0b02c121b78ae2..9eb753569beaf9 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerGroup/useTagPickerGroup.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/TagPicker/TagPickerGroup/useTagPickerGroup.ts @@ -7,7 +7,8 @@ import { isHTMLElement, useEventCallback, useMergedRefs } from '@fluentui/react- import { ArrowRight } from '@fluentui/keyboard-keys'; import type { TagPickerGroupProps, TagPickerGroupState } from './TagPickerGroup.types'; -import { isLastFocusableTag, stringifyDataAttribute } from '../../../utils'; +import { toDataAttributeValue } from '../../../utils'; +import { isLastFocusableTag } from '../../../utils/tagFocusUtils'; /** * Returns the state for a headless TagPickerGroup. @@ -54,7 +55,7 @@ export const useTagPickerGroup = (props: TagPickerGroupProps, ref: React.Ref): To // eslint-disable-next-line react-hooks/immutability state.root.focusgroup = `toolbar ${state.vertical ? 'block' : 'inline'} wrap`; // eslint-disable-next-line react-hooks/immutability - state.root['data-vertical'] = stringifyDataAttribute(state.vertical); + state.root['data-vertical'] = toDataAttributeValue(state.vertical); return state; }; diff --git a/packages/react-components/react-headless-components-preview/library/src/components/Tooltip/useTooltip.ts b/packages/react-components/react-headless-components-preview/library/src/components/Tooltip/useTooltip.ts index 0bf96cc0846798..2d342b62395703 100644 --- a/packages/react-components/react-headless-components-preview/library/src/components/Tooltip/useTooltip.ts +++ b/packages/react-components/react-headless-components-preview/library/src/components/Tooltip/useTooltip.ts @@ -21,7 +21,7 @@ import { KEYBORG_FOCUSIN, useIsNavigatingWithKeyboard } from '@fluentui/react-ta import type { OnVisibleChangeData, TooltipProps, TooltipState, TooltipTriggerProps } from './Tooltip.types'; import { resolvePositioningShorthand, usePositioning } from '../../positioning'; -import { stringifyDataAttribute } from '../../utils'; +import { toDataAttributeValue } from '../../utils'; /** * Create the state required to render Tooltip. @@ -70,7 +70,7 @@ export const useTooltip = (props: TooltipProps): TooltipState => { const { targetRef, containerRef } = usePositioning(positioningOptions); state.content.id = useId('tooltip-', state.content.id); - state.content['data-open'] = stringifyDataAttribute(state.visible); + state.content['data-open'] = toDataAttributeValue(state.visible); const contentRef = useMergedRefs(state.content.ref, containerRef); state.content.ref = contentRef; diff --git a/packages/react-components/react-headless-components-preview/library/src/utils.ts b/packages/react-components/react-headless-components-preview/library/src/utils.ts new file mode 100644 index 00000000000000..9b87d2d7eaca89 --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/src/utils.ts @@ -0,0 +1 @@ +export { toDataAttributeValue } from './utils/toDataAttributeValue'; diff --git a/packages/react-components/react-headless-components-preview/library/src/utils/index.ts b/packages/react-components/react-headless-components-preview/library/src/utils/index.ts index 3ef52e1d67cb97..ed2201f713f9b5 100644 --- a/packages/react-components/react-headless-components-preview/library/src/utils/index.ts +++ b/packages/react-components/react-headless-components-preview/library/src/utils/index.ts @@ -1,2 +1,2 @@ -export { stringifyDataAttribute } from './stringifyDataAttribute'; +export { toDataAttributeValue } from './toDataAttributeValue'; export { focusLastTag, isLastFocusableTag } from './tagFocusUtils'; diff --git a/packages/react-components/react-headless-components-preview/library/src/utils/stringifyDataAttribute.test.ts b/packages/react-components/react-headless-components-preview/library/src/utils/stringifyDataAttribute.test.ts deleted file mode 100644 index 06e62f0b9744c5..00000000000000 --- a/packages/react-components/react-headless-components-preview/library/src/utils/stringifyDataAttribute.test.ts +++ /dev/null @@ -1,14 +0,0 @@ -import { stringifyDataAttribute } from './stringifyDataAttribute'; - -describe('stringifyDataAttribute', () => { - it.each([ - [true, ''], - [false, undefined], - [undefined, undefined], - ['test', 'test'], - [123, '123'], - [0, '0'], - ])('should convert %p to %p', (input, expected) => { - expect(stringifyDataAttribute(input)).toBe(expected); - }); -}); diff --git a/packages/react-components/react-headless-components-preview/library/src/utils/stringifyDataAttribute.ts b/packages/react-components/react-headless-components-preview/library/src/utils/stringifyDataAttribute.ts deleted file mode 100644 index 59b058220dac06..00000000000000 --- a/packages/react-components/react-headless-components-preview/library/src/utils/stringifyDataAttribute.ts +++ /dev/null @@ -1,17 +0,0 @@ -/** - * Converts a boolean, string, number, or undefined value into a string suitable for use as a data attribute value. - * - `true` becomes an empty string (indicating the presence of the attribute). - * - `false` and `undefined` become `undefined` (indicating the absence of the attribute). - * - Other values are converted to strings. - */ -export function stringifyDataAttribute(value: boolean | string | number | undefined): string | undefined { - if (value === true) { - return ''; - } - - if (value === false || value === undefined) { - return undefined; - } - - return String(value); -} diff --git a/packages/react-components/react-headless-components-preview/library/src/utils/toDataAttributeValue.test.ts b/packages/react-components/react-headless-components-preview/library/src/utils/toDataAttributeValue.test.ts new file mode 100644 index 00000000000000..c16de74d8016ee --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/src/utils/toDataAttributeValue.test.ts @@ -0,0 +1,16 @@ +import { toDataAttributeValue } from './toDataAttributeValue'; + +describe('toDataAttributeValue', () => { + it.each([ + [true, ''], + ['true', ''], + [false, undefined], + ['false', undefined], + [undefined, undefined], + ['test', 'test'], + [123, '123'], + [0, '0'], + ])('should convert %p to %p', (input, expected) => { + expect(toDataAttributeValue(input)).toBe(expected); + }); +}); diff --git a/packages/react-components/react-headless-components-preview/library/src/utils/toDataAttributeValue.ts b/packages/react-components/react-headless-components-preview/library/src/utils/toDataAttributeValue.ts new file mode 100644 index 00000000000000..da7d707d0ffb1a --- /dev/null +++ b/packages/react-components/react-headless-components-preview/library/src/utils/toDataAttributeValue.ts @@ -0,0 +1,27 @@ +/** + * Converts a value to the representation used by headless `data-*` state attributes. + * + * Boolean values and their string equivalents are represented by the presence or absence of the attribute: + * - `true` and `'true'` become an empty string, indicating that the attribute is present. + * - `false`, `'false'`, and `undefined` become `undefined`, indicating that the attribute is omitted. + * - Other string and number values are converted to strings. + * + * @example + * ```ts + * toDataAttributeValue(true); // '' + * toDataAttributeValue(false); // undefined + * toDataAttributeValue('mixed'); // 'mixed' + * toDataAttributeValue(0); // '0' + * ``` + */ +export function toDataAttributeValue(value: boolean | string | number | undefined): string | undefined { + if (value === true || value === 'true') { + return ''; + } + + if (value === false || value === 'false' || value === undefined) { + return undefined; + } + + return String(value); +} diff --git a/scripts/beachball/base.config.js b/scripts/beachball/base.config.js index f5703fd32664dd..d14f984c54b7d0 100644 --- a/scripts/beachball/base.config.js +++ b/scripts/beachball/base.config.js @@ -22,6 +22,7 @@ const config = { '**/.storybook/**', '**/bundle-size/**', '**/monosize.config.mjs', + '**/bundle-isolation.config.json', '**/common/isConformant.ts', '**/src/testing/**', '**/src/e2e/**', diff --git a/scripts/beachball/src/config.test.ts b/scripts/beachball/src/config.test.ts index 593e22dc3d55c4..42b906f85d763b 100644 --- a/scripts/beachball/src/config.test.ts +++ b/scripts/beachball/src/config.test.ts @@ -51,6 +51,7 @@ describe(`beachball configs`, () => { '**/.storybook/**', '**/bundle-size/**', '**/monosize.config.mjs', + '**/bundle-isolation.config.json', '**/common/isConformant.ts', '**/src/testing/**', '**/src/e2e/**', diff --git a/tools/verify-bundle-isolation/README.md b/tools/verify-bundle-isolation/README.md index 9b2fc31f6dddac..7dcf1916b5d54e 100644 --- a/tools/verify-bundle-isolation/README.md +++ b/tools/verify-bundle-isolation/README.md @@ -67,25 +67,26 @@ changed module resolution in a way that makes these packages resolve to sources ## Verdicts -| Verdict | Exit | Meaning | -| ---------------- | ---- | ---------------------------------------------------------------------------------------------------------------------------------- | -| `PASS` | 0 | No forbidden package survived bundling. Only this verdict claims a bundle is free of them. | -| `PASS WITH DEBT` | 0 | Every surviving forbidden package is on the allowlist. The leaks are listed with their module counts and entry points. | -| `FAIL` | 1 | A regression, a stale or orphaned allowlist entry, a fixture that failed to bundle, or — under `--strict` — any allowed violation. | +| Verdict | Exit | Meaning | +| ---------------- | ---- | ------------------------------------------------------------------------------------------------------------------------------------------------- | +| `PASS` | 0 | No forbidden package survived bundling. Only this verdict claims a bundle is free of them. | +| `PASS WITH DEBT` | 0 | Every surviving forbidden package is on the allowlist. The leaks are listed with their module counts and entry points. | +| `FAIL` | 1 | A regression, a stale allowlist entry, an orphaned fixture entry, a fixture that failed to bundle, or — under `--strict` — any allowed violation. | Per fixture the report labels each finding `CLEAN`, `ALLOWED`, `REGRESSION`, `STALE` or `ERROR`; a single fixture can -carry more than one label. Module and export counts come from a build with `minimize: false`, so they measure how much -of a package is retained, not what it costs to ship — use monosize for bytes. +carry more than one label. Fixtures found in `fixturesRoot` but not opted in are listed under `SKIPPED`. Module and +export counts come from a build with `minimize: false`, so they measure how much of a package is retained, not what it +costs to ship — use monosize for bytes. ## Output `dist/bundle-isolation/` is wiped on every run, so it only ever contains the fixtures that currently exist. -| Path | Written | Contents | -| ----------------------- | ---------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------- | -| `summary.json` | always | The console verdict in structured form — overall `status`, and per fixture its `status`, `allowedViolations`, `tolerated`, `regressions`, `stale` and full `leaks` map | -| `/report.html` | with `--analyze` | webpack-bundle-analyzer treemap | -| `/report.json` | with `--analyze` | The same data the treemap renders from — module tree with `statSize`, `parsedSize` and `gzipSize` | +| Path | Written | Contents | +| ----------------------- | ---------------- | -------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `summary.json` | always | The console verdict in structured form — overall `status`, `skippedFixtures`, `orphanedFixtureEntries`, and per fixture its `status`, effective `forbiddenPackages`, `allowedViolations`, `tolerated`, `regressions`, `stale` and full `leaks` map | +| `/report.html` | with `--analyze` | webpack-bundle-analyzer treemap | +| `/report.json` | with `--analyze` | The same data the treemap renders from — module tree with `statSize`, `parsedSize` and `gzipSize` | `leaks` maps a forbidden package to the exports that survived tree shaking and the modules importing them, so the summary answers _what_ leaked and _why_, while the analyzer output answers _how much_ it costs. @@ -100,8 +101,10 @@ summary answers _what_ leaked and _why_, while the analyzer output answers _how "fixturesRoot": "./bundle-size", "externals": ["react", "react-dom", "react/jsx-runtime", "react/compiler-runtime"], "forbiddenPackages": ["tabster", "@griffel/*", "@fluentui/react-icons"], - "allowedViolations": { - "AllComponents.fixture.js": ["@fluentui/react-icons"] + "fixtures": { + "AllComponents.fixture.js": { "allowedViolations": ["@fluentui/react-icons"] }, + "TagPicker.fixture.js": {}, + "Headless.fixture.js": { "forbiddenPackages": ["@griffel/*"] } } } ``` @@ -111,13 +114,16 @@ All configured paths are resolved relative to the package root: - `fixturesRoot` is the directory containing bundle-size fixtures. - `externals` lists host-provided modules excluded from the bundle. - `forbiddenPackages` lists exact package names or scoped globs such as `@griffel/*`. -- `allowedViolations` maps fixture paths, relative to `fixturesRoot` and always with forward slashes, to tolerated - forbidden packages. +- `fixtures` selects which fixtures to verify, keyed by path relative to `fixturesRoot` and always with forward slashes. -The two lists do not take the same values. `forbiddenPackages` declares intent, so it accepts globs. `allowedViolations` -records what actually leaked, so it takes **exact resolved package names** and rejects globs - `@griffel/*` there would -let a newly leaked `@griffel/anything` hide behind an entry approved for something else. The debt has to name what it -is: `@griffel/core` and `@griffel/react`, separately. +A fixture entry takes two optional keys. `allowedViolations` records tolerated forbidden packages for that fixture. +`forbiddenPackages` **replaces** the package-level list for that fixture — it overrides rather than extends, because +otherwise a fixture could only ever widen the intent, never narrow it. + +The two package lists do not take the same values. `forbiddenPackages` declares intent, so it accepts globs. +`allowedViolations` records what actually leaked, so it takes **exact resolved package names** and rejects globs - +`@griffel/*` there would let a newly leaked `@griffel/anything` hide behind an entry approved for something else. The +debt has to name what it is: `@griffel/core` and `@griffel/react`, separately. `$schema` has to be a workspace-relative path. Editors resolve it against the config file and do not apply Node package resolution, so `@fluentui/verify-bundle-isolation/schema.json` will not work there despite the export map. The export @@ -142,13 +148,26 @@ export default { Sharing fixtures keeps isolation checks and bundle-size measurements aligned. +## Selecting fixtures + +`fixturesRoot` is usually shared with monosize, whose fixtures exist to measure bytes and legitimately bundle styling +engines and icons. Verifying everything found there would force every such fixture onto the allowlist, turning tracked +debt into permanent noise and adding churn to an unrelated workflow every time a bundle-size fixture is added. + +So the check is opt-in: only the fixtures named in `fixtures` are bundled. The rest are reported as `SKIPPED` rather +than dropped quietly, so what is left unguarded stays visible in CI output and in `summary.json`. + +The trade-off is that a newly added fixture is not covered until someone lists it. A key naming a fixture that does not +exist fails the run as an orphan, and a configuration where no listed fixture exists fails rather than reporting a clean +run over nothing. + ## Allowed violations `allowedViolations` is tracked debt, not an exemption. It is shrink-only: - A newly retained forbidden package fails the check. - A package that no longer survives bundling also fails the check until its entry is removed. -- An entry for a missing fixture fails the check. +- A fixture entry naming a fixture that does not exist fails the check. This prevents fixed leaks from being silently reintroduced. Deleting an entry is the goal; adding one is a regression. diff --git a/tools/verify-bundle-isolation/schema.json b/tools/verify-bundle-isolation/schema.json index 150dc7052fd16e..f9f10b0c83b596 100644 --- a/tools/verify-bundle-isolation/schema.json +++ b/tools/verify-bundle-isolation/schema.json @@ -32,20 +32,38 @@ "minLength": 1 } }, - "allowedViolations": { - "description": "Bundle-size fixture paths mapped to forbidden packages tolerated as tracked debt. Fixture paths use forward slashes and are relative to fixturesRoot.", + "fixtures": { + "description": "Fixtures to verify, keyed by path relative to fixturesRoot using forward slashes. A fixture present in fixturesRoot but absent here is skipped, which is how a directory shared with monosize is narrowed to the entry points that carry an isolation guarantee.", "type": "object", + "minProperties": 1, "additionalProperties": { - "type": "array", - "uniqueItems": true, - "items": { - "description": "Exact resolved package name. Globs are rejected so a tolerated leak cannot silently cover a new one.", - "type": "string", - "minLength": 1, - "pattern": "^[^*]+$" + "type": "object", + "additionalProperties": false, + "properties": { + "forbiddenPackages": { + "description": "Replaces the top-level list for this fixture. Narrowing has to be possible, so this overrides rather than extends.", + "type": "array", + "minItems": 1, + "uniqueItems": true, + "items": { + "type": "string", + "minLength": 1 + } + }, + "allowedViolations": { + "description": "Forbidden packages tolerated as tracked debt for this fixture.", + "type": "array", + "uniqueItems": true, + "items": { + "description": "Exact resolved package name. Globs are rejected so a tolerated leak cannot silently cover a new one.", + "type": "string", + "minLength": 1, + "pattern": "^[^*]+$" + } + } } } } }, - "required": ["fixturesRoot", "externals", "forbiddenPackages", "allowedViolations"] + "required": ["fixturesRoot", "externals", "forbiddenPackages", "fixtures"] } diff --git a/tools/verify-bundle-isolation/src/cli.ts b/tools/verify-bundle-isolation/src/cli.ts index c9871edf355c02..f7b5ce2c90422c 100644 --- a/tools/verify-bundle-isolation/src/cli.ts +++ b/tools/verify-bundle-isolation/src/cli.ts @@ -14,7 +14,17 @@ import { parseArgs } from 'node:util'; import webpack, { type Configuration, type Stats, type WebpackPluginInstance } from 'webpack'; import { BundleIsolationPlugin, type BundleIsolationReport } from './bundle-isolation-plugin'; -import { findFixtures, findWorkspaceRoot, fixtureOutputPath, loadConfig, outputRoot, readJson } from './config'; +import { + findFixtures, + findWorkspaceRoot, + fixtureOutputPath, + forbiddenFor, + loadConfig, + outputRoot, + readJson, + relativeToWorkspace, + selectFixtures, +} from './config'; import { type FixtureResult, type Report, @@ -37,20 +47,31 @@ export async function cli(): Promise { const config = loadConfig(args.configPath, workspaceRoot); const packageJson = readJson(join(packageRoot, 'package.json')); const fixturesRoot = resolve(packageRoot, config.fixturesRoot); - const fixtures = findFixtures(fixturesRoot); + const discovered = findFixtures(fixturesRoot); - if (fixtures.length === 0) { + if (discovered.length === 0) { console.error(`No bundle-size fixtures found in ${packageJson.name} - nothing to verify.`); process.exit(1); } + const selection = selectFixtures(discovered, config); + + // An all-orphan configuration would otherwise report a clean run without bundling anything. + if (selection.verified.length === 0) { + console.error( + `None of the ${discovered.length} fixtures in ${packageJson.name} are listed under "fixtures" in ` + + `${relativeToWorkspace(args.configPath, workspaceRoot)} - nothing to verify.`, + ); + process.exit(1); + } + const options: RuntimeOptions = { ...args, config, fixturesRoot, packageRoot, workspaceRoot }; // Fixtures come and go; a stale output directory would otherwise be mistaken for a fresh report. rmSync(outputRoot(packageRoot), { recursive: true, force: true }); - const results = await Promise.all(fixtures.map(fixture => verifyFixture(fixture, options))); - const report = createReport({ packageName: packageJson.name, results, fixtures, options }); + const results = await Promise.all(selection.verified.map(fixture => verifyFixture(fixture, options))); + const report = createReport({ packageName: packageJson.name, results, selection, options }); const summaryPath = writeSummary(report); // One stream for the whole report - splitting it would let the shell interleave the verdict. @@ -155,7 +176,7 @@ function createWebpackConfig( optimization: { concatenateModules: false, minimize: false }, plugins: [ new BundleIsolationPlugin({ - forbiddenPackages: options.config.forbiddenPackages, + forbiddenPackages: forbiddenFor(fixture, options.config), workspaceRoot: options.workspaceRoot, packageRoot: options.packageRoot, onReport, diff --git a/tools/verify-bundle-isolation/src/config.spec.ts b/tools/verify-bundle-isolation/src/config.spec.ts index 5ca6a51f007bb9..faa575e2e397c4 100644 --- a/tools/verify-bundle-isolation/src/config.spec.ts +++ b/tools/verify-bundle-isolation/src/config.spec.ts @@ -2,7 +2,16 @@ import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; -import { findFixtures, fixtureOutputPath, loadConfig, outputRoot, relativeToWorkspace } from './config'; +import { + allowedFor, + findFixtures, + fixtureOutputPath, + forbiddenFor, + loadConfig, + outputRoot, + relativeToWorkspace, + selectFixtures, +} from './config'; describe('loadConfig', () => { let root: string; @@ -19,7 +28,7 @@ describe('loadConfig', () => { fixturesRoot: './bundle-size', externals: ['react'], forbiddenPackages: ['tabster'], - allowedViolations: {}, + fixtures: { 'A.fixture.js': {} }, }; const load = (config: object) => { @@ -40,20 +49,36 @@ describe('loadConfig', () => { expect(() => load({ ...valid, forbiddenPackages: [] })).toThrow(/must NOT have fewer than 1 items/); }); + it('rejects an empty fixture list, which would leave nothing to verify', () => { + expect(() => load({ ...valid, fixtures: {} })).toThrow(/must NOT have fewer than 1 properties/); + }); + it('rejects unknown fields, so a typo cannot be mistaken for configuration', () => { expect(() => load({ ...valid, knownViolations: {} })).toThrow(/must NOT have additional properties/); }); + it('rejects an unknown field inside a fixture entry', () => { + expect(() => load({ ...valid, fixtures: { 'A.fixture.js': { allowed: [] } } })).toThrow( + /must NOT have additional properties/, + ); + }); + it('rejects a glob in allowedViolations, which would silently absorb an unrelated leak', () => { - expect(() => load({ ...valid, allowedViolations: { 'A.fixture.js': ['@griffel/*'] } })).toThrow( + expect(() => load({ ...valid, fixtures: { 'A.fixture.js': { allowedViolations: ['@griffel/*'] } } })).toThrow( /must match pattern/, ); }); it('accepts an exact package name in allowedViolations', () => { - expect(load({ ...valid, allowedViolations: { 'A.fixture.js': ['@griffel/core'] } }).allowedViolations).toEqual({ - 'A.fixture.js': ['@griffel/core'], - }); + const config = load({ ...valid, fixtures: { 'A.fixture.js': { allowedViolations: ['@griffel/core'] } } }); + + expect(config.fixtures).toEqual({ 'A.fixture.js': { allowedViolations: ['@griffel/core'] } }); + }); + + it('accepts a per-fixture forbidden list, so a fixture can narrow the package-level intent', () => { + const config = load({ ...valid, fixtures: { 'A.fixture.js': { forbiddenPackages: ['@griffel/*'] } } }); + + expect(config.fixtures['A.fixture.js'].forbiddenPackages).toEqual(['@griffel/*']); }); it('reports the offending path relative to the workspace', () => { @@ -88,6 +113,54 @@ describe('findFixtures', () => { }); }); +describe('selectFixtures', () => { + const config = { + fixturesRoot: './bundle-size', + externals: [], + forbiddenPackages: ['tabster'], + fixtures: { 'A.fixture.js': {}, 'nested/C.fixture.js': {}, 'Gone.fixture.js': {} }, + }; + + it('verifies only what the configuration opted in to', () => { + const selection = selectFixtures(['A.fixture.js', 'B.fixture.js', 'nested/C.fixture.js'], config); + + expect(selection.verified).toEqual(['A.fixture.js', 'nested/C.fixture.js']); + }); + + it('reports a discovered but unlisted fixture as skipped rather than dropping it silently', () => { + const selection = selectFixtures(['A.fixture.js', 'B.fixture.js', 'nested/C.fixture.js'], config); + + expect(selection.skipped).toEqual(['B.fixture.js']); + }); + + it('reports a listed fixture that no longer exists as an orphan', () => { + const selection = selectFixtures(['A.fixture.js', 'B.fixture.js', 'nested/C.fixture.js'], config); + + expect(selection.orphans).toEqual(['Gone.fixture.js']); + }); +}); + +describe('forbiddenFor', () => { + const config = { + fixturesRoot: './bundle-size', + externals: [], + forbiddenPackages: ['tabster', '@griffel/*'], + fixtures: { 'A.fixture.js': {}, 'B.fixture.js': { forbiddenPackages: ['@griffel/*'] } }, + }; + + it('falls back to the package-level list', () => { + expect(forbiddenFor('A.fixture.js', config)).toEqual(['tabster', '@griffel/*']); + }); + + it('replaces rather than extends, so a fixture can narrow the list', () => { + expect(forbiddenFor('B.fixture.js', config)).toEqual(['@griffel/*']); + }); + + it('treats a fixture with no allowlist as carrying no debt', () => { + expect(allowedFor('A.fixture.js', config)).toEqual([]); + }); +}); + describe('paths', () => { it('derives the output directory from the package root', () => { expect(outputRoot('/ws/packages/thing')).toBe('/ws/packages/thing/dist/bundle-isolation'); diff --git a/tools/verify-bundle-isolation/src/config.ts b/tools/verify-bundle-isolation/src/config.ts index ee32b657ed5fb6..2fc8742d905d3f 100644 --- a/tools/verify-bundle-isolation/src/config.ts +++ b/tools/verify-bundle-isolation/src/config.ts @@ -7,11 +7,23 @@ import { dirname, isAbsolute, join, sep } from 'node:path'; import Ajv, { type ErrorObject } from 'ajv'; +export interface FixtureConfig { + forbiddenPackages?: string[]; + allowedViolations?: string[]; +} + export interface Config { fixturesRoot: string; externals: string[]; forbiddenPackages: string[]; - allowedViolations: Record; + fixtures: Record; +} + +/** Discovered fixtures split against the ones the configuration opted in to. */ +export interface FixtureSelection { + verified: string[]; + skipped: string[]; + orphans: string[]; } const schemaPath = join(__dirname, '..', 'schema.json'); @@ -53,6 +65,29 @@ export function findFixtures(fixturesRoot: string): string[] { ); } +/** + * Fixture directories are shared with monosize, so discovery alone would drag in fixtures that were + * never meant to carry an isolation guarantee. Only what the configuration names is verified. + */ +export function selectFixtures(discovered: string[], config: Config): FixtureSelection { + const listed = Object.keys(config.fixtures); + + return { + verified: discovered.filter(fixture => listed.includes(fixture)), + skipped: discovered.filter(fixture => !listed.includes(fixture)), + orphans: listed.filter(fixture => !discovered.includes(fixture)).sort(), + }; +} + +/** Overrides rather than extends, so a fixture can narrow the forbidden set and not only widen it. */ +export function forbiddenFor(fixture: string, config: Config): string[] { + return config.fixtures[fixture]?.forbiddenPackages ?? config.forbiddenPackages; +} + +export function allowedFor(fixture: string, config: Config): string[] { + return config.fixtures[fixture]?.allowedViolations ?? []; +} + export function findWorkspaceRoot(startDir: string): string { let dir = startDir; diff --git a/tools/verify-bundle-isolation/src/report.spec.ts b/tools/verify-bundle-isolation/src/report.spec.ts index a25fb1d89565fb..e990fbab998830 100644 --- a/tools/verify-bundle-isolation/src/report.spec.ts +++ b/tools/verify-bundle-isolation/src/report.spec.ts @@ -1,3 +1,4 @@ +import { type Config, type FixtureConfig, selectFixtures } from './config'; import { type FixtureResult, type RuntimeOptions, @@ -57,7 +58,7 @@ describe('createReport', () => { const report = createReport( input({ results: [fixtureResult({ found: ['allowed-pkg'] })], - allowedViolations: { 'A.fixture.js': ['allowed-pkg'] }, + fixtures: { 'A.fixture.js': { allowedViolations: ['allowed-pkg'] } }, }), ); @@ -68,7 +69,7 @@ describe('createReport', () => { const report = createReport( input({ results: [fixtureResult({ found: ['allowed-pkg'] })], - allowedViolations: { 'A.fixture.js': ['allowed-pkg'] }, + fixtures: { 'A.fixture.js': { allowedViolations: ['allowed-pkg'] } }, strict: true, }), ); @@ -76,18 +77,33 @@ describe('createReport', () => { expect(report).toMatchObject({ failed: true, status: 'failed' }); }); - it('fails on an allowlist entry for a fixture that does not exist', () => { - const report = createReport(input({ results: [fixtureResult()], allowedViolations: { 'Gone.fixture.js': ['x'] } })); + it('fails on a fixture entry for a fixture that does not exist', () => { + const report = createReport( + input({ + results: [fixtureResult()], + fixtures: { 'A.fixture.js': {}, 'Gone.fixture.js': { allowedViolations: ['x'] } }, + }), + ); expect(report.orphans).toEqual([{ fixture: 'Gone.fixture.js', packages: ['x'] }]); expect(report.failed).toBe(true); }); + it('reports a discovered fixture that was not opted in as skipped rather than verified', () => { + const report = createReport( + input({ results: [fixtureResult()], discovered: ['A.fixture.js', 'Monosize.fixture.js'] }), + ); + + expect(report.skipped).toEqual(['Monosize.fixture.js']); + expect(report.failed).toBe(false); + }); + it('totals findings across fixtures', () => { const report = createReport( input({ results: [fixtureResult({ found: ['a-pkg'] }), fixtureResult({ fixture: 'B.fixture.js', found: ['b-pkg'] })], - fixtures: ['A.fixture.js', 'B.fixture.js'], + discovered: ['A.fixture.js', 'B.fixture.js'], + fixtures: { 'A.fixture.js': {}, 'B.fixture.js': {} }, }), ); @@ -107,7 +123,7 @@ describe('formatReport', () => { createReport( input({ results: [fixtureResult({ found: ['forbidden-pkg'], leaks: { 'forbidden-pkg': leak() } })], - allowedViolations: { 'A.fixture.js': ['forbidden-pkg'] }, + fixtures: { 'A.fixture.js': { allowedViolations: ['forbidden-pkg'] } }, }), ), '/ws/summary.json', @@ -117,6 +133,34 @@ describe('formatReport', () => { expect(text).toContain('PASS WITH DEBT - 1 fixture, 0 regressions, 1 allowed violation'); }); + it('names the fixtures it skipped, so an opted-out fixture is never silently unguarded', () => { + const text = formatReport( + createReport(input({ results: [fixtureResult()], discovered: ['A.fixture.js', 'Monosize.fixture.js'] })), + '/ws/summary.json', + ); + + expect(text).toContain(' SKIPPED 1 fixture not listed under "fixtures" in packages/thing/config.json'); + expect(text).toContain(' Monosize.fixture.js'); + expect(text).toContain('PASS - 1 fixture, 1 skipped free of'); + }); + + it('shows the forbidden list only for a fixture that overrides it', () => { + const text = formatReport( + createReport( + input({ + results: [fixtureResult(), fixtureResult({ fixture: 'B.fixture.js' })], + discovered: ['A.fixture.js', 'B.fixture.js'], + fixtures: { 'A.fixture.js': {}, 'B.fixture.js': { forbiddenPackages: ['@scope/*'] } }, + }), + ), + '/ws/summary.json', + ); + + expect(text).toContain(' CLEAN B.fixture.js\n forbidden: @scope/*'); + expect(text).toContain(' CLEAN A.fixture.js\n'); + expect(text).not.toContain(' CLEAN A.fixture.js\n forbidden:'); + }); + it('lists allowlisted leaks with their size and entry points, ordered by cost', () => { const text = formatReport( createReport( @@ -130,7 +174,7 @@ describe('formatReport', () => { }, }), ], - allowedViolations: { 'A.fixture.js': ['forbidden-pkg', '@scope/styles'] }, + fixtures: { 'A.fixture.js': { allowedViolations: ['forbidden-pkg', '@scope/styles'] } }, }), ), '/ws/summary.json', @@ -148,7 +192,7 @@ describe('formatReport', () => { createReport( input({ results: [fixtureResult({ found: ['@scope/styles'], leaks: { '@scope/styles': leak() } })], - allowedViolations: { 'A.fixture.js': ['@scope/styles'] }, + fixtures: { 'A.fixture.js': { allowedViolations: ['@scope/styles'] } }, }), ), '/ws/summary.json', @@ -178,7 +222,9 @@ describe('formatReport', () => { it('tells the reader how to lock in a fix rather than reporting it as a plain failure', () => { const text = formatReport( - createReport(input({ results: [fixtureResult()], allowedViolations: { 'A.fixture.js': ['fixed-pkg'] } })), + createReport( + input({ results: [fixtureResult()], fixtures: { 'A.fixture.js': { allowedViolations: ['fixed-pkg'] } } }), + ), '/ws/summary.json', ); @@ -191,7 +237,7 @@ describe('formatReport', () => { createReport( input({ results: [fixtureResult({ found: ['forbidden-pkg'], leaks: { 'forbidden-pkg': leak() } })], - allowedViolations: { 'A.fixture.js': ['forbidden-pkg'] }, + fixtures: { 'A.fixture.js': { allowedViolations: ['forbidden-pkg'] } }, strict: true, }), ), @@ -221,7 +267,7 @@ describe('createSummary', () => { results: [ fixtureResult({ found: ['forbidden-pkg'], leaks: { 'forbidden-pkg': leak({ via: 'lib/entry.js' }) } }), ], - allowedViolations: { 'A.fixture.js': ['forbidden-pkg'] }, + fixtures: { 'A.fixture.js': { allowedViolations: ['forbidden-pkg'] } }, }), ), ); @@ -303,17 +349,24 @@ function leak({ modules = 2, via = null }: { modules?: number; via?: string | nu function input({ results, - fixtures = ['A.fixture.js'], - allowedViolations = {}, + discovered = ['A.fixture.js'], + fixtures = { 'A.fixture.js': {} }, strict = false, analyze = false, }: { results: FixtureResult[]; - fixtures?: string[]; - allowedViolations?: Record; + discovered?: string[]; + fixtures?: Record; strict?: boolean; analyze?: boolean; }) { + const config: Config = { + fixturesRoot: './bundle-size', + externals: [], + forbiddenPackages: ['forbidden-pkg', '@scope/*'], + fixtures, + }; + const options: RuntimeOptions = { configPath: '/ws/packages/thing/config.json', analyze, @@ -321,13 +374,8 @@ function input({ fixturesRoot: '/ws/packages/thing/bundle-size', packageRoot, workspaceRoot, - config: { - fixturesRoot: './bundle-size', - externals: [], - forbiddenPackages: ['forbidden-pkg', '@scope/*'], - allowedViolations, - }, + config, }; - return { packageName: '@fluentui/thing', results, fixtures, options }; + return { packageName: '@fluentui/thing', results, selection: selectFixtures(discovered, config), options }; } diff --git a/tools/verify-bundle-isolation/src/report.ts b/tools/verify-bundle-isolation/src/report.ts index e027ccd56eebd8..5ca70a89b926a6 100644 --- a/tools/verify-bundle-isolation/src/report.ts +++ b/tools/verify-bundle-isolation/src/report.ts @@ -5,7 +5,14 @@ import { join } from 'node:path'; import type { Leak } from './bundle-isolation-plugin'; -import { type Config, fixtureOutputPath, relativeToWorkspace } from './config'; +import { + type Config, + type FixtureSelection, + allowedFor, + fixtureOutputPath, + forbiddenFor, + relativeToWorkspace, +} from './config'; export interface RuntimeOptions { configPath: string; @@ -29,6 +36,7 @@ export type FixtureStatus = 'error' | 'regression' | 'stale' | 'allowed' | 'clea export interface Outcome extends FixtureResult { status: FixtureStatus; + forbidden: string[]; allowed: string[]; tolerated: string[]; regressions: string[]; @@ -51,6 +59,7 @@ export interface Report { packageName: string; options: RuntimeOptions; outcomes: Outcome[]; + skipped: string[]; orphans: Orphan[]; totals: Totals; failed: boolean; @@ -66,16 +75,21 @@ const MAX_IMPORTERS = 2; export function createReport({ packageName, results, - fixtures, + selection, options, }: { packageName: string; results: FixtureResult[]; - fixtures: string[]; + selection: FixtureSelection; options: RuntimeOptions; }): Report { - const outcomes = results.map(result => classify(result, options.config.allowedViolations[result.fixture] ?? [])); - const orphans = orphanedAllowlistEntries(fixtures, options.config.allowedViolations); + const outcomes = results.map(result => + classify(result, allowedFor(result.fixture, options.config), forbiddenFor(result.fixture, options.config)), + ); + const orphans = selection.orphans.map(fixture => ({ + fixture, + packages: allowedFor(fixture, options.config), + })); const totals: Totals = { errors: outcomes.filter(outcome => outcome.status === 'error').length, regressions: sumBy(outcomes, outcome => outcome.regressions.length), @@ -94,6 +108,7 @@ export function createReport({ packageName, options, outcomes, + skipped: selection.skipped, orphans, totals, failed, @@ -101,7 +116,7 @@ export function createReport({ }; } -export function classify(result: FixtureResult, allowed: string[]): Outcome { +export function classify(result: FixtureResult, allowed: string[], forbidden: string[] = []): Outcome { const regressions = result.found.filter(name => !allowed.includes(name)); const stale = allowed.filter(name => !result.found.includes(name)); const tolerated = allowed.filter(name => result.found.includes(name)); @@ -117,13 +132,7 @@ export function classify(result: FixtureResult, allowed: string[]): Outcome { status = 'allowed'; } - return { ...result, status, allowed, tolerated, regressions, stale }; -} - -export function orphanedAllowlistEntries(fixtures: string[], allowedViolations: Record): Orphan[] { - return Object.entries(allowedViolations) - .filter(([fixture]) => !fixtures.includes(fixture)) - .map(([fixture, packages]) => ({ fixture, packages })); + return { ...result, status, forbidden, allowed, tolerated, regressions, stale }; } export function formatReport(report: Report, summaryPath: string): string { @@ -138,10 +147,23 @@ export function formatReport(report: Report, summaryPath: string): string { lines.push(...formatFixture(outcome, options), ''); } + if (report.skipped.length > 0) { + lines.push( + `${badge('SKIPPED')}${count(report.skipped.length, 'fixture')} not listed under "fixtures" in ${configLabel( + options, + )}`, + ...report.skipped.map(fixture => ` ${fixture}`), + '', + ); + } + for (const orphan of report.orphans) { lines.push( - `${badge('ORPHAN')}${orphan.fixture} - allowlisted (${orphan.packages.join(', ')}) but not a bundle-size fixture`, - ` remove the entry from allowedViolations in ${configLabel(options)}`, + `${badge('ORPHAN')}${orphan.fixture} - listed under "fixtures" but not found in ${relativeToWorkspace( + options.fixturesRoot, + options.workspaceRoot, + )}`, + ` remove the entry from ${configLabel(options)}`, '', ); } @@ -152,12 +174,18 @@ export function formatReport(report: Report, summaryPath: string): string { } function formatFixture(outcome: Outcome, options: RuntimeOptions): string[] { + // Only worth printing when the fixture narrowed or widened the package-level intent. + const override = + outcome.forbidden.join() === options.config.forbiddenPackages.join() + ? [] + : [` forbidden: ${outcome.forbidden.join(', ')}`]; + if (outcome.status === 'error') { - return [`${badge('ERROR')}${outcome.fixture}`, ...formatError(outcome, options.workspaceRoot)]; + return [`${badge('ERROR')}${outcome.fixture}`, ...override, ...formatError(outcome, options.workspaceRoot)]; } if (outcome.status === 'clean') { - return [`${badge('CLEAN')}${outcome.fixture}`]; + return [`${badge('CLEAN')}${outcome.fixture}`, ...override]; } const lines: string[] = []; @@ -270,14 +298,15 @@ function originsOf(leak: Leak, workspaceRoot: string): string[] { function formatVerdict(report: Report): string[] { const { options, totals, orphans } = report; - const fixtures = count(report.outcomes.length, 'fixture'); + const verified = count(report.outcomes.length, 'fixture'); + const fixtures = report.skipped.length > 0 ? `${verified}, ${report.skipped.length} skipped` : verified; if (report.failed) { const parts = [ totals.errors > 0 && `${count(totals.errors, 'fixture')} failed to bundle`, totals.regressions > 0 && count(totals.regressions, 'regression'), totals.stale > 0 && count(totals.stale, 'stale allowlist entry', 'stale allowlist entries'), - orphans.length > 0 && count(orphans.length, 'orphaned allowlist entry', 'orphaned allowlist entries'), + orphans.length > 0 && count(orphans.length, 'orphaned fixture entry', 'orphaned fixture entries'), options.strict && totals.tolerated > 0 && `${count(totals.tolerated, 'allowed violation')} rejected by --strict`, ].filter(Boolean); @@ -324,10 +353,12 @@ export function createSummary(report: Report) { strict: options.strict, status: report.status, forbiddenPackages: options.config.forbiddenPackages, - orphanedAllowlistEntries: report.orphans, + skippedFixtures: report.skipped, + orphanedFixtureEntries: report.orphans, fixtures: report.outcomes.map(outcome => ({ fixture: outcome.fixture, status: outcome.status, + forbiddenPackages: outcome.forbidden, analyzerReport: options.analyze ? toWorkspacePath(join(fixtureOutputPath(outcome.fixture, options.packageRoot), 'report.json')) : null,