Enforce transactional optional dependency installs

Author: cezaraugusto

ci-scripts/run-optional-deps-smoke.mjs

@@ -141,17 +141,7 @@ async function terminateChildProcess(child) {
 }
 
 function buildSmokeEnv(pm) {
-  if (pm !== 'pnpm') return baseEnv
-
-  const npmCommandPath = resolveCommandPath('npm')
-  return {
-    ...baseEnv,
-    // Force the isolated optional-deps cache installs through npm so the
-    // smoke test can focus on lockfile stability instead of host-specific
-    // pnpm shim resolution in nested child processes.
-    EXTENSION_JS_PACKAGE_MANAGER: 'npm',
-    ...(npmCommandPath ? {EXTENSION_JS_PM_EXEC_PATH: npmCommandPath} : {})
-  }
+  return baseEnv
 }
 
 function commandFor(tool) {

programs/develop/webpack/optional-deps-lib/__spec__/installer-engine.spec.ts

@@ -27,6 +27,20 @@ const createSpawn = (
   }
 })
 
+function seedInstalledDependency(installRoot: string, dependencyId: string) {
+  const packageDir = path.join(
+    installRoot,
+    'node_modules',
+    ...dependencyId.split('/')
+  )
+  fs.mkdirSync(packageDir, {recursive: true})
+  fs.writeFileSync(path.join(packageDir, 'index.js'), 'module.exports = {}')
+  fs.writeFileSync(
+    path.join(packageDir, 'package.json'),
+    JSON.stringify({name: dependencyId, version: '0.0.0'})
+  )
+}
+
 vi.mock('child_process', () => ({
   execFileSync: vi.fn(),
   spawnSync: vi.fn(() => ({status: 0})),
@@ -66,7 +80,9 @@ describe('optional-deps-lib installer engine', () => {
 
   it('installs optional tooling into isolated cache root', async () => {
     const {spawn} = (await import('child_process')) as any
-    const {installOptionalDependencies} = await import('../index')
+    const {installOptionalDependencies, resolveOptionalInstallRoot} =
+      await import('../index')
+    seedInstalledDependency(resolveOptionalInstallRoot(), 'postcss')
 
     await installOptionalDependencies('PostCSS', ['postcss'])
 
@@ -80,6 +96,9 @@ describe('optional-deps-lib installer engine', () => {
     process.env.npm_config_user_agent = 'npm'
     const {installOptionalDependencies, resolveOptionalInstallRoot} =
       await import('../index')
+    const installRoot = resolveOptionalInstallRoot()
+    seedInstalledDependency(installRoot, 'react-refresh')
+    seedInstalledDependency(installRoot, '@rspack/plugin-react-refresh')
 
     await installOptionalDependencies('React', [
       'react-refresh',
@@ -97,7 +116,9 @@ describe('optional-deps-lib installer engine', () => {
   it('runs a single command for single-package install', async () => {
     process.env.npm_config_user_agent = 'npm'
     const {spawn} = (await import('child_process')) as any
-    const {installOptionalDependencies} = await import('../index')
+    const {installOptionalDependencies, resolveOptionalInstallRoot} =
+      await import('../index')
+    seedInstalledDependency(resolveOptionalInstallRoot(), 'postcss')
 
     await installOptionalDependencies('PostCSS', ['postcss'])
 
@@ -107,7 +128,12 @@ describe('optional-deps-lib installer engine', () => {
   it('runs root relink for multi-package install', async () => {
     process.env.npm_config_user_agent = 'npm'
     const {spawn} = (await import('child_process')) as any
-    const {installOptionalDependencies} = await import('../index')
+    const {installOptionalDependencies, resolveOptionalInstallRoot} =
+      await import('../index')
+    const installRoot = resolveOptionalInstallRoot()
+    seedInstalledDependency(installRoot, 'vue-loader')
+    seedInstalledDependency(installRoot, '@vue/compiler-sfc')
+    seedInstalledDependency(installRoot, 'vue')
 
     await installOptionalDependencies('Vue', [
       'vue-loader',
@@ -122,6 +148,32 @@ describe('optional-deps-lib installer engine', () => {
     expect(relinkArgs).toContain('install')
   })
 
+  it('recovers from partial install by topping up missing dependency', async () => {
+    process.env.npm_config_user_agent = 'npm'
+    const {spawn} = (await import('child_process')) as any
+    const {installOptionalDependencies, resolveOptionalInstallRoot} =
+      await import('../index')
+    const installRoot = resolveOptionalInstallRoot()
+    seedInstalledDependency(installRoot, '@rspack/plugin-react-refresh')
+
+    let callIndex = 0
+    spawn.mockImplementation(() => {
+      if (callIndex === 2) {
+        seedInstalledDependency(installRoot, 'react-refresh')
+      }
+      callIndex += 1
+      return createSpawn({exitCode: 0})
+    })
+
+    const result = await installOptionalDependencies('React', [
+      'react-refresh',
+      '@rspack/plugin-react-refresh'
+    ])
+
+    expect(result).toBe(true)
+    expect(callIndex).toBeGreaterThanOrEqual(3)
+  })
+
   it('does not fallback on windows when primary manager is npm', async () => {
     setPlatform('win32')
     process.env.npm_config_user_agent = 'npm'

programs/develop/webpack/optional-deps-lib/installer-engine.ts

@@ -1,3 +1,5 @@
+import * as path from 'path'
+import {createRequire} from 'module'
 import colors from 'pintor'
 import * as messages from '../plugin-css/css-lib/messages'
 import {resolveOptionalDependencySpecs} from '../webpack-lib/optional-dependencies'
@@ -34,6 +36,29 @@ export type InstallOptionalDependenciesOptions = {
   forceRecreateInstallRoot?: boolean
 }
 
+function isPathInside(basePath: string, candidatePath: string) {
+  const relative = path.relative(
+    path.resolve(basePath),
+    path.resolve(candidatePath)
+  )
+  return !relative.startsWith('..') && !path.isAbsolute(relative)
+}
+
+function getMissingDependenciesAtInstallRoot(
+  dependencies: string[],
+  installBaseDir: string
+) {
+  const req = createRequire(path.join(installBaseDir, 'package.json'))
+  return dependencies.filter((dependencyId) => {
+    try {
+      const resolvedPath = req.resolve(dependencyId)
+      return !isPathInside(installBaseDir, resolvedPath)
+    } catch {
+      return true
+    }
+  })
+}
+
 function parseWslUncPath(value: string): {distro: string; path: string} | null {
   const match = /^\\\\wsl(?:\.localhost)?\\([^\\]+)\\(.+)$/.exec(value)
   if (!match) return null
@@ -226,6 +251,76 @@ function getRootInstallCommand(
   return buildInstallCommand(pm, ['install', '--silent', ...dirArgs])
 }
 
+async function runInstallAttempt(input: {
+  pm: PackageManagerResolution
+  wslContext: WslContext
+  installBaseDir: string
+  dependencies: string[]
+  isAuthor: boolean
+  integration: string
+}) {
+  const installCommand = getOptionalInstallCommand(
+    input.pm,
+    input.dependencies,
+    input.wslContext.installDir || input.installBaseDir
+  )
+  const execCommand = wrapCommandForWsl(installCommand, input.wslContext)
+  const fallbackNpmCommand = input.wslContext.useWsl
+    ? undefined
+    : buildNpmCliFallback([
+        '--silent',
+        'install',
+        ...resolveOptionalDependencySpecs(input.dependencies),
+        '--prefix',
+        input.installBaseDir,
+        '--save-optional'
+      ])
+  await execInstallWithFallback(execCommand, {
+    cwd: input.wslContext.useWsl ? undefined : input.installBaseDir,
+    fallbackNpmCommand,
+    allowFallbackOnFailure:
+      !input.wslContext.useWsl &&
+      input.pm.name !== 'npm' &&
+      fallbackNpmCommand !== undefined
+  })
+
+  await new Promise((resolve) => setTimeout(resolve, 500))
+
+  const needsRootRelink = input.isAuthor || input.dependencies.length > 1
+  if (!needsRootRelink) return
+
+  if (input.isAuthor) {
+    console.log(messages.optionalToolingRootInstall(input.integration))
+  }
+
+  const rootInstall = getRootInstallCommand(
+    input.pm,
+    input.wslContext.useWsl ? input.wslContext.installDir : undefined
+  )
+  const rootCommand = wrapCommandForWsl(rootInstall, input.wslContext)
+  const rootFallbackCommand = input.wslContext.useWsl
+    ? undefined
+    : buildNpmCliFallback([
+        '--silent',
+        'install',
+        '--prefix',
+        input.installBaseDir
+      ])
+
+  await execInstallWithFallback(rootCommand, {
+    cwd: input.wslContext.useWsl ? undefined : input.installBaseDir,
+    fallbackNpmCommand: rootFallbackCommand,
+    allowFallbackOnFailure:
+      !input.wslContext.useWsl &&
+      input.pm.name !== 'npm' &&
+      rootFallbackCommand !== undefined
+  })
+
+  if (input.isAuthor) {
+    console.log(messages.optionalToolingReady(input.integration))
+  }
+}
+
 export async function installOptionalDependencies(
   integration: string,
   dependencies: string[],
@@ -268,63 +363,63 @@ export async function installOptionalDependencies(
     if (isAuthor) console.warn(setupMessageWithIndex)
     else console.log(setupMessageWithIndex)
 
-    const installCommand = getOptionalInstallCommand(
+    await runInstallAttempt({
       pm,
+      wslContext,
+      installBaseDir,
       dependencies,
-      wslContext.installDir || installBaseDir
-    )
-    const execCommand = wrapCommandForWsl(installCommand, wslContext)
-    const fallbackNpmCommand = wslContext.useWsl
-      ? undefined
-      : buildNpmCliFallback([
-          '--silent',
-          'install',
-          ...resolveOptionalDependencySpecs(dependencies),
-          '--prefix',
-          installBaseDir,
-          '--save-optional'
-        ])
-    await execInstallWithFallback(execCommand, {
-      cwd: wslContext.useWsl ? undefined : installBaseDir,
-      fallbackNpmCommand,
-      allowFallbackOnFailure:
-        !wslContext.useWsl &&
-        pm.name !== 'npm' &&
-        fallbackNpmCommand !== undefined
+      isAuthor,
+      integration
     })
 
-    await new Promise((resolve) => setTimeout(resolve, 500))
+    let missingDependencies = getMissingDependenciesAtInstallRoot(
+      dependencies,
+      installBaseDir
+    )
 
-    const needsRootRelink = isAuthor || dependencies.length > 1
-    if (needsRootRelink) {
-      if (isAuthor) {
-        console.log(messages.optionalToolingRootInstall(integration))
-      }
-      const rootInstall = getRootInstallCommand(
+    if (missingDependencies.length > 0) {
+      await runInstallAttempt({
         pm,
-        wslContext.useWsl ? wslContext.installDir : undefined
+        wslContext,
+        installBaseDir,
+        dependencies: missingDependencies,
+        isAuthor,
+        integration
+      })
+
+      missingDependencies = getMissingDependenciesAtInstallRoot(
+        dependencies,
+        installBaseDir
       )
-      const rootCommand = wrapCommandForWsl(rootInstall, wslContext)
-      const rootFallbackCommand = wslContext.useWsl
-        ? undefined
-        : buildNpmCliFallback([
-            '--silent',
-            'install',
-            '--prefix',
-            installBaseDir
-          ])
-      await execInstallWithFallback(rootCommand, {
-        cwd: wslContext.useWsl ? undefined : installBaseDir,
-        fallbackNpmCommand: rootFallbackCommand,
-        allowFallbackOnFailure:
-          !wslContext.useWsl &&
-          pm.name !== 'npm' &&
-          rootFallbackCommand !== undefined
+    }
+
+    if (missingDependencies.length > 0) {
+      prepareOptionalInstallState({
+        installBaseDir,
+        dependencies,
+        forceRecreateInstallRoot: true
       })
-      if (isAuthor) {
-        console.log(messages.optionalToolingReady(integration))
-      }
+      await runInstallAttempt({
+        pm,
+        wslContext,
+        installBaseDir,
+        dependencies,
+        isAuthor,
+        integration
+      })
+
+      missingDependencies = getMissingDependenciesAtInstallRoot(
+        dependencies,
+        installBaseDir
+      )
     }
+
+    if (missingDependencies.length > 0) {
+      throw new Error(
+        `[${integration}] Optional dependency install reported success but packages are missing: ${missingDependencies.join(', ')}`
+      )
+    }
+
     return true
   } catch (error) {
     console.error('[extension.js][optional-deps] debug', {

programs/develop/webpack/plugin-js-frameworks/__spec__/js-tools/react.spec.ts

@@ -8,9 +8,7 @@ vi.mock('../../frameworks-lib/integrations', () => ({
 
 vi.mock('../../../webpack-lib/optional-deps-resolver', () => ({
   resolveOptionalPackageWithoutInstall: vi.fn(() => '/mock/react-refresh'),
-  loadOptionalModuleWithoutInstall: vi.fn(
-    () => class ReactRefreshPluginMock {}
-  )
+  loadOptionalModuleWithoutInstall: vi.fn(() => class ReactRefreshPluginMock {})
 }))
 
 // Ensure require.resolve('react-refresh') succeeds to avoid install+exit

programs/develop/webpack/plugin-js-frameworks/js-tools/react.ts

@@ -95,17 +95,16 @@ export async function maybeUseReact(
     verifyPackageIds: reactDependencies
   })
 
-  const ReactRefreshPlugin = loadOptionalModuleWithoutInstall<
-    ReactRefreshPluginCtor
-  >({
-    integration: 'React',
-    projectPath,
-    dependencyId: '@rspack/plugin-react-refresh',
-    installDependencies: reactDependencies,
-    verifyPackageIds: reactDependencies,
-    moduleAdapter: (mod: any) =>
-      ((mod && mod.default) || mod) as ReactRefreshPluginCtor
-  })
+  const ReactRefreshPlugin =
+    loadOptionalModuleWithoutInstall<ReactRefreshPluginCtor>({
+      integration: 'React',
+      projectPath,
+      dependencyId: '@rspack/plugin-react-refresh',
+      installDependencies: reactDependencies,
+      verifyPackageIds: reactDependencies,
+      moduleAdapter: (mod: any) =>
+        ((mod && mod.default) || mod) as ReactRefreshPluginCtor
+    })
 
   const reactPlugins: RspackPluginInstance[] = [
     new ReactRefreshPlugin({

programs/develop/webpack/webpack-lib/__spec__/preflight-optional-deps.spec.ts

@@ -112,5 +112,4 @@ describe('preflight-optional-deps', () => {
 
     expect(installOptionalDependenciesBatch).not.toHaveBeenCalled()
   })
-
 })

programs/develop/webpack/webpack-lib/optional-deps-resolver.ts

@@ -625,7 +625,9 @@ export async function ensureOptionalPackageResolved(
   )
 }
 
-export function resolveOptionalPackageWithoutInstall(input: EnsureResolveInput) {
+export function resolveOptionalPackageWithoutInstall(
+  input: EnsureResolveInput
+) {
   const installDependencies = input.installDependencies || [input.dependencyId]
   const verifyPackageIds = input.verifyPackageIds || installDependencies
   const installRoot = resolveOptionalInstallRoot()