fix: revert fix force serverless target (#388)

Author: lindsaylevine

helpers/doesNotNeedPlugin.js

@@ -1,14 +1,20 @@
 const findUp = require('find-up')
 
 // Checks all the cases for which the plugin should do nothing
-const doesSiteUseNextOnNetlify = require('./doesSiteUseNextOnNetlify')
 const isStaticExportProject = require('./isStaticExportProject')
+const doesSiteUseNextOnNetlify = require('./doesSiteUseNextOnNetlify')
+const hasCorrectNextConfig = require('./hasCorrectNextConfig')
 
-const doesNotNeedPlugin = ({ netlifyConfig, packageJson }) => {
+const doesNotNeedPlugin = async ({ netlifyConfig, packageJson, failBuild }) => {
   const { build } = netlifyConfig
-  const { scripts = {} } = packageJson
+  const { name, scripts = {} } = packageJson
+  const nextConfigPath = await findUp('next.config.js')
 
-  return isStaticExportProject({ build, scripts }) || doesSiteUseNextOnNetlify({ packageJson })
+  return (
+    isStaticExportProject({ build, scripts }) ||
+    doesSiteUseNextOnNetlify({ packageJson }) ||
+    !(await hasCorrectNextConfig({ nextConfigPath, failBuild }))
+  )
 }
 
 module.exports = doesNotNeedPlugin

helpers/getNextConfig.js

@@ -1,6 +1,7 @@
 'use strict'
 
 const { cwd: getCwd } = require('process')
+const { resolve } = require('path')
 
 const moize = require('moize')
 

helpers/hasCorrectNextConfig.js

@@ -0,0 +1,23 @@
+const getNextConfig = require('./getNextConfig')
+
+// Checks if site has the correct next.config.js
+const hasCorrectNextConfig = async ({ nextConfigPath, failBuild }) => {
+  // In the plugin's case, no config is valid because we'll make it ourselves
+  if (nextConfigPath === undefined) return true
+
+  const { target } = await getNextConfig(failBuild)
+
+  // If the next config exists, log warning if target isnt in acceptableTargets
+  const acceptableTargets = ['serverless', 'experimental-serverless-trace']
+  const isValidTarget = acceptableTargets.includes(target)
+  if (!isValidTarget) {
+    console.log(
+      `Your next.config.js must set the "target" property to one of: ${acceptableTargets.join(', ')}. Update the
+      target property to allow this plugin to run.`,
+    )
+  }
+
+  return isValidTarget
+}
+
+module.exports = hasCorrectNextConfig

helpers/verifyBuildTarget.js

@@ -1,13 +1,19 @@
+const fs = require('fs')
+const path = require('path')
+const util = require('util')
+
+const findUp = require('find-up')
 const makeDir = require('make-dir')
 
 const { restoreCache, saveCache } = require('./helpers/cacheBuild')
 const copyUnstableIncludedDirs = require('./helpers/copyUnstableIncludedDirs')
 const doesNotNeedPlugin = require('./helpers/doesNotNeedPlugin')
 const getNextConfig = require('./helpers/getNextConfig')
 const validateNextUsage = require('./helpers/validateNextUsage')
-const verifyBuildTarget = require('./helpers/verifyBuildTarget')
 const nextOnNetlify = require('./src/index.js')
 
+const pWriteFile = util.promisify(fs.writeFile)
+
 // * Helpful Plugin Context *
 // - Between the prebuild and build steps, the project's build command is run
 // - Between the build and postbuild steps, any functions are bundled
@@ -23,13 +29,22 @@ module.exports = {
       return failBuild('Could not find a package.json for this project')
     }
 
-    if (doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild })) {
-      return
+    const pluginNotNeeded = await doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild })
+
+    if (!pluginNotNeeded) {
+      const nextConfigPath = await findUp('next.config.js')
+      if (nextConfigPath === undefined) {
+        // Create the next config file with target set to serverless by default
+        const nextConfig = `
+            module.exports = {
+              target: 'serverless'
+            }
+          `
+        await pWriteFile('next.config.js', nextConfig)
+      }
     }
 
-    // Populates the correct config if needed
-    await verifyBuildTarget({ netlifyConfig, packageJson, failBuild })
-
+    // Because we memoize nextConfig, we need to do this after the write file
     const nextConfig = await getNextConfig(utils.failBuild)
 
     if (nextConfig.images.domains.length !== 0 && !process.env.NEXT_IMAGE_ALLOWED_DOMAINS) {
@@ -49,7 +64,7 @@ module.exports = {
   }) {
     const { failBuild } = utils.build
 
-    if (doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild })) {
+    if (await doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild })) {
       return
     }
 
