fix: importing an optional peer dep should throw an runtime error #20029
Conversation
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
sapphi-red
added
has workaround
p2-edge-case
sapphi-red
commented
May 12, 2025
Comment on lines
+492
to
498
| // rollup + @rollup/plugin-commonjs hoists dynamic `require`s by default | ||
| // If we add a `throw` statement, it will be injected to the top-level and break the whole bundle | ||
| // Instead, we mock the module for now | ||
| // This can be fixed when we migrate to rolldown | ||
| if (isRequire === 'true' && isProduction) { | ||
| return 'export default {}' | ||
| } |
Member
Author
There was a problem hiding this comment.
We can remove this part and fix the require case when we migrate to rolldown (vitejs#167).
bluwy
approved these changes
May 12, 2025
Member
bluwy
left a comment
bluwy
left a comment
There was a problem hiding this comment.
Makes sense to me. Good catch 👍
|
Any reason for not merging this? |
Member
Author
I'm holding off until another team member has a chance to review it, or until I can revisit it with fresh eyes. |
7 tasks
github-merge-queue bot
pushed a commit
to rolldown/rolldown
that referenced
this pull request
Jun 17, 2025
… an runtime error (#4980) Port of vitejs/vite#20029 and vitejs/rolldown-vite#167.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
Description
Currently
await import('optional-peer-dep')andrequire('optional-peer-dep')does not throw an runtime error after bundling with Vite. This is caused by mocking the optional peer dep module (introduced by #9321).Note: This current mocking behavior can be skipped by adding the module to
externaloption.While we still need to keep the module mocked for
require(see comment in the code for details), we can makeawait import('optional-peer-dep')to throw an runtime error to align with the input behavior (although,import 'optional-peer-dep'should be an early error, not a runtime error).refs #6007 #9321 vitejs#165 rolldown/rolldown#4051 (comment)