Repository navigation
fix(build): scan what each Astryx package publishes, so a linked workspace builds - #7775
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
AI review status for this pull request.
|
meta-cla
Bot
added
the
CLA Signed
PR Analysis Report📚 Storybook Preview🧪 Sandbox PreviewNo new or modified components detected. Bundle Size SummaryNo component packages changed. Accessibility AuditStatus: No accessibility violations detected. Visual Regression40 of 486 shot(s) changed. A change here is a question, not a failure: check whether the after is the
and 20 more. 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.
This looks ready to merge: scanning each package's published files keeps a workspace-linked source build out of its link cycles, and registry installs still compile the same files into identical CSS. Two optional follow-ups:
- A
filesglob such assrc/*only reaches files directly insrc, while npm also packs nested files such assrc/Box/Box.tsx; no Astryx package uses a glob entry today, so a fix and a test for the glob and!branches can come later. - The comment above
libraryGlobsnames the cycle as core -> a11y-spec -> core, but@astryxdesign/a11y-spechas no Astryx dependency; in this workspace the cycle runs through core's dev dependency@astryxdesign/cli.
[Automated review]
| } else if (isGlob(pattern) || LIBRARY_SOURCE.test(pattern)) { | ||
| // A file or glob entry, as written; only its .ts and .tsx matches count. | ||
| include.push(`${base}/${pattern}`); |
There was a problem hiding this comment.
A glob entry is globbed as written, so src/* reaches only files directly in src, while npm pack of a package with files: ["src/*"] also packs src/Box/Box.tsx. No Astryx package uses a glob entry today, so this is latent; globbing the contents of matched directories (or treating a trailing /* as /**) plus a test for the glob and ! branches would close it.
There was a problem hiding this comment.
Done in e82bd86. A glob entry now brings its matches and everything below a directory it matches, so src/* includes src/Box/Box.tsx as npm pack does. A ! entry drops its matches with their contents. A new case in postcss.workspace-links.test.mjs covers both and fails on the previous head. The registry file lists are unchanged (1,317 / 1,317 / 1,562).
| * `node_modules`, whose links can lead back to the package (core -> a11y-spec | ||
| * -> core); neither is scanned. |
There was a problem hiding this comment.
@astryxdesign/a11y-spec declares no Astryx dependency, so it does not link back to core. In this workspace the cycle runs through core's dev dependency @astryxdesign/cli, whose node_modules/@astryxdesign/core links to packages/core.
There was a problem hiding this comment.
Right, thanks: the cycle runs core → @astryxdesign/cli → core. Fixed in e82bd86 in the comment, the commit message and the description, and the test workspace now models that chain.
e0c2a20 to
e82bd86
Compare
There was a problem hiding this comment.
This looks ready to merge: reading each package's published files keeps a workspace-linked source build out of its link cycles, the new test fails on main and passes here, and registry installs still compile the same files into byte-identical CSS. One optional follow-up:
filesreading still differs fromnpm packin shapes no Astryx package uses yet: a!entry listed before the directory it carves from, a slash-less!entry such as!*.stories.tsx, and a glob such assrc/B*that matches a directory (details onpackages/build/src/index.jslines 77-85).
[Automated review]
| if (negated) { | ||
| // An excluded directory takes everything below it along. | ||
| ignore.push(`${base}/${pattern}`, `${base}/${pattern}/**`); | ||
| } else if (isDirectory) { | ||
| include.push(`${base}/${pattern}/**/*.{ts,tsx}`); | ||
| } else if (isGlob(pattern)) { | ||
| // The glob's own matches, and everything below a directory it matches: | ||
| // `src/*` packs `src/Box/Box.tsx` too. | ||
| include.push(`${base}/${pattern}`, `${base}/${pattern}/**/*.{ts,tsx}`); |
There was a problem hiding this comment.
These branches read files differently from npm pack (and pnpm pack, which agrees) in three shapes no Astryx package uses yet:
files |
npm and pnpm pack | This plugin compiles |
|---|---|---|
["!src/internal", "src"] |
src/a.ts, src/internal/secret.ts |
src/a.ts |
["src", "!*.stories.tsx"] |
src/a.ts |
also src/top.stories.tsx, src/Box/Box.stories.tsx |
["src/B*"] |
src/Bar.ts |
also src/Box/Box.tsx |
npm applies a ! entry only to the entries before it, matches a slash-less one at any depth, and reads only a trailing /* as the whole directory. Mirroring those rules with a test case each, or narrowing the comment at line 44 and the test name to the shapes handled, would keep the "the way npm packs them" claim true.
There was a problem hiding this comment.
Changed in 6e3a0b6. Unsupported files shapes now fail loudly instead of approximating npm pack. A plain entry works as before: a directory brings the files below it and a file brings itself. An entry with glob characters or a leading ! fails the build with @astryxdesign/build reads plain files entries only; <package> lists "<entry>". No Astryx package lists one today (checked in the workspace, in the current canary and in 0.7.1). The refusal is tested, and registry installs still scan identical file lists.
…space builds
The PostCSS plugin scanned node_modules/@astryxdesign/**/*.{ts,tsx}, so a
package linked from a workspace contributed everything in its directory: its
own node_modules, whose links can cycle back to it (here, core's dev
dependency @astryxdesign/cli links core again) until the build runs out of
memory, and unpublished files such as @astryxdesign/build's intentionally
invalid test fixtures.
Scan each installed package's published files instead: each `files` entry
is a directory, which brings the files below it, or a file, and a package
without `files` is scanned whole minus nested node_modules. An entry with
glob characters or a leading `!` fails the build naming the package and the
entry, rather than approximating npm pack. For a registry install the
scanned files are the same as before.
e82bd86 to
6e3a0b6
Compare
There was a problem hiding this comment.
This looks ready to merge: reading each installed package's plain files entries keeps a workspace-linked source build out of its link cycles, the new test fails on main and passes here, and registry installs still scan the same files into byte-identical CSS. Refusing glob and ! entries is safe today, since every stable @astryxdesign/* release lists only plain entries. As an optional follow-up, a repository check that published manifests keep plain files entries would surface a future glob in this repository's CI rather than in consumers' source builds.
[Automated review]
) <!-- template: maintenance.md --> ## Maintainer impact `main` is red: `test-node` fails in `packages/build/src/postcss.test.mjs` ("compiles Astryx source without the app's Babel config"). Every pull request based on current `main`, and any release that gates on a green `main`, is blocked. ## Intended invariant The test keeps proving what #7774 added: the PostCSS plugin's library pass compiles Astryx source without the app's Babel config, while the app's own files still get it. It should run against an Astryx package installed the way a real one is. ## Change #7775 and #7774 each passed on their own but conflict once both are on `main`: - #7775 makes the plugin scan what each installed `@astryxdesign/*` package publishes, read from that package's `package.json`; a directory without one is not treated as an Astryx package. - #7774 added this test with a fixture that installs `node_modules/@astryxdesign/core/src/Box/Box.tsx` but no `package.json`. So, on `main`, the fixture's Astryx module is compiled as app code (`@layer product`) and the assertion on `@layer astryx-base` fails. The fix gives the fixture's package a minimal manifest (`{"name": "@astryxdesign/core"}`, no `files`, so the whole package is scanned). Test-only; no plugin change. ## Evidence - Failure before the change: on current `main`, `pnpm exec vitest run --project node packages/build/src/postcss.test.mjs` fails with `expected '@layer product {…' to match /@layer astryx-base \{[^]*color:\s*red/`, the same failure as `main`'s `test-node` job. - Success after the change: the same file passes, and so does the whole `packages/build` suite. - Product/runtime behavior verified unchanged: only the test fixture changes. ## Scope - [x] No intended public API, product behavior, visual direction, or policy change. - [x] Product changes discovered during the work were removed or split. - [x] Public text and artifacts contain no internal Meta context. ## Testing - `pnpm exec vitest run --project node packages/build`: 7 files, 85 tests pass (the target test fails without this change). - Prettier and `pnpm check:changesets`: pass. No Changeset: test-only.
User impact
An app that builds Astryx from source with the packages linked from a pnpm workspace (
packages/corelinked into itsnode_modules) cannot finishnext build --webpack. The PostCSS plugin scansnode_modules/@astryxdesign/**/*.{ts,tsx}, so it walks everything a linked package carries, not what it publishes:node_modules, whose workspace links cycle (core → its dev dependency@astryxdesign/cli→ core): in this repository's workspace the build runs out of memory at 4 GB after about eight minutes;@astryxdesign/build's test fixtures, one of them invalid on purpose.Problem and solution fit
files; the scan treats both alike.Expected behavior and authority
spec:AST-080FR7 (current) requires a source build to produce the rules of the package's modules; this one never completes. A package'sfilesfield defines what it publishes; #7754 likewise keepsnode_modulesout of its Turbopack globs.Smallest restoration
The library scan lists
node_modules/@astryxdesign/*once per real directory and reads each package'sfiles: a directory brings the.ts/.tsxfiles below it, a file itself,, and a glob or!entry fails the build, naming the package and entry, rather than approximatingnpm pack(no Astryx package lists one). Withoutfiles, the whole package minus nestednode_modules.Evidence
apps/example-nextjs-sourceadded to this workspace (Astryx linked frompackages/*), with fix(build): withAstryx keeps symlink resolution so pnpm's default layout builds #7773 applied, runs out of memory as above.postcss.workspace-links.test.mjsbuilds a workspace whose links cycle. On main two cases compile 60 files each, fixtures included; here only the published module, or afiles-less package's source. Two more expect a glob and a!entry to be refused.Scope
Testing
pnpm exec vitest run --project node packages/build: 6 files, 82 tests pass; everypostcss.workspace-links.test.mjscase fails on main.next build --webpackin the workspace copy with fix(build): withAstryx keeps symlink resolution so pnpm's default layout builds #7773 and this change: completes (226 of 226 rendered classes have a rule); with fix(build): withAstryx keeps symlink resolution so pnpm's default layout builds #7773 alone it runs out of memory.pnpm -F @astryxdesign/core typecheck, Prettier,pnpm check:repo, changeset coverage: pass (ESLint ignorespackages/build/src).