Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
PR Analysis Report🔎 Preview ·
|
There was a problem hiding this comment.
Requesting changes. The Next.js webpack guidance is accurate and well placed; CI now confirms the CLI's experimental-isolation goldens need regenerating.
- The new
nextjs-webpacksection is missing frompackages/cli/test/__golden__/no-icon-set/docs-tree.json, and itsexperimental.optimizePackageImportswording adds a fourth result toastryx search experimental, so three snapshot tests inpackages/cli/test/experimental-isolation.test.mjsfail intest-node (2/4): runvitest run packages/cli/test/experimental-isolation.test.mjs --update, commit thedocs-tree.jsonandsearch-experimentalgolden changes, and note in the description that the newexperimentalmentions are Next.js's option, not an Astryx experimental surface.
test-node (4/4) also failed, but only on a temp-folder cleanup timeout in packed-gather-cache.test.mjs after all its tests passed, which this change does not touch; a re-run should clear it.
Claims and files checked
- Claims
- Changeset: the guide gains a Next.js with webpack section: verified (
tokens-and-setup.doc.mjs:141-167). - Changeset: the section shows the
optimizePackageImportssetting innext.config.mjs: verified (tokens-and-setup.doc.mjs:149-161). - Changeset: it recommends a component entry such as
@astryxdesign/core/Buttonin Server Components: verified (tokens-and-setup.doc.mjs:164). - Changeset: without either, a webpack build ships every client component to a page whose Server Component imports the package root: consistent with Core's root barrel, which has no
'use client'while component modules do (packages/core/src/index.ts,Button/Button.tsx:3); the sizes are the author's measurement. - Changeset: the theme preview starter has the same setting: verified (
packages/cli/assets/preview-app/next.config.mjs:4-10). - Changeset and description: no API changes: verified.
- Description: first-load sizes of 853.8, 273.6 and 257.2 kB: not reproduced; consistent with the mechanism.
- Description: Turbopack, Vite, plain webpack and
'use client'files are not affected: consistent with the mechanism; not measured. - Description:
optimizePackageImportsis a Next.js experimental option that rewrites root imports to the modules they name: verified against Next's reference page for the option. - Description: Core has an entry per component,
astryx component <Name>prints it, and shipped docs already use it: verified (packages/core/package.json./Button;getting-started.doc.mjs:162). - Description: three example apps and the starter set the option, and a test keeps the starter equal to
apps/example-nextjs: verified (theme-preview.test.mjs:713-724; the four files are identical). - Description: the example README gains step 4: verified (
apps/example-nextjs/README.md:59-72). - Description: a
[docs]patch changeset for@astryxdesign/cli, which ships the docs and the starter: verified (packages/cli/package.jsonfilesincludesassets). - Description:
apps/example-nextjs-sourceand the Core README are unchanged: verified. - Description: the pull request names every necessary generated artifact: false, the experimental-isolation goldens are missing.
- Description: public text has no internal context: verified.
- Description: the listed checks pass: not rerun;
experimental-isolation.test.mjs, which fails in CI, is not among them.
- Changeset: the guide gains a Next.js with webpack section: verified (
- Files
.changeset/next-webpack-optimize-package-imports.md: clear.apps/example-nextjs-stylex/next.config.mjs: clear.apps/example-nextjs-tailwind/next.config.mjs: clear.apps/example-nextjs/README.md: clear.apps/example-nextjs/next.config.mjs: clear.packages/cli/assets/docs/tree/tokens-and-setup.doc.mjs: finding, the goldens above.packages/cli/assets/preview-app/next.config.mjs: clear; identical to the example, as the copy test requires.
|
/approve-spec 1f142ba |
There was a problem hiding this comment.
Approved pending CI. The Next.js webpack guidance fits the shipped docs' purpose, and the experimental-isolation goldens now record the new nextjs-webpack section, which fixes the one finding from the last review. CI has not run in full on this head: the required test, build and docsite-test checks report "Heavy CI deferred" by design, and the CLI test shards that check these goldens run in the pre-merge re-run of all jobs.
Claims and files checked
- Claims
- Changeset: the guide gains a Next.js with webpack section: verified (
tokens-and-setup.doc.mjs:141-167). - Changeset: the section shows the
optimizePackageImportssetting innext.config.mjs: verified (tokens-and-setup.doc.mjs:149-161). - Changeset: it recommends a component entry such as
@astryxdesign/core/Buttonin Server Components: verified (tokens-and-setup.doc.mjs:164). - Changeset: without either, a webpack build ships every client component to a page whose Server Component imports the package root: consistent with Core's directive-less root barrel (
packages/core/src/index.ts) and component modules that start with'use client'(Button/Button.tsx:3); the sizes are the author's measurement. - Changeset: the theme preview starter has the same setting: verified (
packages/cli/assets/preview-app/next.config.mjs:4-10). - Changeset and description: no API changes: verified.
- Description: first-load sizes of 853.8, 273.6, 257.2, 287.3 and 268.6 kB: not reproduced; consistent with the mechanism.
- Description: Turbopack, Vite, plain webpack and
'use client'files do not have the every-component cost: consistent with the mechanism; not measured. - Description: Next.js declares the option in its config types and rewrites root imports to the modules they name: not checked here; the guidance rests on the author's measurement.
- Description: Core has an entry per component,
astryx component <Name>prints it, and shipped docs already use it: verified (packages/core/package.json./Button;component.mjs:283;getting-started.doc.mjs:162). - Description: three example apps and the starter set the option, and a test keeps the starter equal to
apps/example-nextjs: verified (the four files are identical;theme-preview.test.mjs:713-724). - Description: the example README gains step 4: verified (
apps/example-nextjs/README.md:59-72). - Description: a
[docs]patch changeset for@astryxdesign/cli, which ships the docs and the starter: verified; the changeset check passes in CI. - Description: the goldens list the new section, and their new
experimentalmentions are Next.js's config key: verified (docs-tree.json:1239-1250, bothsearch-experimentalgoldens). - Description:
apps/example-nextjs-source(built throughwithAstryx) and the Core README are unchanged and show no such config: verified. - Description: the pull request is documentation plus named generated artifacts, with no runtime, API, visual or policy change and no internal context: verified.
- Description: the listed local checks pass: not rerun; Lint and the CLI smoke test pass in CI, and the five updated snapshots match the five snapshot assertions over the three goldens.
- Changeset: the guide gains a Next.js with webpack section: verified (
- Files
.changeset/next-webpack-optimize-package-imports.md: clear.apps/example-nextjs-stylex/next.config.mjs: clear.apps/example-nextjs-tailwind/next.config.mjs: clear.apps/example-nextjs/README.md: clear.apps/example-nextjs/next.config.mjs: clear.packages/cli/assets/docs/tree/tokens-and-setup.doc.mjs: clear.packages/cli/assets/preview-app/next.config.mjs: clear; identical to the example, as the copy test requires.packages/cli/test/__golden__/no-icon-set/docs-tree.json: clear; matches the test's sentence rule for the new section.packages/cli/test/__golden__/no-icon-set/search-experimental.json: clear.packages/cli/test/__golden__/no-icon-set/search-experimental.txt: clear.
Reader impact
Who: a Next.js app router project that builds with webpack (
next build --webpack) and imports from the Core package root, such asimport {Button} from '@astryxdesign/core', in a Server Component.Before: that page loads every Astryx client component. In a one-button page built with Next.js 16.4.6 and webpack, first-load JS was 853.8 kB gzip. The same page with
import {Button} from '@astryxdesign/core/Button'was 273.6 kB.After: with the setting this PR documents, the root-import page is 257.2 kB gzip, the same as the subpath page. Turbopack (the Next.js 16 default), Vite, plain webpack, and imports in
'use client'files are not affected.New public API: none.
Source of truth
'use client', it follows every import, and each'use client'module it reaches becomes a browser entry. Core's root index re-exports every component and has no directive, and most component modules are'use client'. So one root import from a Server Component adds the whole library to the page.experimental.optimizePackageImportsis typed innext/dist/server/config-shared.d.ts(ExperimentalConfig). It rewrites root imports of the listed package to the modules they name, so the walk reaches only those.package.jsonexportshas an entry per component (./Buttonand so on).astryx component <Name>prints that entry, and the shipped docs already use it in samples.Correction
astryx docs styling/tokens-and-setuphas a new section,nextjs-webpack. It shows thenext.config.mjssetting, says why in one sentence, and recommends a component entry in Server Components.apps/example-nextjs,apps/example-nextjs-tailwindandapps/example-nextjs-stylexset it innext.config.mjs. The theme preview starter app (packages/cli/assets/preview-app) is a copy ofapps/example-nextjs, and a test keeps them the same, so it gets the same file.apps/example-nextjs/README.mdhas a short step 4 for the config.[docs]patch changeset for@astryxdesign/cli, which ships the docs and the starter app.packages/cli/test/__golden__/no-icon-set/docs-tree.json,search-experimental.json,search-experimental.txt) list the new section. Their new "experimental" mentions are Next.js's ownexperimentalconfig key, not an Astryx experimental surface, so they are not an isolation leak.Not changed:
apps/example-nextjs-sourcebuilds Core from source throughwithAstryx, andpackages/core/README.mdshows no Next.js config.Evidence
Fixture: a copy of
apps/example-nextjswith Core and the neutral theme packed from this branch, Next.js 16.4.6,next build --webpack. First-load JS is the gzip sum of every script each prerendered page loads.import {Button} from '@astryxdesign/core'import {Button} from '@astryxdesign/core/Button''use client')Rendered section (
astryx docs styling/tokens-and-setup nextjs-webpack):Scope
Components: none
The example and starter
next.config.mjsfiles change build configuration only; no Core or CLI code changes.Testing
pnpm check:knowledge: aligned.vitest run packages/cli/test/experimental-isolation.test.mjs --update: 46 passed, 5 snapshots updated (the goldens above).vitestonpackages/cli/api/docs/docs.test.mjsand the starter-app copy test inpackages/cli/api/theme-edit/theme-preview.test.mjs: pass.pnpm specs:for --diff origin/mainandpnpm review:pre: 0 findings; no concern-routed claim applies to a docs section.