fix: backport update failure points in plugin to do nothing instead to v1

helpers/doesNotNeedPlugin.js

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

helpers/doesSiteUseNextOnNetlify.js

@@ -0,0 +1,20 @@
+// Checks if site is already using next-on-netlify
+const { name: pluginName } = require('../package.json')
+
+const doesSiteUseNextOnNetlify = ({ packageJson }) => {
+  const { name, scripts = {}, dependencies = {} } = packageJson
+
+  const hasNextOnNetlifyInstalled = dependencies['next-on-netlify'] !== undefined
+  const hasNextOnNetlifyPostbuildScript =
+    typeof scripts.postbuild === 'string' && scripts.postbuild.includes('next-on-netlify')
+  const isUsingNextOnNetlify = (hasNextOnNetlifyInstalled || hasNextOnNetlifyPostbuildScript) && pluginName !== name
+  if (isUsingNextOnNetlify) {
+    console.log(
+      `This plugin does not support sites that manually use next-on-netlify. Uninstall next-on-netlify as a dependency and/or remove it from your postbuild script to allow this plugin to run.`,
+    )
+  }
+
+  return isUsingNextOnNetlify
+}
+
+module.exports = doesSiteUseNextOnNetlify

helpers/hasCorrectNextConfig.js

@@ -0,0 +1,27 @@
+const path = require('path')
+
+// Checks if site has the correct next.cofig.js
+const hasCorrectNextConfig = (nextConfigPath) => {
+  // In the plugin's case, no config is valid because we'll make it ourselves
+  if (nextConfigPath === undefined) return true
+
+  // We cannot load `next` at the top-level because we validate whether the
+  // site is using `next` inside `onPreBuild`.
+  const { PHASE_PRODUCTION_BUILD } = require('next/constants')
+  const { default: loadConfig } = require('next/dist/next-server/server/config')
+
+  // If the next config exists, log warning if target isnt in acceptableTargets
+  const acceptableTargets = ['serverless', 'experimental-serverless-trace']
+  const nextConfig = loadConfig(PHASE_PRODUCTION_BUILD, path.resolve('.'))
+  const isValidTarget = acceptableTargets.includes(nextConfig.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/isStaticExportProject.js

@@ -1,7 +1,6 @@
 // Takes 1. Netlify config's build details and
 // 2. the project's package.json scripts to determine if
 // the Next.js app uses static HTML export
-
 const isStaticExportProject = ({ build, scripts }) => {
   const NEXT_EXPORT_COMMAND = 'next export'
 

index.js

@@ -1,13 +1,11 @@
 const fs = require('fs')
 const path = require('path')
 const util = require('util')
-
-const findUp = require('find-up')
 const makeDir = require('make-dir')
+const findUp = require('find-up')
 
-const { name: pluginName } = require('./package.json')
-const isStaticExportProject = require('./helpers/isStaticExportProject')
 const validateNextUsage = require('./helpers/validateNextUsage')
+const doesNotNeedPlugin = require('./helpers/doesNotNeedPlugin')
 
 const pWriteFile = util.promisify(fs.writeFile)
 
@@ -21,58 +19,31 @@ module.exports = {
 
     validateNextUsage(failBuild)
 
-    if (Object.keys(packageJson).length === 0) {
-      return failBuild(`Could not find a package.json for this project`)
+    const hasNoPackageJson = Object.keys(packageJson).length === 0
+    if (hasNoPackageJson) {
+      return failBuild('Could not find a package.json for this project')
     }
 
-    const { build } = netlifyConfig
-    const { name, scripts = {}, dependencies = {} } = packageJson
-
-    if (isStaticExportProject({ build, scripts })) {
-      return failBuild(
-        `Static HTML export Next.js projects do not require this plugin. Check your project's build command for 'next export'.`,
-      )
-    }
-
-    const hasNextOnNetlifyInstalled = dependencies['next-on-netlify'] !== undefined
-    const hasNextOnNetlifyPostbuildScript =
-      typeof scripts.postbuild === 'string' && scripts.postbuild.includes('next-on-netlify')
-    const isAlreadyUsingNextOnNetlify =
-      (hasNextOnNetlifyInstalled || hasNextOnNetlifyPostbuildScript) && pluginName !== name
-    if (isAlreadyUsingNextOnNetlify) {
-      return failBuild(
-        `This plugin does not support sites that manually use next-on-netlify. Uninstall next-on-netlify as a dependency to resolve.`,
-      )
+    if (await doesNotNeedPlugin({ netlifyConfig, packageJson })) {
+      return
     }
 
     const nextConfigPath = await findUp('next.config.js')
-    if (nextConfigPath !== undefined) {
-      // We cannot load `next` at the top-level because we validate whether the
-      // site is using `next` inside `onPreBuild`.
-      const { PHASE_PRODUCTION_BUILD } = require('next/constants')
-      const { default: loadConfig } = require('next/dist/next-server/server/config')
-
-      // If the next config exists, fail build if target isnt in acceptableTargets
-      const acceptableTargets = ['serverless', 'experimental-serverless-trace']
-      const nextConfig = loadConfig(PHASE_PRODUCTION_BUILD, path.resolve('.'))
-      const isValidTarget = acceptableTargets.includes(nextConfig.target)
-      if (!isValidTarget) {
-        return failBuild(
-          `Your next.config.js must set the "target" property to one of: ${acceptableTargets.join(', ')}`,
-        )
-      }
-    } else {
+    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)
-      console.log(`** Adding next.config.js with target set to 'serverless' **`)
     }
   },
-  async onBuild({ constants: { PUBLISH_DIR, FUNCTIONS_SRC = DEFAULT_FUNCTIONS_SRC } }) {
+  async onBuild({ netlifyConfig, packageJson, constants: { PUBLISH_DIR, FUNCTIONS_SRC = DEFAULT_FUNCTIONS_SRC } }) {
+    if (await doesNotNeedPlugin({ netlifyConfig, packageJson })) {
+      return
+    }
+
     console.log(`** Running Next on Netlify package **`)
 
     await makeDir(PUBLISH_DIR)

test/index.js

@@ -61,71 +61,74 @@ const DUMMY_PACKAGE_JSON = { name: 'dummy', version: '1.0.0' }
 const netlifyConfig = { build: {} }
 
 describe('preBuild()', () => {
-  test('fail build if the app has static html export in npm script', async () => {
-    await expect(
-      plugin.onPreBuild({
-        netlifyConfig: { build: { command: 'npm run build' } },
-        packageJson: { ...DUMMY_PACKAGE_JSON, scripts: { build: 'next export' } },
-        utils,
-        constants: { FUNCTIONS_SRC: 'out_functions' },
-      }),
-    ).rejects.toThrow(
-      `Static HTML export Next.js projects do not require this plugin. Check your project's build command for 'next export'.`,
-    )
+  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 not fail build if the app has next export in an unused script', async () => {
-    await expect(
-      plugin.onPreBuild({
-        netlifyConfig,
-        packageJson: { ...DUMMY_PACKAGE_JSON, scripts: { export: 'next export' } },
-        utils,
-        constants: {},
-      }),
-    ).resolves
+  test('do nothing if the app has static html export in npm script', async () => {
+    await plugin.onPreBuild({
+      netlifyConfig: { build: { command: 'npm run build' } },
+      packageJson: { ...DUMMY_PACKAGE_JSON, scripts: { build: 'next export' } },
+      utils,
+      constants: { FUNCTIONS_SRC: 'out_functions' },
+    })
+
+    expect(await pathExists('next.config.js')).toBeFalsy()
   })
 
-  test('fail build if the app has static html export in toml/ntl config', async () => {
-    await expect(
-      plugin.onPreBuild({
-        netlifyConfig: { build: { command: 'next build && next export' } },
-        packageJson: DUMMY_PACKAGE_JSON,
-        utils,
-        constants: { FUNCTIONS_SRC: 'out_functions' },
-      }),
-    ).rejects.toThrow(
-      `Static HTML export Next.js projects do not require this plugin. Check your project's build command for 'next export'.`,
-    )
+  test('run plugin if the app has next export in an unused script', async () => {
+    await plugin.onPreBuild({
+      netlifyConfig,
+      packageJson: { ...DUMMY_PACKAGE_JSON, scripts: { export: 'next export' } },
+      utils,
+      constants: {},
+    })
+
+    expect(await pathExists('next.config.js')).toBeTruthy()
   })
 
-  test('fail build if app has next-on-netlify installed', async () => {
+  test('do nothing if app has static html export in toml/ntl config', async () => {
+    await plugin.onPreBuild({
+      netlifyConfig: { build: { command: 'next build && next export' } },
+      packageJson: DUMMY_PACKAGE_JSON,
+      utils,
+      constants: { FUNCTIONS_SRC: 'out_functions' },
+    })
+
+    expect(await pathExists('next.config.js')).toBeFalsy()
+  })
+
+  test('do nothing if app has next-on-netlify installed', async () => {
     const packageJson = {
       dependencies: { 'next-on-netlify': '123' },
     }
-    await expect(
-      plugin.onPreBuild({
-        netlifyConfig,
-        packageJson,
-        utils,
-      }),
-    ).rejects.toThrow(
-      `This plugin does not support sites that manually use next-on-netlify. Uninstall next-on-netlify as a dependency to resolve.`,
-    )
+    await plugin.onPreBuild({
+      netlifyConfig,
+      packageJson,
+      utils,
+    })
+
+    expect(await pathExists('next.config.js')).toBeFalsy()
   })
 
-  test('fail build if app has next-on-netlify postbuild script', async () => {
+  test('do nothing if app has next-on-netlify postbuild script', async () => {
     const packageJson = {
       scripts: { postbuild: 'next-on-netlify' },
     }
-    await expect(
-      plugin.onPreBuild({
-        netlifyConfig,
-        packageJson,
-        utils,
-      }),
-    ).rejects.toThrow(
-      `This plugin does not support sites that manually use next-on-netlify. Uninstall next-on-netlify as a dependency to resolve.`,
-    )
+    await plugin.onPreBuild({
+      netlifyConfig,
+      packageJson,
+      utils,
+    })
+
+    expect(await pathExists('next.config.js')).toBeFalsy()
   })
 
   test('fail build if the app has no package.json', async () => {
@@ -138,42 +141,48 @@ describe('preBuild()', () => {
       }),
     ).rejects.toThrow(`Could not find a package.json for this project`)
   })
+})
 
-  test('create next.config.js with correct target if file does not exist', async () => {
-    await plugin.onPreBuild({
+describe('onBuild()', () => {
+  test('does not run onBuild if using next-on-netlify', async () => {
+    const packageJson = {
+      scripts: { postbuild: 'next-on-netlify' },
+    }
+    await useFixture('publish_copy_files')
+    await moveNextDist()
+    const PUBLISH_DIR = 'publish'
+    await plugin.onBuild({
       netlifyConfig,
-      packageJson: DUMMY_PACKAGE_JSON,
-      utils,
-      constants: { FUNCTIONS_SRC: 'out_functions' },
+      packageJson,
+      constants: {},
     })
 
-    expect(await pathExists('next.config.js')).toBeTruthy()
+    expect(await pathExists(`${PUBLISH_DIR}/index.html`)).toBeFalsy()
   })
 
   test.each(['invalid_next_config', 'deep_invalid_next_config'])(
-    `fail build if the app's next config has an invalid target`,
+    `do nothing if the app's next config has an invalid target`,
     async (fixtureName) => {
       await useFixture(fixtureName)
-      await expect(
-        plugin.onPreBuild({
-          netlifyConfig,
-          packageJson: DUMMY_PACKAGE_JSON,
-          utils,
-          constants: { FUNCTIONS_SRC: 'out_functions' },
-        }),
-      ).rejects.toThrow(
-        `Your next.config.js must set the "target" property to one of: serverless, experimental-serverless-trace`,
-      )
+      const PUBLISH_DIR = 'publish'
+      await plugin.onBuild({
+        netlifyConfig,
+        packageJson: DUMMY_PACKAGE_JSON,
+        utils,
+        constants: { FUNCTIONS_SRC: 'out_functions' },
+      })
+
+      expect(await pathExists(`${PUBLISH_DIR}/index.html`)).toBeFalsy()
     },
   )
-})
 
-describe('onBuild()', () => {
   test('copy files to the publish directory', async () => {
     await useFixture('publish_copy_files')
     await moveNextDist()
     const PUBLISH_DIR = 'publish'
     await plugin.onBuild({
+      netlifyConfig,
+      packageJson: DUMMY_PACKAGE_JSON,
       constants: {
         PUBLISH_DIR,
         FUNCTIONS_SRC: 'functions',
@@ -191,6 +200,8 @@ describe('onBuild()', () => {
     await useFixture('functions_copy_files')
     await moveNextDist()
     await plugin.onBuild({
+      netlifyConfig,
+      packageJson: DUMMY_PACKAGE_JSON,
       constants: {
         FUNCTIONS_SRC,
         PUBLISH_DIR: '.',