fix(builder): resolve postcss plugins from the app root for monorepo installs#8755
Open
BleedingDev wants to merge 2 commits into
Open
fix(builder): resolve postcss plugins from the app root for monorepo installs#8755BleedingDev wants to merge 2 commits into
BleedingDev wants to merge 2 commits into
Conversation
🦋 Changeset detectedLatest commit: 33e8a0a The changes in this PR will be included in the next version bump. This PR includes changesets to release 117 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
✅ Deploy Preview for modernjs-byted ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 71e2bfb9ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+27
to
+29
| } catch (error) { | ||
| firstError ??= error; | ||
| } |
There was a problem hiding this comment.
When a resolver actually finds the package but the package throws during evaluation (for example an app-root PostCSS plugin has an ESM-only entry or a missing transitive dependency), this catch treats it the same as a lookup miss and tries the next roots. In the new app-root fallback path that can load a different copy from process.cwd() or, if none exists, throw the saved builder MODULE_NOT_FOUND, hiding the real failure. Please only continue on MODULE_NOT_FOUND for the requested name and rethrow other load errors.
Useful? React with 👍 / 👎.
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.
Problem
Builder loads built-in PostCSS plugins with a require anchored to the builder module. In pnpm monorepos, a PostCSS plugin dependency can be resolvable from the app root while not being resolvable from the builder package.
Fix
Add an app-root-aware PostCSS plugin loader that falls back through the existing builder resolver. Pass the builder context root path when loading the default PostCSS plugin set.
Test
Added a builder unit test that creates a temporary app root with a PostCSS plugin in node_modules and resolves it through the loader. Ran
PUPPETEER_SKIP_DOWNLOAD=true pnpm install,pnpm --filter @modern-js/builder build, andpnpm --filter @modern-js/builder test.🤖 Generated with Claude Code