fix: Throw error only when 'umd' or 'iife' are used

vitejs · Aug 28, 2022 · 0602746 · 0602746
1 parent 45d6797
commit 0602746
Show file tree

Hide file tree

Showing 2 changed files with 209 additions and 99 deletions.
diff --git a/packages/vite/src/node/__tests__/build.spec.ts b/packages/vite/src/node/__tests__/build.spec.ts
@@ -1,8 +1,10 @@
 import { resolve } from 'node:path'
 import { fileURLToPath } from 'node:url'
-import { describe, expect, test } from 'vitest'
+import type { OutputOptions } from 'rollup'
+import { describe, expect, test, vi } from 'vitest'
 import type { LibraryFormats, LibraryOptions } from '../build'
-import { resolveLibFilename } from '../build'
+import { resolveBuildOutputs, resolveLibFilename } from '../build'
+import { createLogger } from '../logger'
 
 const __dirname = resolve(fileURLToPath(import.meta.url), '..')
 
@@ -12,105 +14,204 @@ const baseLibOptions: LibraryOptions = {
   entry: 'mylib.js'
 }
 
-describe('resolveLibFilename', () => {
-  test('custom filename function', () => {
-    const filename = resolveLibFilename(
-      {
-        fileName: (format) => `custom-filename-function.${format}.js`,
-        entry: 'mylib.js'
-      },
-      'es',
-      resolve(__dirname, 'packages/name')
-    )
-
-    expect(filename).toBe('custom-filename-function.es.js')
-  })
+describe('build', () => {
+  describe('resolveBuildOutputs', () => {
+    test('resolves outputs correctly', () => {
+      const logger = createLogger()
+      const libOptions: LibraryOptions = { ...baseLibOptions }
+      const outputs: OutputOptions[] = [{ format: 'es' }]
+      const resolvedOutputs = resolveBuildOutputs(outputs, libOptions, logger)
 
-  test('custom filename string', () => {
-    const filename = resolveLibFilename(
-      {
-        fileName: 'custom-filename',
-        entry: 'mylib.js'
-      },
-      'es',
-      resolve(__dirname, 'packages/name')
-    )
-
-    expect(filename).toBe('custom-filename.mjs')
-  })
+      expect(resolvedOutputs).toEqual([
+        {
+          format: 'es'
+        }
+      ])
+    })
 
-  test('package name as filename', () => {
-    const filename = resolveLibFilename(
-      {
-        entry: 'mylib.js'
-      },
-      'es',
-      resolve(__dirname, 'packages/name')
-    )
+    test('resolves outputs from lib options', () => {
+      const logger = createLogger()
+      const libOptions: LibraryOptions = { ...baseLibOptions, name: 'lib' }
+      const resolvedOutputs = resolveBuildOutputs(void 0, libOptions, logger)
 
-    expect(filename).toBe('mylib.mjs')
-  })
+      expect(resolvedOutputs).toEqual([
+        {
+          format: 'es'
+        },
+        {
+          format: 'umd'
+        }
+      ])
+    })
+
+    test('does not change outputs when lib options are missing', () => {
+      const logger = createLogger()
+      const outputs: OutputOptions[] = [{ format: 'es' }]
+      const resolvedOutputs = resolveBuildOutputs(outputs, false, logger)
+
+      expect(resolvedOutputs).toEqual(outputs)
+    })
+
+    test('logs a warning when outputs is an array and formats are specified', () => {
+      const logger = createLogger()
+      const loggerSpy = vi.spyOn(logger, 'warn').mockImplementation(() => {})
+      const libOptions: LibraryOptions = {
+        ...baseLibOptions,
+        formats: ['iife']
+      }
+      const outputs: OutputOptions[] = [{ format: 'es' }]
+
+      resolveBuildOutputs(outputs, libOptions, logger)
+
+      expect(loggerSpy).toHaveBeenCalledWith(
+        expect.stringContaining('"build.lib.formats" will be ignored because')
+      )
+    })
+
+    test('throws an error when lib.name is missing on iife format', () => {
+      const logger = createLogger()
+      const libOptions: LibraryOptions = {
+        ...baseLibOptions,
+        formats: ['iife']
+      }
+      const resolveBuild = () => resolveBuildOutputs(void 0, libOptions, logger)
+
+      expect(resolveBuild).toThrowError(/Option "build\.lib\.name" is required/)
+    })
+
+    test('throws an error when lib.name is missing on umd format', () => {
+      const logger = createLogger()
+      const libOptions: LibraryOptions = { ...baseLibOptions, formats: ['umd'] }
+      const resolveBuild = () => resolveBuildOutputs(void 0, libOptions, logger)
+
+      expect(resolveBuild).toThrowError(/Option "build\.lib\.name" is required/)
+    })
+
+    test('throws an error when output.name is missing on iife format', () => {
+      const logger = createLogger()
+      const libOptions: LibraryOptions = { ...baseLibOptions }
+      const outputs: OutputOptions[] = [{ format: 'iife' }]
+      const resolveBuild = () =>
+        resolveBuildOutputs(outputs, libOptions, logger)
+
+      expect(resolveBuild).toThrowError(
+        /Entries in "build\.rollupOptions\.output" must specify "name"/
+      )
+    })
+
+    test('throws an error when output.name is missing on umd format', () => {
+      const logger = createLogger()
+      const libOptions: LibraryOptions = { ...baseLibOptions }
+      const outputs: OutputOptions[] = [{ format: 'umd' }]
+      const resolveBuild = () =>
+        resolveBuildOutputs(outputs, libOptions, logger)
 
-  test('custom filename and no package name', () => {
-    const filename = resolveLibFilename(
-      {
-        fileName: 'custom-filename',
-        entry: 'mylib.js'
-      },
-      'es',
-      resolve(__dirname, 'packages/noname')
-    )
-
-    expect(filename).toBe('custom-filename.mjs')
+      expect(resolveBuild).toThrowError(
+        /Entries in "build\.rollupOptions\.output" must specify "name"/
+      )
+    })
   })
 
-  test('missing filename', () => {
-    expect(() => {
-      resolveLibFilename(
+  describe('resolveLibFilename', () => {
+    test('custom filename function', () => {
+      const filename = resolveLibFilename(
         {
+          fileName: (format) => `custom-filename-function.${format}.js`,
           entry: 'mylib.js'
         },
         'es',
-        resolve(__dirname, 'packages/noname')
+        resolve(__dirname, 'packages/name')
       )
-    }).toThrow()
-  })
 
-  test('commonjs package extensions', () => {
-    const formatsToFilenames: FormatsToFileNames = [
-      ['es', 'my-lib.mjs'],
-      ['umd', 'my-lib.umd.js'],
-      ['cjs', 'my-lib.js'],
-      ['iife', 'my-lib.iife.js']
-    ]
+      expect(filename).toBe('custom-filename-function.es.js')
+    })
 
-    for (const [format, expectedFilename] of formatsToFilenames) {
+    test('custom filename string', () => {
       const filename = resolveLibFilename(
-        baseLibOptions,
-        format,
-        resolve(__dirname, 'packages/noname')
+        {
+          fileName: 'custom-filename',
+          entry: 'mylib.js'
+        },
+        'es',
+        resolve(__dirname, 'packages/name')
       )
 
-      expect(filename).toBe(expectedFilename)
-    }
-  })
+      expect(filename).toBe('custom-filename.mjs')
+    })
+
+    test('package name as filename', () => {
+      const filename = resolveLibFilename(
+        {
+          entry: 'mylib.js'
+        },
+        'es',
+        resolve(__dirname, 'packages/name')
+      )
 
-  test('module package extensions', () => {
-    const formatsToFilenames: FormatsToFileNames = [
-      ['es', 'my-lib.js'],
-      ['umd', 'my-lib.umd.cjs'],
-      ['cjs', 'my-lib.cjs'],
-      ['iife', 'my-lib.iife.js']
-    ]
+      expect(filename).toBe('mylib.mjs')
+    })
 
-    for (const [format, expectedFilename] of formatsToFilenames) {
+    test('custom filename and no package name', () => {
       const filename = resolveLibFilename(
-        baseLibOptions,
-        format,
-        resolve(__dirname, 'packages/module')
+        {
+          fileName: 'custom-filename',
+          entry: 'mylib.js'
+        },
+        'es',
+        resolve(__dirname, 'packages/noname')
       )
 
-      expect(expectedFilename).toBe(filename)
-    }
+      expect(filename).toBe('custom-filename.mjs')
+    })
+
+    test('missing filename', () => {
+      expect(() => {
+        resolveLibFilename(
+          {
+            entry: 'mylib.js'
+          },
+          'es',
+          resolve(__dirname, 'packages/noname')
+        )
+      }).toThrow()
+    })
+
+    test('commonjs package extensions', () => {
+      const formatsToFilenames: FormatsToFileNames = [
+        ['es', 'my-lib.mjs'],
+        ['umd', 'my-lib.umd.js'],
+        ['cjs', 'my-lib.js'],
+        ['iife', 'my-lib.iife.js']
+      ]
+
+      for (const [format, expectedFilename] of formatsToFilenames) {
+        const filename = resolveLibFilename(
+          baseLibOptions,
+          format,
+          resolve(__dirname, 'packages/noname')
+        )
+
+        expect(filename).toBe(expectedFilename)
+      }
+    })
+
+    test('module package extensions', () => {
+      const formatsToFilenames: FormatsToFileNames = [
+        ['es', 'my-lib.js'],
+        ['umd', 'my-lib.umd.cjs'],
+        ['cjs', 'my-lib.cjs'],
+        ['iife', 'my-lib.iife.js']
+      ]
+
+      for (const [format, expectedFilename] of formatsToFilenames) {
+        const filename = resolveLibFilename(
+          baseLibOptions,
+          format,
+          resolve(__dirname, 'packages/module')
+        )
+
+        expect(expectedFilename).toBe(filename)
+      }
+    })
   })
 })
diff --git a/packages/vite/src/node/build.ts b/packages/vite/src/node/build.ts
@@ -643,36 +643,45 @@ export function resolveLibFilename(
   return `${name}.${format}.${extension}`
 }
 
-function resolveBuildOutputs(
+export function resolveBuildOutputs(
   outputs: OutputOptions | OutputOptions[] | undefined,
   libOptions: LibraryOptions | false,
   logger: Logger
 ): OutputOptions | OutputOptions[] | undefined {
   if (libOptions) {
-    const formats = libOptions.formats || ['es', 'umd']
-    if (
-      (formats.includes('umd') || formats.includes('iife')) &&
-      !libOptions.name
-    ) {
-      throw new Error(
-        `Option "build.lib.name" is required when output formats ` +
-          `include "umd" or "iife".`
-      )
+    const libFormats = libOptions.formats || ['es', 'umd']
+
+    if (!Array.isArray(outputs)) {
+      const requiresName =
+        libFormats.includes('umd') || libFormats.includes('iife')
+
+      if (requiresName && !libOptions.name) {
+        throw new Error(
+          'Option "build.lib.name" is required when output formats include "umd" or "iife".'
+        )
+      }
+
+      return libFormats.map((format) => ({ ...outputs, format }))
     }
-    if (!outputs) {
-      return formats.map((format) => ({ format }))
-    } else if (!Array.isArray(outputs)) {
-      return formats.map((format) => ({ ...outputs, format }))
-    } else if (libOptions.formats) {
-      // user explicitly specifying own output array
+
+    // By this point, we know "outputs" is an Array.
+    if (libOptions.formats) {
       logger.warn(
         colors.yellow(
-          `"build.lib.formats" will be ignored because ` +
-            `"build.rollupOptions.output" is already an array format`
+          '"build.lib.formats" will be ignored because "build.rollupOptions.output" is already an array format.'
         )
       )
     }
+
+    outputs.forEach((output) => {
+      if (['umd', 'iife'].includes(output.format!) && !output.name) {
+        throw new Error(
+          'Entries in "build.rollupOptions.output" must specify "name" when output formats include "umd" or "iife".'
+        )
+      }
+    })
   }
+
   return outputs
 }