Conversation
…DE_PATH After facebook#5327 made node_modules strict, three CommonJS requires kept resolving without a declaration: pnpm's command shims for vitest and next put node_modules/.pnpm/node_modules on NODE_PATH, and require folds NODE_PATH in. Run without it, each one fails with MODULE_NOT_FOUND. - packages/build/src/index.js, the ./postcss entry, requires postcss, which the package has never declared (facebook#1447). It is now an optional peer: the PostCSS plugin guidelines keep postcss a peer so a plugin works on the host's AST, and the Babel, Vite and Next entries never load it. - apps/sandbox/next.config.mjs resolves lexical to pin its ESM build (facebook#4380). lexical is a richtext dependency; the sandbox never declared it. - packages/build/src/next.test.mjs drives enhanced-resolve (facebook#5932), which only arrives transitively through next. All three resolve to versions the lockfile already carries, so it gains importer entries and no new package versions.
resolveLexicalDist caught a failed lookup and returned null, and the webpack hook then skipped the lexical$ alias. lab's bare lexical imports would then resolve through lexical's `source` export to its raw TS, which this app's Babel pipeline rejects (facebook#4380) -- an error about class fields, far from the missing package that caused it. lexical is now a declared devDependency, so a failed lookup means a broken install rather than an optional package. Let it throw where the cause is.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
meta-cla
Bot
added
the
CLA Signed
github-actions
Bot
added
community
facebook#4380 pinned bare lexical imports issued from @astryxdesign modules to lexical's built ESM: lab's RichTextEditor imported lexical, and withAstryx's `source` condition sent it to lexical's raw TS. facebook#4678 moved the editor into @astryxdesign/richtext, which the sandbox does not depend on, and left the alias behind. Nothing the sandbox compiles imports lexical, @lexical/* or richtext, and the build output contains no lexical code with or without the alias. Remove it, along with the lexical devDependency the earlier commit declared for it.
ButtonGroup.test.tsx and Selector.source-build.test.mjs hand Babel bare preset names. Babel looks those up from the cwd -- the repo root, which declares none of them -- so they resolved only through the node_modules/.pnpm/node_modules entry pnpm's vitest shim puts on NODE_PATH. Without it, the first fails on @babel/preset-typescript and the second on next. Resolve each preset from the test's own package instead. core already declares both Babel presets; it now declares next too, at the 16.3.0-preview.5 the test was already running against, so the lockfile reuses the existing snapshot.
Addresses the review on facebook#6372. - ./postcss required postcss only to parse its generated CSS. It now passes replaceWith() a string, which the host's own postcss parses, so both the require and the optional peer the earlier commit declared go away. - ./vite required lightningcss and browserslist from its own location, where neither is declared, and its try/catch turned the miss into a silently skipped pass. It now resolves both from @stylexjs/unplugin, which depends on them and whose output that pass reproduces. The test guarding the pass looked for any -webkit- prefix, which an authored one always satisfied. It now checks a prefix only the pass adds, and fails on the old code without NODE_PATH. - apps/sandbox declares autoprefixer. build's postcss() config adds it, and Next resolves it from the app.
Addresses the review on facebook#6372. - ./postcss required postcss only to parse its generated CSS. It now passes replaceWith() a string, which the host's own postcss parses, so both the require and the optional peer the earlier commit declared go away. - ./vite required lightningcss and browserslist from its own location, where neither is declared, and its try/catch turned the miss into a silently skipped pass. It now resolves both from @stylexjs/unplugin, which depends on them and whose output that pass reproduces. The test guarding the pass looked for any -webkit- prefix, which an authored one always satisfied. It now checks a prefix only the pass adds, and fails on the old code without NODE_PATH. - apps/sandbox declares autoprefixer. build's postcss() config adds it, and Next resolves it from the app.
Han5991
changed the title
Han5991
marked this pull request as ready for review
September 19, 2026 05:54
Han5991
requested review from
cixzhang,
imdreamrunner and
josephfarina
as code owners
September 19, 2026 05:54
Contributor
PR Analysis Report📚 Storybook PreviewView Storybook for this PR 🧪 Sandbox PreviewView Sandbox for this PR No new or modified components detected. Bundle Size Summary
Accessibility AuditStatus: 36 accessibility violation(s) found — 2 critical, 34 serious. Button pattern - 1 issue(s)
ChatComposer - 1 issue(s)
ChatComposerInput - 1 issue(s)
ChatReasoning - 1 issue(s)
ChatToolCalls - 1 issue(s)
Checkbox pattern - 1 issue(s)
CheckboxList - 1 issue(s)
ClickableCard - 1 issue(s)
CodeEditor - 1 issue(s)
CodeEditorPerf - 1 issue(s)
CodeEditorTheme - 1 issue(s)
CodeTheme - 1 issue(s)
DateRangeInput - 1 issue(s)
FileInput - 1 issue(s)
GridMasonry - 1 issue(s)
Heading - 1 issue(s)
Icon - 1 issue(s)
LogStream - 1 issue(s)
MediaTheme Auto - 1 issue(s)
PowerSearch - 1 issue(s)
ProgressBar - 1 issue(s)
Radio group pattern - 1 issue(s)
RadioList - 1 issue(s)
RichTextEditor - 2 issue(s)
SelectableCard - 1 issue(s)
Stepper - 1 issue(s)
TableGroupedRows - 1 issue(s)
TableTree - 1 issue(s)
Text - 1 issue(s)
Theme - 1 issue(s)
PopArt - 1 issue(s)
Thumbnail - 1 issue(s)
Timestamp - 1 issue(s)
Token - 1 issue(s)
Tokenizer - 1 issue(s)
Generated by PR Enrichment workflow | Storybook | Sandbox | View full report |
|
AI review status for this pull request.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Maintainer impact
pnpm's command shims put
node_modules/.pnpm/node_modulesonNODE_PATH. That let these lookups resolve undervitest/nextwithout being declared, and each fails outside a shim:@astryxdesign/build/postcss→postcss(feat: consolidate build plugins into @xds/build #1447)@astryxdesign/build/vite→lightningcss,browserslist. The miss was caught, so the lightningcss pass was silently skipped.packages/build/src/next.test.mjs→enhanced-resolve(fix(build): resolve app imports of astryx to source in withAstryx #5932)autoprefixerapps/sandbox/next.config.mjs→lexical(fix(lab): RichTextEditor Tab keyboard trap — Escape then Tab moves focus #4380), for an alias nothing has used since feat(richtext,lab): promote RichTextEditor from lab into @astryxdesign/richtext #4678ButtonGroup.test.tsxandSelector.source-build.test.mjs→ Babel presets passed by bare nameIntended invariant
A package resolves only what it declares (#5327).
Change
./postcsspasses the host's postcss a CSS string to parse, so it needs nopostcssof its own../viteresolveslightningcssandbrowserslistfrom@stylexjs/unplugin, which depends on both. Its test now checks for a prefix only that pass adds.enhanced-resolvein buildautoprefixerin the sandboxnextin core, at the16.3.0-preview.5the test already ran againstlexical$alias.Evidence
NODE_PATH. The old vite test stays green even when the pass is skipped.NODE_PATH. The new vite assertion fails on the old code.Scope
@astryxdesign/buildhas a changeset.Testing
Ran locally:
pnpm build,check:repo,lint, every CI typecheck gate,verify-exports, CLI smoke tests, docsitegenerate+test, storybook and sandbox builds, and vitestnode+ui. The only failures were the macOS-only ones noted in #5327. The theme-layer cascade guard needs Playwright's browser, which is left to CI.