Skip to content

spec(AST-080): every PR reports component size; touch-only JS and CSS have a budget - #8271

Merged
cixzhang merged 4 commits into
mainfrom
spec/gr-size-budget
Oct 24, 2026
Merged

cixzhang merged 4 commits into
mainfrom
spec/gr-size-budget

Conversation

@cixzhang

@cixzhang cixzhang commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

Exact decision needed

How much a component change may add to what every product downloads, and how a pull request shows that cost.

Existing authority searched

Decision

  • spec:AST-080 FR8 / DEC-4: every pull request that changes styled modules reports, base against head, each changed package stylesheet's gzip size and each changed component's gzip CSS delta. A feature that runs only on a coarse pointer and adds more than 1 KB gzip of CSS to one component is flagged and justified; the CSS stays in the package stylesheet.
  • architecture:component-style-authoring INV6: every pull request that changes a component reports the gzip JavaScript its import pulls in, transitive modules included, and its CSS delta, base against head. Touch-only work over 2 KB gzip JavaScript or 1 KB gzip CSS per component is flagged; the JavaScript is loaded lazily, only when a coarse pointer needs it, or the cost is justified. A shared JavaScript tuning table ships to a component only the rows that component uses; StyleX variable groups and message catalogs keep their own owners.

Rejected alternatives

  • A separate stylesheet for touch-only CSS: breaks the one-import-per-package setup of spec:AST-080 DEC-3.

Claim scope and non-goals

Size reporting and the touch-only budget only. Budgets for other kinds of work, render or listener counts, and the report's CI implementation are outside this PR.

Evidence and implementation relationship

The pull-request size report measures no stylesheet, reports a component's re-export file instead of what its import pulls in, and never fills its before and delta columns. A report fix and the budget flags follow as separate implementation PRs. No implementation is included.

Writing rubric results

  • R1 narration: none.
  • R2 research in the record: none.
  • R3 evidence naming a change: none; verification names report states.
  • R4 rejected alternatives over one line or outside a decision: one, inside DEC-4, one line.
  • R5 pull request or issue references remaining: none.
  • R6 ruled open questions still listing options: none.

Scope

  • This PR contains specification records only.
  • It records observable behavior and preserves equivalent internal implementations.
  • Any named internal mechanism is intentionally public, names its dependent caller/system, and explains why an equivalent implementation would not satisfy the contract.
  • Verification states the evidence layer and failure signal without prescribing CI job/workflow topology.
  • It records one intentional durable decision, not a rescue for a separable tagalong.
  • The canonical knowledge owner, affected current claims, and overlapping open work are named; any ownership collision is resolved or called out for human decision.
  • Adjacent API, visual, accessibility, composition, and implementation facts remain outside scope unless this exact decision depends on them.
  • Public text and artifacts contain no internal Meta context.

Validation

  • pnpm check:knowledge (CI lint)
  • Prettier
  • git diff --check

… have a budget

AST-080 FR8 and DEC-4: each pull request that changes styled modules reports
each changed package stylesheet's gzip size and each changed component's CSS
delta, base against head; touch-only CSS over 1 KB gzip per component is
flagged and justified. architecture:component-style-authoring INV6 adds the
per-import gzip JavaScript report, a 2 KB gzip touch-only JavaScript budget
met by lazy loading or a justification, and value tables that ship only the
rows a component uses.
@vercel

vercel Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

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

Project Deployment Actions Updated
astryx Ready Ready Preview Oct 24, 2026 7:07pm UTC

Request Review

@astracat-bot

astracat-bot Bot commented Oct 24, 2026 •

Copy link
Copy Markdown

AI review status for this pull request.

Review status Updated
🟢 Reviewed (for maintainers only) Oct 24, 2026, 7:33 PM UTC

@github-actions github-actions Bot added the needs:spec-owner-review Current knowledge records await owner approval label Oct 24, 2026
@meta-cla meta-cla Bot added the CLA Signed This label is managed by the Meta Open Source bot. label Oct 24, 2026
@github-actions

github-actions Bot commented Oct 24, 2026 •

Copy link
Copy Markdown
Contributor

PR Analysis Report

Previews ready: Storybook and Sandbox were built with this PR's docsite. CI evidence will be added when available.

View Storybook for this PR · View Sandbox for this PR

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This spec change adds requirements that no current record states (spec:AST-080 FR8 and DEC-4, architecture:component-style-authoring INV6), so approval waits on an engineering owner's decision: any engineering owner, including the author if they are one, records it by commenting /approve-spec <full head SHA> on this pull request. Two fixes are needed either way:

  • INV6's "flagged CSS is justified, because it stays in the package stylesheet" can be read as exempting that CSS from the justification FR8 requires; say that its pull request justifies the cost.
  • INV6's deciding-spec entry credits DEC-4 with the JavaScript budget, but DEC-4 covers only CSS; extend DEC-4 (or add a decision) for the 2 KB JavaScript flag, lazy loading and the value-table rule, and add the JavaScript report gap to Current-state impact.