@@ -61,7 +76,7 @@ module.exports = {
   },
 
   async onPostBuild({ netlifyConfig, packageJson, constants: { FUNCTIONS_DIST }, utils }) {
-    if (doesNotNeedPlugin({ netlifyConfig, packageJson, utils })) {
+    if (await doesNotNeedPlugin({ netlifyConfig, packageJson, utils })) {
       return
     }
 

index.js

@@ -1,12 +1,10 @@
 const path = require('path')
 const process = require('process')
-
-const cpy = require('cpy')
 const pathExists = require('path-exists')
 const { dir: getTmpDir } = require('tmp-promise')
+const cpy = require('cpy')
 
 const plugin = require('..')
-const getNextConfig = require('../helpers/getNextConfig')
 
 const FIXTURES_DIR = `${__dirname}/fixtures`
 const SAMPLE_PROJECT_DIR = `${__dirname}/sample`
@@ -49,12 +47,6 @@ const useFixture = async function (fixtureName) {
 // In each test, we change cwd to a temporary directory.
 // This allows us not to have to mock filesystem operations.
 beforeEach(async () => {
-  // This is so we can test the target setting code
-  delete process.env.NEXT_PRIVATE_TARGET
-  delete require.cache[require.resolve('next/dist/telemetry/ci-info')]
-  delete require.cache[require.resolve('next/dist/next-server/server/config')]
-
-  getNextConfig.clear()
   const { path, cleanup } = await getTmpDir({ unsafeCleanup: true })
   const restoreCwd = changeCwd(path)
   Object.assign(this, { cleanup, restoreCwd })
@@ -74,6 +66,17 @@ const DUMMY_PACKAGE_JSON = { name: 'dummy', version: '1.0.0' }
 const netlifyConfig = { build: {} }
 
 describe('preBuild()', () => {
+  test('create next.config.js with correct target if file does not exist', async () => {
+    await plugin.onPreBuild({
+      netlifyConfig,
+      packageJson: DUMMY_PACKAGE_JSON,
+      utils,
+      constants: { FUNCTIONS_SRC: 'out_functions' },
+    })
+
+    expect(await pathExists('next.config.js')).toBeTruthy()
+  })
+
   test('do nothing if the app has static html export in npm script', async () => {
     await plugin.onPreBuild({
       netlifyConfig: { build: { command: 'npm run build' } },
@@ -92,7 +95,8 @@ describe('preBuild()', () => {
       utils,
       constants: {},
     })
-    expect(process.env.NEXT_PRIVATE_TARGET).toBe('serverless')
+
+    expect(await pathExists('next.config.js')).toBeTruthy()
   })
 
   test('do nothing if app has static html export in toml/ntl config', async () => {
@@ -103,7 +107,7 @@ describe('preBuild()', () => {
       constants: { FUNCTIONS_SRC: 'out_functions' },
     })
 
-    expect(process.env.NEXT_PRIVATE_TARGET).toBeUndefined()
+    expect(await pathExists('next.config.js')).toBeFalsy()
   })
 
   test('do nothing if app has next-on-netlify installed', async () => {
@@ -116,7 +120,7 @@ describe('preBuild()', () => {
       utils,
     })
 
-    expect(process.env.NEXT_PRIVATE_TARGET).toBeUndefined()
+    expect(await pathExists('next.config.js')).toBeFalsy()
   })
 
   test('do nothing if app has next-on-netlify postbuild script', async () => {
@@ -129,7 +133,7 @@ describe('preBuild()', () => {
       utils,
     })
 
-    expect(process.env.NEXT_PRIVATE_TARGET).toBeUndefined()
+    expect(await pathExists('next.config.js')).toBeFalsy()
   })
 
   test('fail build if the app has no package.json', async () => {
@@ -181,7 +185,6 @@ describe('preBuild()', () => {
 })
 
 describe('onBuild()', () => {
-  // eslint-disable-next-line max-lines
   test('does not run onBuild if using next-on-netlify', async () => {
     const packageJson = {
       scripts: { postbuild: 'next-on-netlify' },
@@ -199,6 +202,23 @@ describe('onBuild()', () => {
     expect(await pathExists(`${PUBLISH_DIR}/index.html`)).toBeFalsy()
   })
 
+  test.each(['invalid_next_config', 'deep_invalid_next_config'])(
+    `do nothing if the app's next config has an invalid target`,
+    async (fixtureName) => {
+      await useFixture(fixtureName)
+      const PUBLISH_DIR = 'publish'
+      await plugin.onBuild({
+        netlifyConfig,
+        packageJson: DUMMY_PACKAGE_JSON,
+        utils,
+        constants: { FUNCTIONS_SRC: 'out_functions' },
+        utils,
+      })
+
+      expect(await pathExists(`${PUBLISH_DIR}/index.html`)).toBeFalsy()
+    },
+  )
+
   test('copy files to the publish directory', async () => {
     await useFixture('publish_copy_files')
     await moveNextDist()