Repository navigation
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
the
needs:design-review
|
AI review status for this pull request.
|
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNo new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: The accessibility audit did not run on this pull request. It runs after merges to Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
![astracat-bot[bot]](https://avatars.githubusercontent.com/in/4796231?s=60&v=4){kind=small}
There was a problem hiding this comment.
The Core output matches astryx.css byte for byte and the Turbopack fixture builds correctly with Core installed in node_modules, but approval waits on a maintainer decision. spec:AST-080 leaves bundler wiring and module selection out of scope, so a maintainer needs to provide or identify the governing spec for Turbopack support in withAstryx() (including the condition it adds to the app's own turbopack.rules), the new @astryxdesign/build/next/postcss export with @astryx;, include and cwd, and how a source build picks its modules. Independently of that decision, please fix:
- With Core workspace-linked (a monorepo package,
pnpm linkorfile:), the fixture still builds, but every Astryx class takes the app's StyleX prefix and has no rule, because the loader glob and the keep-off condition innext.jsmatch only unresolvednode_modulespaths. - With
@astryxdesign/labinstalled,@astryx;throws on Lab'sCodeEditorbecausestylexOptions()addspropertyValidationMode: 'throw', which the prebuilt build does not use; without it, Lab compiles tolab.cssbyte for byte. - The
includedefault is documented as every source file but skips.mdx, and FR7's development-build and theme-override states have no test.
[Automated review]
| const escaped = packages.map(pkg => | ||
| sourcePath(pkg).replace(/[.*+?^${}()|[\]\\]/g, '\\$&'), | ||
| ); | ||
| const astryxSource = { | ||
| path: new RegExp(`(^|/)node_modules/(${escaped.join('|')})/`), | ||
| }; | ||
| const rules = {}; | ||
| for (const [glob, collection] of Object.entries(turbopack.rules || {})) { | ||
| rules[glob] = keepOffAstryx(collection, astryxSource); | ||
| } | ||
| for (const pkg of packages) { | ||
| rules[`**/node_modules/${sourcePath(pkg)}/**/*.{ts,tsx}`] = { | ||
| loaders: [ | ||
| {loader: TURBOPACK_LOADER, options: stylexOptions(pkg, packages)}, | ||
| ], | ||
| as: '*', | ||
| }; |
There was a problem hiding this comment.
The loader glob and the keep-off condition are built from the unresolved node_modules path that findPackageDir() returns, but Turbopack matches rules against the resolved file. With Core workspace-linked (a monorepo package, pnpm link, file:), the resolved path is packages/core/src/…, so Astryx is compiled with the app's StyleX options (its *.tsx rule or Babel config) instead: the fixture builds with no warning, yet all 58 Astryx classes come out p… with no rule. Deriving both patterns from fs.realpathSync(pkg.srcDir) as well, or refusing when a package resolves outside node_modules, plus a linked-layout case in next.turbopack.test.mjs, would close it.
There was a problem hiding this comment.
Fixed in 86ab34b. Each package's rule now also matches its resolved path, taken from the closest directory that holds both the app and the package's real source (packages/core/src for a workspace link), next to the unresolved node_modules path. The condition that keeps caller rules off Astryx uses the same patterns, and the Astryx rules share one *.{ts,tsx} key, so a caller rule under that key is merged, not replaced.
The e2e fixture now links Core from outside node_modules (its app loader refuses any linked or installed Astryx file) and checks that every Astryx class is x… with a rule; on the previous head the app's *.tsx rule reached that Core (312 build errors). A unit test covers a monorepo layout.
| return { | ||
| dev: false, | ||
| runtimeInjection: false, | ||
| propertyValidationMode: 'throw', |
There was a problem hiding this comment.
scripts/build-css.mjs does not set propertyValidationMode, so this option makes the source build stricter than the prebuilt one. Because sourceStylesheet() compiles every module of every installed source package, an app with @astryxdesign/lab installed fails on CodeEditor.tsx ("border is not supported") even if it imports only Core's Button. Without this line the Lab every-module compile equals lab.css byte for byte; a Lab parity test beside Core's would keep it that way.
There was a problem hiding this comment.
Fixed in 86ab34b: stylexOptions() no longer sets propertyValidationMode, matching scripts/build-css.mjs. A parity test now compiles every Lab module and compares the result with lab.css as build-css.mjs builds it, byte for byte (Core likewise against astryx.css); on the previous head it fails on CodeEditor's border. The e2e fixture installs Lab and renders a Lab Stat.
| `include` takes globs of the files whose Astryx imports count; the default is | ||
| every source file outside `node_modules` and `.next`. |
There was a problem hiding this comment.
The default glob is **/*.{js,jsx,mjs,cjs,ts,tsx}, so .mdx pages are not covered: a component used only in MDX gets no rules, and adding **/*.mdx to include throws a parse error. Saying "JS and TS files" here and in next-postcss.js (or reading MDX imports) would match the behavior.
There was a problem hiding this comment.
Fixed in 86ab34b: the default include now covers .mdx (the plugin reads the import and export statements in an MDX file's ESM blocks), documented as JS, TS and MDX files outside node_modules and .next, with a test.
For FR7's states, next.turbopack.test.mjs now also runs next dev on the fixture, and checks in Chromium that a theme override in @layer astryx-theme beats the source-built rules whichever stylesheet loads first. The test-node job installs Chromium before the Node suite so that check runs in CI.
There was a problem hiding this comment.
include and its .mdx default moved to the follow-up with module selection (vjeux:build-next-source-subset), along with the tests for them. This PR no longer reads the app's files. The development-build and theme-override tests stay here.
6d97fae to
86ab34b
Compare
There was a problem hiding this comment.
The workspace-link and Lab fixes work, MDX imports are now read, and the new Turbopack tests pass, but approval still waits on a maintainer decision: spec:AST-080 leaves bundler wiring and module selection out of scope, so a maintainer needs to provide or identify the governing spec for Turbopack support in withAstryx() (including the condition it adds to the app's own turbopack.rules), the @astryxdesign/build/next/postcss export with @astryx;, include and cwd, and how a source build picks its modules. Independently of that decision, please fix:
- With the README's default
includeand an Astryx package linked from a pnpm workspace,next buildruns out of memory: thedir-dependencyglob reported atsource-stylesheet.js:711lets Turbopack walknode_modulesand its symlinks, whileinclude: ['src/**/*.{ts,tsx}']builds the same app in 17 s. - An
.mdxfile whose code example has a blank line beforeexport, or a.tsfile with an angle-bracket assertion, stopsnext buildwith a parse error (source-stylesheet.js:365-378). - Nothing tests the default
includeor FR7's "Core source-built beside a prebuilt Lab stylesheet" state.
[Automated review]
| */ | ||
| async function sourceStylesheet({cwd, include = [CODE_GLOB]}) { | ||
| const packages = sourcePackages(cwd); | ||
| const watch = include.map(glob => ({dir: cwd, glob})); |
There was a problem hiding this comment.
With the default include, this reports {dir: cwd, glob: '**/*.{js,jsx,mjs,cjs,ts,tsx,mdx}'} as a dir-dependency, and the IGNORE list doesn't travel with it, so Turbopack tracks the glob through the app's node_modules and follows its symlinks. With Core linked from this repository's workspace (packages/core/node_modules/@astryxdesign/cli links back to Core through packages/cli), the README's setup made next build pass 14 GiB in 15 s (76.9 GB before the kernel killed it when unguarded); with include: ['src/**/*.{ts,tsx}'] the same app builds in 17 s. Reporting only globs that can't reach node_modules or .next (for example one dir-dependency per top-level entry of the app other than those), plus a fixture build with the default include and a linked package that has its own node_modules, would close it.
There was a problem hiding this comment.
Fixed in 6f07dd5. The plugin now reports only globs that can't reach node_modules, a dot directory such as .next, or a symlink. It walks each include pattern from its static directory: a tree with none of those gets one deep glob, and any other directory gets a glob for its own files plus the globs of each subtree.
A unit test checks the exact globs for an app with node_modules, .next, a nested node_modules and a symlinked directory; it fails on the previous head. The Turbopack fixture now uses the default include, with Core linked from outside the app with its own node_modules, and builds and serves under next dev.
On the review's last point: that fixture build is the default-include test, and a new compile test covers FR7's "Core source-built beside a prebuilt Lab stylesheet" state: a Core build of a one-component app defines every Core variable and keyframes that lab.css uses.
One limit found along the way, now in the README: in next dev, Turbopack re-runs PostCSS only for changes inside the app (a dependency on a linked package's source, by real path or through node_modules, never fires), so a linked package's rules refresh when next dev restarts. The previous head registered no such dependency either.
There was a problem hiding this comment.
The app's watch globs moved to the follow-up with include (vjeux:build-next-source-subset), along with this test. This PR's output depends on no app file. It registers one dependency per source-built package, on that package's source, so an upgrade rebuilds the stylesheet under Turbopack's persistent cache.
| if (file.endsWith('.mdx')) { | ||
| // MDX keeps imports and exports in top-level ESM blocks; a blank line ends one. | ||
| code = code | ||
| .split(/\n[ \t]*\n/) | ||
| .filter(block => /^(import|export)\b/.test(block.trimStart())) | ||
| .join('\n\n'); | ||
| } | ||
| const ast = babel().parseSync(code, { | ||
| filename: file, | ||
| babelrc: false, | ||
| configFile: false, | ||
| sourceType: 'unambiguous', | ||
| parserOpts: { | ||
| plugins: /\.[cm]?tsx?$/.test(file) ? ['typescript', 'jsx'] : ['jsx'], |
There was a problem hiding this comment.
Two inputs a Next.js app can hold make this parse throw, and the error stops next build: an .mdx page whose fenced example has a blank line before export default function … (the kept block ends with the closing fence, which Babel reads as a template literal: "Unterminated template"), and a .ts file with an angle-bracket assertion such as <HTMLElement>document.body, which the jsx plugin rejects ("Unterminated JSX contents"). Skipping fenced code when collecting MDX's ESM blocks and enabling jsx only for .tsx, .jsx, .js and MDX would fix both; a test for each would keep it.
There was a problem hiding this comment.
Fixed in 6f07dd5. MDX's ESM blocks are now collected outside fenced code, and jsx is enabled only where JSX can appear (.tsx, JavaScript and MDX), so <HTMLElement>document.body parses in a .ts file.
A file the parser still can't read no longer stops the build either: the plugin warns and keeps every module the file names, whole, so no import it really has is dropped. Tests cover both inputs and the fallback; on the previous head the three inputs throw Unterminated template, Unterminated JSX contents and Unexpected token.
There was a problem hiding this comment.
Parsing the app's files moved to the follow-up with module selection (vjeux:build-next-source-subset). The MDX, .ts and unparseable-file handling and their tests moved with it. This PR never parses app files.
86ab34b to
6f07dd5
Compare
There was a problem hiding this comment.
The earlier memory, MDX and TypeScript-parse problems are fixed and the Turbopack tests pass, but approval waits on a maintainer decision: spec:AST-080 leaves how @astryxdesign/build plugs into each bundler and how a source build picks its modules out of scope, so a maintainer needs to provide or identify the governing spec for Turbopack support in withAstryx() (including the condition it adds to the app's own turbopack.rules), the @astryxdesign/build/next/postcss export with @astryx;, include and cwd, and import-based module selection. Independently of that decision, please fix two things:
- In a
turbopack.ruleslist that mixes bare loaders with rule objects,keepOffAstryx()leaves the bare loaders running on Astryx source (packages/build/src/next.js:244), although the README says the app's own rules are kept off it. - The new "Install Playwright for the Node suite's browser checks" step in
test-nodeneeds the sametooling_only != 'true'guard as the theme-family step below it, so the node-tooling lane keeps skipping browser work.
[Automated review]
| if (!collection.some(isRule)) { | ||
| return [exclude({loaders: collection})]; | ||
| } | ||
| return collection.map(item => (isRule(item) ? exclude(item) : item)); |
There was a problem hiding this comment.
In a list that mixes bare loaders with rule objects, the bare items come back unchanged, and Turbopack runs them on every matching file: with '*.tsx': [{loader: probe}, {loaders: [probe], condition: {not: 'foreign'}}] wrapped in withAstryx(), the bare loader ran on 528 Astryx source files and the rule object on none. Wrapping each bare item (or each run of consecutive bare items) in exclude({loaders: [item]}) would keep the README's promise that the app's own rules stay off Astryx source, and the mixed shape could join the "in every rule shape" test in next.test.mjs.
There was a problem hiding this comment.
Fixed in bc71a0e. Next.js turns each bare loader in a list into a rule of its own, with no condition, and applies matching rules in list order. keepOffAstryx() now does the same split, wrapping each bare loader in place as {loaders: [item]} with the keep-off condition. The order on app files is therefore Next's own.
This also fixed a second bug: for a list of only bare loaders, the previous head merged them into one rule, which ran them in reverse (b, a where Next runs a, b).
Tests:
- The "every rule shape" test now covers a mixed list and a two-loader bare list, and checks the whole returned config against Next's own config schema.
- The fixture's
*.tsxrule is now a mixed list: two bare loaders around the app's StyleX rule object. Each loader logs the files it runs on and refuses Astryx source. The test asserts they runa, stylex, bonpage.tsxand only on the app's files. - On 6f07dd5 that build fails (the bare loader reaches Lab's
Stat.tsx), and the unit test fails too.
Against the same app without withAstryx(), both lists run in the same order: a, stylex, b and a, b.
|
|
||
| # The Build package's Next.js suite checks a theme override in Chromium. | ||
| - name: Install Playwright for the Node suite's browser checks | ||
| if: needs.check-scope.outputs.docsite_only != 'true' && needs.check-scope.outputs.spec_only != 'true' |
There was a problem hiding this comment.
This step lacks the needs.check-scope.outputs.tooling_only != 'true' guard that the theme-family Chromium step below carries, so a pull request that only touches admitted Node tooling now downloads Chromium and runs the theme-override browser check, which spec:AST-030 FR8 says that lane skips. Adding the guard lets it.skipIf(!hasChromium) skip the check there.
There was a problem hiding this comment.
Fixed in bc71a0e: the step now carries the same tooling_only != 'true' guard as the theme-family steps below it. In the tooling-only lane it.skipIf(!hasChromium) skips the browser check, and the CI workflow tests pass.
There was a problem hiding this comment.
Rebased in 0ef2281 past #7705, which splits test-node into four Vitest --shard legs. The step keeps both guards and now runs in every leg. --shard decides which leg runs the Build suite, and its Chromium check skips wherever Chromium is missing, so installing Chromium in one leg alone could skip the check without notice.
There was a problem hiding this comment.
In a92978d the install step also sets ASTRYX_REQUIRE_CHROMIUM for the rest of the leg, and the Turbopack test skips its Chromium check only when that variable is unset. In CI, a missing browser now fails the check instead of skipping it. Local runs, and deploy.yml's push gate, which installs no Chromium, skip as before.
The install costs 12–13 s per leg, and the legs run in parallel. Running the check once in leg 1 instead would mean changing ci-test-routing's contract, which allows one unsharded step in a sharded lane: the FR23 benchmark.
6f07dd5 to
bc71a0e
Compare
There was a problem hiding this comment.
The earlier mixed-rule-list and CI-guard problems are fixed, and the Turbopack build, next dev and the Core and Lab parity tests pass, but approval waits on a maintainer decision. spec:AST-080 leaves how @astryxdesign/build plugs into each bundler and how a source build picks its modules out of scope, so a maintainer needs to provide or identify the governing spec for Turbopack support in withAstryx() (including the condition it adds to the app's own turbopack.rules), the @astryxdesign/build/next/postcss export with @astryx;, include and cwd, and import-based module selection. Beyond that decision, one optional README suggestion is inline.
[Automated review]
| `include` takes globs of the files whose Astryx imports count; the default is | ||
| every JS, TS and MDX file outside `node_modules` and `.next`. | ||
|
|
||
| 3. Your global stylesheet, with `@astryx;` in place of `astryx.css`: |
There was a problem hiding this comment.
Optional: when the app imports Lab, @astryx; also prints Lab's rules, while Rich Text and Charts are not source-built and keep their prebuilt stylesheets. Saying so here (in place of astryx.css and lab.css; keep richtext.css and charts.css) would keep a Lab user from loading Lab's rules twice, matching how spec:AST-080 FR5 describes a source build replacing each source-built package's stylesheet.
There was a problem hiding this comment.
Done in 9d6fcad. I checked the list first. Only packages in withAstryx()'s list whose root export carries a source entry are built from source. With Core, Lab, Rich Text, Charts and the neutral theme all installed, that is Core and Lab: those two get the source aliases and loader rules. The neutral theme is in the list but ships no source entry. Rich Text and Charts resolve to their published files.
Step 3 now says @astryx; replaces astryx.css and lab.css, and that any richtext.css or charts.css import stays.
bc71a0e to
9d6fcad
Compare
There was a problem hiding this comment.
The Turbopack build, next dev and the Core and Lab parity tests pass, and a Turbopack build of the fixture renders the same pixels with @astryx; as with the prebuilt stylesheets, but approval waits on a maintainer decision. spec:AST-080 leaves how @astryxdesign/build plugs into each bundler and how a source build picks its modules out of scope, so a maintainer needs to provide or identify the governing spec for Turbopack support in withAstryx() (including the condition it adds to the app's own turbopack.rules), the @astryxdesign/build/next/postcss export with @astryx;, include and cwd, and import-based module selection. One optional test suggestion is inline.
[Automated review]
| ), | ||
| ); | ||
| } | ||
| expect(colors[0]).not.toBe('rgb(0, 120, 255)'); |
There was a problem hiding this comment.
Optional: this assertion passes even though the Button's own colors never resolve here. The fixture's globals.css loads no reset.css, so nothing declares color-scheme, and Turbopack lowers each light-dark() value to --lightningcss-light/--lightningcss-dark fallbacks that nothing sets: in the built page --color-error computes to #e3193b#f5394f, and the destructive Button has a transparent background with black text. Importing @astryxdesign/core/reset.css in the fixture, as README step 3 does, and expecting rgb(227, 25, 59) here would let the check prove the source-built color as well as the layer order.
There was a problem hiding this comment.
Done in b4d580f. The fixture's globals.css now imports @astryxdesign/core/reset.css, as README step 3 does. The check now expects the destructive Button's source-built background, rgb(227, 25, 59), without the override, and rgb(0, 120, 255) with it, in both orders.
It fails in both directions:
- With the override removed, all three cases compute
rgb(227, 25, 59). - With
reset.cssremoved, the Button computesrgba(0, 0, 0, 0).
Step 3's theme import is still played by the stylesheet the test injects. The Node suite builds Core but no theme package, so there's no built theme.css to load.
9d6fcad to
b4d580f
Compare
There was a problem hiding this comment.
This now builds and renders correctly under Turbopack: the parity, next build, next dev and Chromium cascade tests pass (the cascade test now loads reset.css and checks the source-built color), and the fixture renders identically with @astryx; and with the prebuilt stylesheets. Approval still needs a maintainer decision, because spec:AST-080 leaves bundler integration and source-build module selection out of scope: a maintainer must provide or identify the spec covering Turbopack support in withAstryx() (including the condition it adds to the app's own turbopack.rules), the @astryxdesign/build/next/postcss export with @astryx;, include and cwd, and import-based module selection. There is nothing further for you to change before that decision.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: spec:AST-080 governs the CSS a source build emits but leaves how @astryxdesign/build plugs into each bundler out of scope, so the governing spec needs to be provided or identified for withAstryx()'s Turbopack support (its aliases, its loader rules and the condition it adds to the app's own turbopack.rules), the new @astryxdesign/build/next/postcss export with @astryx; and cwd, and the refusal added to @astryxdesign/build/postcss, including whether this direction stands beside #6841. Any engineering owner, the author included, can record that decision by commenting /approve-spec followed by the full head SHA. One fix is needed whatever is decided:
packageModules()still never adds Core'stheme/dataTokens.stylex.ts, so a Core-only@astryx;declares none of the 56--color-data-*defaults thatastryx.csscarries (a Turbopack build with only Core rendersdataVarscolours transparent); seed it asscripts/build-css.mjs'sstylexEntriesdoes, and build the parity test's reference from that script's targets so the test catches it.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: spec:AST-080 governs the CSS a source build emits, and this head meets it (the @astryx; output for Core and Lab matches astryx.css and lab.css byte for byte, Core's block carries the data-colour defaults, and the Turbopack fixture builds, serves under next dev and lets a theme override win in Chromium), but its Non-goals leave how @astryxdesign/build plugs into each bundler out of scope. The governing spec therefore needs to be provided or identified for withAstryx()'s Turbopack support (its aliases, its loader rules and the condition it adds to the app's own turbopack.rules), the new @astryxdesign/build/next/postcss export with @astryx; and cwd, and the refusal added to @astryxdesign/build/postcss, including whether this direction stands beside #6841. Any engineering owner, the author included, can record that decision by commenting /approve-spec followed by the full head SHA. Nothing else in the change needs fixing before that decision.
[Automated review]
d8a5837 to
c0f9ca2
Compare
There was a problem hiding this comment.
The Turbopack source build works as described: Core and Lab compile to the same rules as astryx.css and lab.css, the fixture builds and serves under next dev, and a theme override wins in either load order. Approval waits on one decision, because spec:AST-080 governs the CSS a source build emits but leaves how @astryxdesign/build plugs into each bundler out of scope: the governing spec needs to be provided or identified for withAstryx()'s Turbopack configuration (its aliases, its loader rules and the condition it adds to the app's own turbopack.rules), the new @astryxdesign/build/next/postcss export with @astryx; and cwd, and the refusal added to @astryxdesign/build/postcss, including whether this direction stands beside #6841. Any engineering owner, the author included, can record that decision by commenting /approve-spec followed by the full head SHA. Nothing else in the change needs fixing first.
[Automated review]
c0f9ca2 to
522ab78
Compare
There was a problem hiding this comment.
The Turbopack source build works as described: Core and Lab compile to the same rules as astryx.css and lab.css, the fixture builds and serves under next dev, and a theme override wins in either load order. Approval waits on one decision, because spec:AST-080 governs the CSS a source build emits but leaves how @astryxdesign/build plugs into each bundler out of scope: the governing spec needs to be provided or identified for withAstryx()'s Turbopack configuration (its aliases, its loader rules and the condition it adds to the app's own turbopack.rules), the new @astryxdesign/build/next/postcss export with @astryx; and cwd, and the refusal added to @astryxdesign/build/postcss, including whether this direction stands beside #6841. Any engineering owner, the author included, can record that decision by commenting /approve-spec followed by the full head SHA. Nothing else in the change needs fixing first; the red test-node (1/4) leg timed out in an unrelated CLI test that passes on its own, so a re-run should show whether that was a one-off.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: spec:AST-080 governs the CSS a source build emits, and this head meets it, but its Non-goals leave how @astryxdesign/build plugs into each bundler out of scope. The governing spec needs to be provided or identified for withAstryx()'s Turbopack configuration (its aliases, loader rules and the condition it adds to the app's own turbopack.rules), the new @astryxdesign/build/next/postcss export with @astryx; and cwd, and the refusal added to @astryxdesign/build/postcss, including whether this direction stands beside #6841. Any engineering owner, the author included, can record that decision by commenting /approve-spec followed by the full head SHA. Independently, the real Turbopack build test now runs only with ASTRYX_TURBOPACK_E2E=1, which no CI step sets, while spec:AST-030 FR11 puts the Build package's CSS-layer cascade checks in its test owner: adding it to pnpm test:build, beside the ASTRYX_STYLEX_BROWSER=1 check, keeps it running on that job's own runner without slowing test-node.
[Automated review]
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: spec:AST-080 governs the CSS a source build emits, and this head meets it, but its Non-goals leave how @astryxdesign/build plugs into each bundler out of scope. The governing spec needs to be provided or identified for withAstryx()'s Turbopack configuration (its aliases, loader rules and the condition it adds to the app's own turbopack.rules), the new @astryxdesign/build/next/postcss export with @astryx; and cwd, and the refusal added to @astryxdesign/build/postcss, including whether this direction stands beside #6841. Any engineering owner, the author included, can record that decision by commenting /approve-spec followed by the full head SHA. Nothing else needs fixing first: the real Turbopack build check now runs in pnpm test:build on the Build package's own CI job, and it passes at this head.
[Automated review]
withAstryx() configures Turbopack instead of refusing to: each source entry point gets a resolveAlias, each source-built package a loader rule, matched on its resolved path, that compiles StyleX as the prebuilt stylesheets do, and the caller's own turbopack.rules are kept off Astryx source. The new @astryxdesign/build/next/postcss plugin replaces an `@astryx;` rule with each source-built package's prebuilt stylesheet, compiled from that source (spec:AST-080 FR7), and depends on the packages' source so an upgrade rebuilds it. Under Turbopack, the webpack setup's PostCSS plugin refuses to run beside withAstryx(). The webpack setup is unchanged.
…ource A source build compiled StyleX with its commonJS resolution, which resolves a request such as `@astryxdesign/core/Indicator/indicator.markers.stylex` through the package's `exports` before any alias. With Core's `dist` installed, as every published install has it, another package's use of a Core marker, constant or hashed variable was named after `dist/...stylex.js` while Core names it after its source, so the selector never matched. The prebuilt stylesheets resolve such requests to source (scripts/stylex-module-resolution.cjs); the source build now does the same, in the PostCSS plugin and the Turbopack loader alike, and names files by StyleX's own rule, so Core's class names are unchanged.
scripts/build-css.mjs compiles Core's public theme/dataTokens.stylex.ts into astryx.css although no Core module imports it (its stylexEntries), so an app reading the data colours as plain var() references gets their 56 defaults. The source build never seeded it: a Core-only `@astryx;` declared none of them, and with Lab installed they landed in Lab's block. The source build now seeds the same entries. As build-css.mjs does, it also keeps the constants a package reads from its provider's modules, so they resolve where they are read. The parity test builds its reference from build-css.mjs's own targets, which now export, so it covers both.
The end-to-end test's two Next.js production builds, dev server and Chromium took every core for minutes beside the other tests in a test-node leg, and pushed a CLI test past its 30 s timeout. As a test that reads files it would also run on every scoped pull request. It now runs with ASTRYX_TURBOPACK_E2E=1, the way the RTL browser check runs with ASTRYX_RTL_BROWSER=1; the default suite reports it skipped, and with the flag set but no Chromium it fails. The workflow no longer installs Chromium for it. Every fix keeps an always-on test that fails without it: a new unit test compiles Core's Button with the Turbopack loader and checks each class name against astryx.css.
spec:AST-030 FR11 puts the Build package's CSS-layer cascade behaviour in its test owner, so the end-to-end check runs in `pnpm test:build`, beside the StyleX browser check, on test-build's own runner, which installs Chromium. The routing contract pins test:build's commands, so it lists the new one.
readManifest() returned null for any read or parse error, so a package with a malformed package.json silently dropped out of the source build. The StyleX resolver returned null for any error, not only for an import it cannot find. Now a broken manifest throws, and only MODULE_NOT_FOUND or ERR_PACKAGE_PATH_NOT_EXPORTED reads as unresolved, as StyleX's own resolvers treat a failed resolution. getCanonicalFilePath reads the nearest manifest as scripts/stylex-module-resolution.cjs does, and the changeset names the behaviour.
f231c43 to
7f090fc
Compare
There was a problem hiding this comment.
Approval waits on an engineering owner's decision: spec:AST-080 governs the CSS a source build emits, and this head meets it, but its Non-goals leave how @astryxdesign/build plugs into each bundler out of scope. The governing spec needs to be provided or identified for withAstryx()'s Turbopack configuration (its aliases, loader rules, the condition it adds to the app's own turbopack.rules, and failing on an unreadable installed package.json), the new @astryxdesign/build/next/postcss export with @astryx; and cwd, and the refusal added to @astryxdesign/build/postcss, including whether this direction stands beside #6841. Any engineering owner, the author included, can record that decision by commenting /approve-spec followed by the full head SHA. Nothing else needs fixing first: the Core and Lab parity tests and the real Turbopack build check pass at this head.
[Automated review]
cixzhang
added
the
ci:run
User need
Why. Next.js 16 defaults to Turbopack, where Astryx's source build can't run;
@astryxdesign/buildhad 44,577 downloads last week.What led to it. One of our Next.js 16 products, moving onto Astryx, needs AST-080's source build, which loads only the CSS it uses. In September #5921 found it silently unstyled under Turbopack; #6109 made that an error. Astryx's Next.js example uses
--webpack.Problem and solution fit
withAstryx()configured onlynextConfig.webpack, which Turbopack never calls. It now configures Turbopack; a new PostCSS plugin replaces@astryx;with the published stylesheets, built from source.Current specification
spec:AST-080FR7 sets the output but leaves bundler wiring out, so one owner decision is open: whether@astryxdesign/buildsupports Turbopack this way (proposed, pending owner review).Public delta and caller burden
withAstryx()returns mergedtranspilePackages,turbopack.resolveAliasandturbopack.rules; webpack is unchanged.@astryxdesign/build/next/postcss(optioncwd).@astryxdesign/build/postcss(webpack setup) now throws beside it under Turbopack, instead of building unstyled.package.jsonfails the build instead of being left out.Design and alternatives
next devfirst page takes 53 s, not 9 s; every module prints until the subset PR (vjeux:build-next-source-subset).Evidence
A webpack app builds the same 55 files as on main; Testing covers Turbopack.
Scope
Testing
Always on (
pnpm exec vitest run --project node packages/build), each failing without its fix:.stylexresolutionReal build, in test-build (spec:AST-030 FR11):
pnpm test:buildrunsASTRYX_TURBOPACK_E2E=1 vitest run packages/build/src/next.turbopack.test.mjsbeside the StyleX browser check, on the Build package's own job. It builds and serves the fixture, checking colours in Chromium: green here, red on main ("withAstryx() requires the webpack bundler"). The Node suite skips it (7 tests); there, its builds pushed a neighbouring test past its timeout. Without Chromium, it fails. Cost: a few minutes per code PR on test-build's standard ARM runner. The test-build job is being reshaped in #8916 and #8918; this check follows whatever shape lands there.Each e2e check fails without its fix: linked Core unstyled, installed Lab failing to build; the app's loaders reaching Lab's source, a pair in reverse (
b, a); a build from the parent stopping atCan't resolve '@astryxdesign/lab'; a Core-only swatch atrgba(0, 0, 0, 0); the Button red without the override, transparent withoutreset.css.Other setups: Rich Text and Charts keep their stylesheets; the README's webpack setup under Turbopack, once 57 of 57 unstyled, fails clearly; an upgraded Lab rebuilds despite the persistent cache; the published canary builds fully styled.
Local gates at 7f090fc:
pnpm test:buildwith the Turbopack check (7 of 7, red on main's build package as above),packages/buildwith the CI routing tests (381 passed, 7 skipped: the opt-in check),pnpm check:repoand Prettier all pass. Inpnpm test:nodethe only failures are load-sensitive CLI tests inpackages/cli, which also fail on main or pass when run alone; none are inpackages/buildor touch the build package.