fix: importing an optional peer dep should throw an runtime error

Author: sapphi-red

packages/vite/src/node/plugins/resolve.ts

@@ -772,14 +772,7 @@ export function resolvePlugin(
           }
         }
         if (id.startsWith(optionalPeerDepId)) {
-          const [, peerDep, parentDep, isRequire] = id.split(':')
-          // 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 {}'
-          }
+          const [, peerDep, parentDep] = id.split(':')
           return (
             'export default {};' +
             `throw new Error(\`Could not resolve "${peerDep}" imported by "${parentDep}".${isProduction ? '' : ' Is it installed?'}\`)`
@@ -1048,7 +1041,7 @@ export function tryNodeResolve(
           mainPkg.peerDependenciesMeta?.[pkgName]?.optional
         ) {
           return {
-            id: `${optionalPeerDepId}:${id}:${mainPkg.name}:${!!options.isRequire}`,
+            id: `${optionalPeerDepId}:${id}:${mainPkg.name}`,
           }
         }
       }

playground/optimize-deps/__tests__/optimize-deps.spec.ts

@@ -161,10 +161,9 @@ test('dep with optional peer dep (cjs)', async () => {
   await expectWithRetry(() =>
     page.textContent('.dep-with-optional-peer-dep-cjs'),
   ).toMatch(`[success]`)
-  // FIXME
-  // await expectWithRetry(() =>
-  //   page.textContent('.dep-with-optional-peer-dep-cjs-error'),
-  // ).toMatch(`[success]`)
+  await expectWithRetry(() =>
+    page.textContent('.dep-with-optional-peer-dep-cjs-error'),
+  ).toMatch(`[success]`)
 })
 
 test('dep with css import', async () => {