Defining how a component's CSS delta and "KB" are measured would also let two reports flag the same change alike.

[Automated review]

coarse pointer and adds more than 2 KB gzip of JavaScript or 1 KB gzip of CSS
to one component is flagged. Its pull request either loads that JavaScript
lazily, only when a coarse pointer needs it, so the component's import does
not carry it, or justifies the cost; flagged CSS is justified, because it

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

"flagged CSS is justified, because it stays in the package stylesheet" can be read as saying flagged CSS is already justified, while spec:AST-080 FR8 requires its pull request to justify the cost. Suggest: "flagged CSS cannot load lazily, because it stays in the package stylesheet (spec:AST-080 FR1), so its pull request justifies the cost."

Comment on lines +216 to +217
- `spec:AST-080/DEC-4` — every pull request reports component size, and
touch-only JavaScript and CSS over budget is flagged (INV6).

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

DEC-4 decides only the stylesheet report and the 1 KB coarse-pointer CSS flag; it says nothing about the JavaScript figure, the 2 KB flag, lazy loading or the value-table rule. Extend DEC-4 (or add a decision) to state and justify those, or narrow this entry to the CSS half.

@cixzhang

Copy link
Copy Markdown
Contributor Author

/approve-spec aa384ca

@github-actions github-actions Bot removed the needs:spec-owner-review Current knowledge records await owner approval label Oct 24, 2026

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

An engineering owner has approved FR8, DEC-4 and INV6 at this head, but two sentences still need a wording fix because they contradict current records this pull request leaves unchanged:

  • INV6's shared-value-table rule (docs/architecture/component-style-authoring.md lines 111–114) does not say whether StyleX variable groups count, and its motion-table example fits the duration and easing groups that spec:AST-066 FR3 and DEC-2 require to ship whole; scope it to JavaScript tuning tables and say that variable groups follow AST-066 FR3.
  • FR8 and DEC-4 (docs/specs/AST-080-package-stylesheets/spec.md lines 140–142 and 264) say every product loads a package's whole stylesheet, while the same record's Intent, FR7 and DEC-1 let a source build compile only the modules a product ships; limit the premise to products that load the prebuilt stylesheet.

A new commit resets the approval, so an engineering owner, the author included, re-approves the fixed head by commenting /approve-spec <full head SHA>.

[Automated review]

Comment on lines +111 to +114
import and every module that import reaches. A shared value table,
such as a motion or gesture tuning table, ships to a component only the rows
that component uses: importing a component does not pull in rows only other
components read.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This rule doesn't say whether a StyleX variable group counts as a shared value table, and its motion-table example also fits durationDefaults and easeDefaults in packages/core/src/theme/tokens.stylex.ts, which components import as one module. Read that way it conflicts with spec:AST-066 FR3 and DEC-2, which require a variable group to ship whole and reject splitting it to simulate per-row pruning. Suggest scoping it, for example: "A shared JavaScript table that components read at run time, such as a gesture or spring tuning table, ships to a component only the rows that component uses …; StyleX variable groups ship whole under spec:AST-066 FR3."

Comment on lines +140 to +142
- **FR8 — Stylesheet growth is reported and budgeted per component.** Every
product loads a package's whole stylesheet (FR1), so CSS a component adds
reaches every product, including products that never render it. Every pull

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The Intent (lines 28–29), FR7 (133–134) and DEC-1 (223–225) let a product compile only the modules it ships through @astryxdesign/build, so not every product loads the whole stylesheet. Suggest: "Every product that loads a package's prebuilt stylesheet (FR1, FR5) loads all of it, so CSS a component adds reaches each of them, including products that never render it."

**Reference:** `spec:AST-080/DEC-4`
**Decider:** `cixzhang`, `2026-10-24`

Because one whole stylesheet ships to every product (DEC-3), CSS for a feature

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Same premise as FR8: a source build may compile only the modules a product ships (DEC-1), so limit this to products that load the prebuilt stylesheet.

… tables are JavaScript, variable groups follow AST-066
@cixzhang

Copy link
Copy Markdown
Contributor Author

/approve-spec 5c386b1

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

