fix: should throw errors if there is bad require() in vue.config.js (#…

…5500) reverts #5305 this makes the tests a little more tedious, need to find a better way to test these functionalities fixes #5442
vuejs · May 19, 2020 · 01d4bea · 01d4bea
1 parent 02a365d
commit 01d4bea
Show file tree

Hide file tree

Showing 5 changed files with 70 additions and 57 deletions.
diff --git a/packages/@vue/cli-service/__tests__/Service.spec.js b/packages/@vue/cli-service/__tests__/Service.spec.js
@@ -29,6 +29,12 @@ beforeEach(() => {
   delete process.env.BAZ
 })
 
+afterEach(() => {
+  if (fs.existsSync('/vue.config.js')) {
+    fs.unlinkSync('/vue.config.js')
+  }
+})
+
 test('env loading', () => {
   process.env.FOO = 0
   fs.writeFileSync('/.env.local', `FOO=1\nBAR=2`)
@@ -124,6 +130,7 @@ test('keep publicPath when empty', () => {
 })
 
 test('load project options from vue.config.js', () => {
+  fs.writeFileSync(path.resolve('/', 'vue.config.js'), '')  // only to ensure fs.existsSync returns true
   jest.mock(path.resolve('/', 'vue.config.js'), () => ({ lintOnSave: false }), { virtual: true })
   mockPkg({
     vue: {
@@ -136,7 +143,8 @@ test('load project options from vue.config.js', () => {
 })
 
 test('load project options from vue.config.js as a function', () => {
-  jest.mock('/vue.config.js', () => function () { return { lintOnSave: false } }, { virtual: true })
+  fs.writeFileSync(path.resolve('/', 'vue.config.js'), '')
+  jest.mock(path.resolve('/', 'vue.config.js'), () => function () { return { lintOnSave: false } }, { virtual: true })
   mockPkg({
     vue: {
       lintOnSave: 'default'

diff --git a/packages/@vue/cli-service/__tests__/ServiceESM.spec.js b/packages/@vue/cli-service/__tests__/ServiceESM.spec.js
@@ -1,11 +1,28 @@
-const { join } = require('path')
 const Service = require('../lib/Service')
 
-const mockDir = join(__dirname, 'mockESM')
-const configPath = join(mockDir, 'vue.config.cjs')
+const path = require('path')
+const configPath = path.resolve('/', 'vue.config.cjs')
+
+jest.mock('fs')
+const fs = require('fs')
+
+beforeEach(() => {
+  fs.writeFileSync(path.resolve('/', 'package.json'), JSON.stringify({
+    type: 'module',
+    vue: {
+      lintOnSave: 'default'
+    }
+  }, null, 2))
+})
+
+afterEach(() => {
+  if (fs.existsSync(configPath)) {
+    fs.unlinkSync(configPath)
+  }
+})
 
 const createService = () => {
-  const service = new Service(mockDir, {
+  const service = new Service('/', {
     plugins: [],
     useBuiltIn: false
   })
@@ -21,12 +38,14 @@ test('load project options from package.json', async () => {
 })
 
 test('load project options from vue.config.cjs', async () => {
+  fs.writeFileSync(configPath, '')
   jest.mock(configPath, () => ({ lintOnSave: true }), { virtual: true })
   const service = createService()
   expect(service.projectOptions.lintOnSave).toBe(true)
 })
 
 test('load project options from vue.config.cjs as a function', async () => {
+  fs.writeFileSync(configPath, '')
   jest.mock(configPath, () => function () { return { lintOnSave: true } }, { virtual: true })
   const service = createService()
   expect(service.projectOptions.lintOnSave).toBe(true)

diff --git a/packages/@vue/cli-service/__tests__/mockESM/package.json b/packages/@vue/cli-service/__tests__/mockESM/package.json
diff --git a/packages/@vue/cli-service/lib/Service.js b/packages/@vue/cli-service/lib/Service.js
@@ -1,3 +1,4 @@
+const fs = require('fs')
 const path = require('path')
 const debug = require('debug')
 const merge = require('webpack-merge')
@@ -10,22 +11,6 @@ const { chalk, warn, error, isPlugin, resolvePluginId, loadModule, resolvePkg }
 
 const { defaults, validate } = require('./options')
 
-const loadConfig = configPath => {
-  let fileConfig = require(configPath)
-
-  if (typeof fileConfig === 'function') {
-    fileConfig = fileConfig()
-  }
-
-  if (!fileConfig || typeof fileConfig !== 'object') {
-    error(
-      `Error loading ${chalk.bold('vue.config.js')}: should export an object or a function that returns object.`
-    )
-    fileConfig = null
-  }
-  return fileConfig
-}
-
 module.exports = class Service {
   constructor (context, { plugins, pkg, inlineOptions, useBuiltIn } = {}) {
     process.VUE_CLI_SERVICE = this
@@ -314,41 +299,46 @@ module.exports = class Service {
   }
 
   loadUserOptions () {
-    // vue.config.js
-    // vue.config.cjs
+    // vue.config.c?js
     let fileConfig, pkgConfig, resolved, resolvedFrom
     const esm = this.pkg.type && this.pkg.type === 'module'
-    const jsConfigPath = path.resolve(this.context, 'vue.config.js')
-    const cjsConfigPath = path.resolve(this.context, 'vue.config.cjs')
-    const configPath = (
-      process.env.VUE_CLI_SERVICE_CONFIG_PATH ||
-      jsConfigPath
-    )
-
-    try {
-      fileConfig = loadConfig(configPath)
-    } catch (e) {
-      if (e.code !== 'MODULE_NOT_FOUND') {
-        if (e.code === 'ERR_REQUIRE_ESM') {
-          warn(`Rename ${chalk.bold('vue.config.js')} to ${chalk.bold('vue.config.cjs')} when ECMAScript modules is enabled`)
-        }
-        error(`Error loading ${chalk.bold('vue.config.js')}:`)
-        throw e
+
+    const possibleConfigPaths = [
+      process.env.VUE_CLI_SERVICE_CONFIG_PATH,
+      './vue.config.js',
+      './vue.config.cjs'
+    ]
+
+    let fileConfigPath
+    for (const p of possibleConfigPaths) {
+      const resolvedPath = p && path.resolve(this.context, p)
+      if (resolvedPath && fs.existsSync(resolvedPath)) {
+        fileConfigPath = resolvedPath
       }
     }
 
-    // vue.config.js not found, esm enabled, no env set
-    if (!fileConfig && esm && !process.env.VUE_CLI_SERVICE_CONFIG_PATH) {
+    if (fileConfigPath) {
+      if (esm && fileConfigPath === './vue.config.js') {
+        throw new Error(`Please rename ${chalk.bold('vue.config.js')} to ${chalk.bold('vue.config.cjs')} when ECMAScript modules is enabled`)
+      }
+
       try {
-        fileConfig = loadConfig(cjsConfigPath)
-      } catch (e) {
-        if (e.code !== 'MODULE_NOT_FOUND') {
-          error(`Error loading ${chalk.bold('vue.config.cjs')}:`)
-          throw e
+        fileConfig = loadModule(fileConfigPath, this.context)
+
+        if (typeof fileConfig === 'function') {
+          fileConfig = fileConfig()
         }
-      }
-      if (fileConfig) {
-        warn(`ECMAScript modules is detected, config loaded from ${chalk.bold('vue.config.cjs')}`)
+
+        if (!fileConfig || typeof fileConfig !== 'object') {
+          // TODO: show throw an Error here, to be fixed in v5
+          error(
+            `Error loading ${chalk.bold(fileConfigPath)}: should export an object or a function that returns object.`
+          )
+          fileConfig = null
+        }
+      } catch (e) {
+        error(`Error loading ${chalk.bold(fileConfigPath)}:`)
+        throw e
       }
     }
 

diff --git a/packages/@vue/cli-shared-utils/lib/module.js b/packages/@vue/cli-shared-utils/lib/module.js
@@ -61,8 +61,10 @@ exports.resolveModule = function (request, context) {
 }
 
 exports.loadModule = function (request, context, force = false) {
-  // createRequire doesn't work with jest mock modules (which we used in migrator, for inquirer)
-  if (process.env.VUE_CLI_TEST && request.endsWith('migrator')) {
+  // createRequire doesn't work with jest mock modules
+  // (which we used in migrator for inquirer, and in tests for cli-service)
+  // TODO: it's supported in Jest 25
+  if (process.env.VUE_CLI_TEST && (request.endsWith('migrator') || context === '/')) {
     return require(request)
   }