Repository navigation
spec(AST-080): every PR reports component size; touch-only JS and CSS have a budget - #8271
Conversation
… 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.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report
|
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
"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."
| - `spec:AST-080/DEC-4` — every pull request reports component size, and | ||
| touch-only JavaScript and CSS over budget is flagged (INV6). |
There was a problem hiding this comment.
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.
…efine gzip KB and the CSS delta
|
/approve-spec aa384ca |
There was a problem hiding this comment.
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.mdlines 111–114) does not say whether StyleX variable groups count, and its motion-table example fits the duration and easing groups thatspec:AST-066FR3 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.mdlines 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]
| 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. |
There was a problem hiding this comment.
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."
| - **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 |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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
|
/approve-spec 5c386b1 |
There was a problem hiding this comment.
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.mdlines 111–115) also covers the generated English message catalog, whicharchitecture:internationalizationINV15 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-contractsINV13 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]
| 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. |
There was a problem hiding this comment.
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."
|
/approve-spec d2a481e |
There was a problem hiding this comment.
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]
…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.
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
spec:AST-080FR1, FR2, DEC-3: each package ships one whole stylesheet, so CSS any component adds reaches every product.architecture:component-style-authoringINV1–INV5: no size outcome.architecture:react-component-runtimeINV5 (acquire optional resources only while enabled) and INV8 (reuse shared owners): no byte budget.spec:AST-036FR22–FR23: the only numeric budgets, for the Markdown parser only.architecture:internationalizationINV15), docs(spec): CodeBlock native language set by use, plus importable language modules under one provider #8286 (a component-local tokenizer byte budget, unaffected).Decision
spec:AST-080FR8 / 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-authoringINV6: 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
spec:AST-080DEC-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
Scope
Validation
pnpm check:knowledge(CI lint)git diff --check