An engineering owner approved FR8, DEC-4 and INV6 at this head and both earlier wording fixes are in, but one sentence still contradicts a current record this pull request leaves unchanged, and the description lacks its overlap search. Two changes are needed:

  • INV6's shared-value-table rule (docs/architecture/component-style-authoring.md lines 111–115) also covers the generated English message catalog, which architecture:internationalization INV15 has the built-in fallback import whole; scope it to tuning tables and say message catalogs follow INV15, or amend INV15.
  • Under "Existing authority searched", record the search terms and the overlapping open pull requests (#8277, #8052, #8053, #7358, #8286), as architecture:knowledge-contracts INV13 asks.

A new commit resets the approval, so an engineering owner, the author included, re-approves the fixed head by commenting /approve-spec <full head SHA>.

[Automated review]

Comment on lines +111 to +115
import and every module that import reaches. A shared JavaScript value
table, such as a gesture or spring tuning table, ships to a component only
the rows that component uses: importing a component does not pull in rows
only other components read. StyleX variable groups are not value tables;
`spec:AST-066` FR3 decides which groups a stylesheet carries.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

This also covers the generated English runtime catalog: it is a shared JavaScript key-to-message table, and every component that calls useTranslator imports all of it through packages/core/src/i18n/resolve.ts, as architecture:internationalization INV15 requires ("The built-in English fallback imports the generated English module"). Suggest scoping it, for example: "A shared JavaScript table of tuning values, such as a gesture or spring tuning table, ships to a component only the rows that component uses …; StyleX variable groups and message catalogs are not value tables: spec:AST-066 FR3 and architecture:internationalization INV15 decide how they ship."

@cixzhang

Copy link
Copy Markdown
Contributor Author

/approve-spec d2a481e

@astracat-bot astracat-bot Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Recommended for approval: an engineering owner approved FR8, DEC-4 and INV6 at this head, and they contradict no current record now that the value-table rule covers tuning tables only and leaves StyleX variable groups to AST-066 FR3 and message catalogs to internationalization INV15. One note for the size-report work these records govern: INV6 does not say whether the measured JavaScript bundle is minified or keeps peer dependencies such as React external, and FR8 does not say how a shared atomic StyleX rule counts, so settling both in the implementation keeps two reports flagging the same change alike.

[Automated review]

@cixzhang
cixzhang merged commit f574bd0 into main Oct 24, 2026
65 checks passed
cixzhang added a commit that referenced this pull request Oct 24, 2026
…ta from the merge base (#8277)

## User impact

Reviewers and authors read the PR comment's size report to see what a change costs the people who download Astryx. Today it shows no real numbers: "Bundle Size (ESM)" is always N/A, the ESM file is reported under a "CJS" label, and Before and Delta are always N/A. So size growth, including touch-only code that ships to every desktop product, lands unseen.

## Problem and solution fit

- The analyzer looked for `dist/<Component>/index.mjs`, but Babel writes ES modules with a `.js` extension in `"type": "module"` packages. Entries are now resolved by the package's module type: `index.js` is ESM there; `index.mjs`/`index.cjs` keep their meaning; a non-module package (Vega) keeps `index.mjs` as ESM and `index.js` as CommonJS.
- The comment read `baseComponentStats`, which nothing wrote, and CI built only the PR head. A new `size-base` job checks out the merge base, builds Core (plus any other component package the PR changes), and runs `analyze-pr.js --base-root .size-base --base-only`. It runs beside `build-storybook`, so no browser lane waits on the second build.
- The PR comment downloads that `pr-size-base` artifact, uses it only when it names the exact head being reported, and renders Before and Delta for modified components and package gzip size.

## Expected behavior and authority

The report's own columns (Before, After, Delta, ESM) promise these figures. The pending size-budget record in #8271 requires a base-against-head report on every PR; the per-import transitive size, the CSS delta, and the budget flags follow in a separate PR once it merges.

## Smallest restoration

Only the entry resolution, the base measurement, and its rendering. The per-component figure is still the component's entry file, now labeled "Entry File"; measuring what an import pulls in is the follow-up.

## Evidence

- Reproduction before the change: every recent PR comment shows "Bundle Size (ESM) N/A" and Before/Delta "N/A".
- Result after the change: the new fixture test measures a `"type": "module"` entry as ESM (CJS null) and reads base sizes from a merge-base checkout; the comment test renders `| Entry File (ESM) | 100B | 160B | +60B |` and a package gzip delta.
- Representative unchanged path: without base stats, the comment keeps its previous package table and N/A cells; the shallow-clone recovery test still passes.
- Regression test or other mutation-sensitive proof: `analyze-pr.test.mjs` (base and head sizes), `generate-pr-comment.test.mjs` (base sizes), `pr-preview.test.mjs` (base stats used only for the exact head).

## Scope

- [x] One defect is restored.
- [x] Separable contradictory or unsettled tagalongs were removed or split.
- [x] No new public API, visual representation, default, or interaction model is hidden under the bug-fix label.
- [x] Public text and artifacts contain no internal Meta context.

## Testing

- `pnpm vitest run .github/scripts/analyze-pr.test.mjs .github/scripts/generate-pr-comment.test.mjs .github/scripts/lib/pr-preview.test.mjs .github/scripts/ci-*.test.mjs .github/scripts/release-check-workflow.test.mjs`
- `actionlint` on the changed workflows (no new findings)
- The new job's own run on this PR is the end-to-end check.
@github-actions
github-actions Bot deleted the spec/gr-size-budget branch October 25, 2026 06:20

This branch was successfully deployed

1 active deployment
Preview — d2a481ed Deployed Oct 24, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant