From 8dc7ca1e63f56ab399baa3b2958a6dd2c0e3eb83 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Mon, 24 May 2021 11:43:31 +0100 Subject: [PATCH 1/5] fix: force serverless target --- helpers/getNextConfig.js | 1 - helpers/hasCorrectNextConfig.js | 23 ++++++++++++++++++++--- index.js | 17 ++--------------- 3 files changed, 22 insertions(+), 19 deletions(-) diff --git a/helpers/getNextConfig.js b/helpers/getNextConfig.js index c7a5f6f3ab..15dabac9ea 100644 --- a/helpers/getNextConfig.js +++ b/helpers/getNextConfig.js @@ -1,7 +1,6 @@ 'use strict' const { cwd: getCwd } = require('process') -const { resolve } = require('path') const moize = require('moize') diff --git a/helpers/hasCorrectNextConfig.js b/helpers/hasCorrectNextConfig.js index 36bf0ff2b5..6878353061 100644 --- a/helpers/hasCorrectNextConfig.js +++ b/helpers/hasCorrectNextConfig.js @@ -1,5 +1,4 @@ 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 @@ -12,9 +11,27 @@ const hasCorrectNextConfig = async ({ nextConfigPath, failBuild }) => { 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.`, + `The "target" config property must be one of "${acceptableTargets.join('", "')}". Setting it to "serverless".`, ) + + /* eslint-disable fp/no-delete, node/no-unpublished-require */ + + // We emulate Vercel so that we can set target to serverless if needed + process.env.NOW_BUILDER = true + // If no valid target is set, we use an internal Next env var to force it + process.env.NEXT_PRIVATE_TARGET = 'serverless' + + // 🐉 We need Next to recalculate "isZeitNow" var so we can set the target, but it's + // set as an import side effect so we need to clear the require cache first. 🐲 + // https://github.com/vercel/next.js/blob/canary/packages/next/telemetry/ci-info.ts + + delete require.cache[require.resolve('next/dist/telemetry/ci-info')] + delete require.cache[require.resolve('next/dist/next-server/server/config')] + + // Clear memoized cache + getNextConfig.clear() + + /* eslint-enable fp/no-delete, node/no-unpublished-require */ } return isValidTarget diff --git a/index.js b/index.js index 8039d44938..324d2caf61 100644 --- a/index.js +++ b/index.js @@ -28,21 +28,8 @@ module.exports = { if (hasNoPackageJson) { return failBuild('Could not find a package.json for this project') } - - 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 doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild }) // Because we memoize nextConfig, we need to do this after the write file const nextConfig = await getNextConfig(utils.failBuild) From 5b71269dcbaa81a66aabd43e7bff81185c32a44c Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Tue, 25 May 2021 11:14:27 +0100 Subject: [PATCH 2/5] fix: changes from review --- helpers/hasCorrectNextConfig.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/helpers/hasCorrectNextConfig.js b/helpers/hasCorrectNextConfig.js index 6878353061..47a2f0f407 100644 --- a/helpers/hasCorrectNextConfig.js +++ b/helpers/hasCorrectNextConfig.js @@ -11,7 +11,9 @@ const hasCorrectNextConfig = async ({ nextConfigPath, failBuild }) => { const isValidTarget = acceptableTargets.includes(target) if (!isValidTarget) { console.log( - `The "target" config property must be one of "${acceptableTargets.join('", "')}". Setting it to "serverless".`, + `The "target" config property must be one of "${acceptableTargets.join( + '", "', + )}". Building with "serverless" target.`, ) /* eslint-disable fp/no-delete, node/no-unpublished-require */ From a1ab7bf2c6ba172bdcae6ae3052abb2ba1528193 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Tue, 25 May 2021 15:52:47 +0100 Subject: [PATCH 3/5] fix: extract target checks --- helpers/doesNotNeedPlugin.js | 14 ++++------- helpers/hasCorrectNextConfig.js | 42 --------------------------------- helpers/verifyBuildTarget.js | 38 +++++++++++++++++++++++++++++ index.js | 19 +++++++-------- 4 files changed, 51 insertions(+), 62 deletions(-) delete mode 100644 helpers/hasCorrectNextConfig.js create mode 100644 helpers/verifyBuildTarget.js diff --git a/helpers/doesNotNeedPlugin.js b/helpers/doesNotNeedPlugin.js index 92cd9dcc4e..f1c01b9f15 100644 --- a/helpers/doesNotNeedPlugin.js +++ b/helpers/doesNotNeedPlugin.js @@ -1,20 +1,14 @@ 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 isStaticExportProject = require('./isStaticExportProject') -const doesNotNeedPlugin = async ({ netlifyConfig, packageJson, failBuild }) => { +const doesNotNeedPlugin = ({ netlifyConfig, packageJson }) => { const { build } = netlifyConfig - const { name, scripts = {} } = packageJson - const nextConfigPath = await findUp('next.config.js') + const { scripts = {} } = packageJson - return ( - isStaticExportProject({ build, scripts }) || - doesSiteUseNextOnNetlify({ packageJson }) || - !(await hasCorrectNextConfig({ nextConfigPath, failBuild })) - ) + return isStaticExportProject({ build, scripts }) || doesSiteUseNextOnNetlify({ packageJson }) } module.exports = doesNotNeedPlugin diff --git a/helpers/hasCorrectNextConfig.js b/helpers/hasCorrectNextConfig.js deleted file mode 100644 index 47a2f0f407..0000000000 --- a/helpers/hasCorrectNextConfig.js +++ /dev/null @@ -1,42 +0,0 @@ -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( - `The "target" config property must be one of "${acceptableTargets.join( - '", "', - )}". Building with "serverless" target.`, - ) - - /* eslint-disable fp/no-delete, node/no-unpublished-require */ - - // We emulate Vercel so that we can set target to serverless if needed - process.env.NOW_BUILDER = true - // If no valid target is set, we use an internal Next env var to force it - process.env.NEXT_PRIVATE_TARGET = 'serverless' - - // 🐉 We need Next to recalculate "isZeitNow" var so we can set the target, but it's - // set as an import side effect so we need to clear the require cache first. 🐲 - // https://github.com/vercel/next.js/blob/canary/packages/next/telemetry/ci-info.ts - - delete require.cache[require.resolve('next/dist/telemetry/ci-info')] - delete require.cache[require.resolve('next/dist/next-server/server/config')] - - // Clear memoized cache - getNextConfig.clear() - - /* eslint-enable fp/no-delete, node/no-unpublished-require */ - } - - return isValidTarget -} - -module.exports = hasCorrectNextConfig diff --git a/helpers/verifyBuildTarget.js b/helpers/verifyBuildTarget.js new file mode 100644 index 0000000000..86e380f0b9 --- /dev/null +++ b/helpers/verifyBuildTarget.js @@ -0,0 +1,38 @@ +const getNextConfig = require('./getNextConfig') +// Checks if site has the correct next.config.js +const verifyBuildTarget = async ({ failBuild }) => { + 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) { + return + } + console.log( + `The "target" config property must be one of "${acceptableTargets.join( + '", "', + )}". Building with "serverless" target.`, + ) + + /* eslint-disable fp/no-delete, node/no-unpublished-require */ + + // We emulate Vercel so that we can set target to serverless if needed + process.env.NOW_BUILDER = true + // If no valid target is set, we use an internal Next env var to force it + process.env.NEXT_PRIVATE_TARGET = 'serverless' + + // 🐉 We need Next to recalculate "isZeitNow" var so we can set the target, but it's + // set as an import side effect so we need to clear the require cache first. 🐲 + // https://github.com/vercel/next.js/blob/canary/packages/next/telemetry/ci-info.ts + + delete require.cache[require.resolve('next/dist/telemetry/ci-info')] + delete require.cache[require.resolve('next/dist/next-server/server/config')] + + // Clear memoized cache + getNextConfig.clear() + + /* eslint-enable fp/no-delete, node/no-unpublished-require */ +} + +module.exports = verifyBuildTarget diff --git a/index.js b/index.js index 324d2caf61..4fd818ad90 100644 --- a/index.js +++ b/index.js @@ -1,8 +1,3 @@ -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') @@ -10,10 +5,9 @@ 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 @@ -28,8 +22,13 @@ module.exports = { if (hasNoPackageJson) { return failBuild('Could not find a package.json for this project') } + + if (doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild })) { + return + } + // Populates the correct config if needed - await doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild }) + await verifyBuildTarget({ netlifyConfig, packageJson, failBuild }) // Because we memoize nextConfig, we need to do this after the write file const nextConfig = await getNextConfig(utils.failBuild) @@ -51,7 +50,7 @@ module.exports = { }) { const { failBuild } = utils.build - if (await doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild })) { + if (doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild })) { return } @@ -63,7 +62,7 @@ module.exports = { }, async onPostBuild({ netlifyConfig, packageJson, constants: { FUNCTIONS_DIST }, utils }) { - if (await doesNotNeedPlugin({ netlifyConfig, packageJson, utils })) { + if (doesNotNeedPlugin({ netlifyConfig, packageJson, utils })) { return } From 630cb1a52982c1bb40de3e92e5b09b4af9097d36 Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Tue, 25 May 2021 15:53:00 +0100 Subject: [PATCH 4/5] fix: fix tests --- test/index.js | 49 +++++++++++++++---------------------------------- 1 file changed, 15 insertions(+), 34 deletions(-) diff --git a/test/index.js b/test/index.js index bc7c3afd0c..ef69a79006 100644 --- a/test/index.js +++ b/test/index.js @@ -1,10 +1,12 @@ 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` @@ -47,6 +49,11 @@ 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 () => { + 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 }) @@ -66,17 +73,6 @@ 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' } }, @@ -89,14 +85,15 @@ describe('preBuild()', () => { }) test('run plugin if the app has next export in an unused script', async () => { + process.env.hi = 'ok' await plugin.onPreBuild({ netlifyConfig, packageJson: { ...DUMMY_PACKAGE_JSON, scripts: { export: 'next export' } }, utils, constants: {}, }) - - expect(await pathExists('next.config.js')).toBeTruthy() + 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 () => { @@ -107,7 +104,7 @@ describe('preBuild()', () => { constants: { FUNCTIONS_SRC: 'out_functions' }, }) - expect(await pathExists('next.config.js')).toBeFalsy() + expect(process.env.NEXT_PRIVATE_TARGET).toBeUndefined() }) test('do nothing if app has next-on-netlify installed', async () => { @@ -120,7 +117,7 @@ describe('preBuild()', () => { utils, }) - expect(await pathExists('next.config.js')).toBeFalsy() + expect(process.env.NEXT_PRIVATE_TARGET).toBeUndefined() }) test('do nothing if app has next-on-netlify postbuild script', async () => { @@ -133,7 +130,7 @@ describe('preBuild()', () => { utils, }) - expect(await pathExists('next.config.js')).toBeFalsy() + expect(process.env.NEXT_PRIVATE_TARGET).toBeUndefined() }) test('fail build if the app has no package.json', async () => { @@ -185,6 +182,7 @@ 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' }, @@ -202,23 +200,6 @@ 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() From 09642bdf9806588ddb58a7871eab1184818860cf Mon Sep 17 00:00:00 2001 From: Matt Kane Date: Tue, 25 May 2021 16:54:34 +0100 Subject: [PATCH 5/5] fix: changes from review --- index.js | 1 - test/index.js | 3 +-- 2 files changed, 1 insertion(+), 3 deletions(-) diff --git a/index.js b/index.js index 4fd818ad90..18779bee69 100644 --- a/index.js +++ b/index.js @@ -30,7 +30,6 @@ module.exports = { // 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) { diff --git a/test/index.js b/test/index.js index ef69a79006..0fbeacc540 100644 --- a/test/index.js +++ b/test/index.js @@ -49,6 +49,7 @@ 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')] @@ -85,7 +86,6 @@ describe('preBuild()', () => { }) test('run plugin if the app has next export in an unused script', async () => { - process.env.hi = 'ok' await plugin.onPreBuild({ netlifyConfig, packageJson: { ...DUMMY_PACKAGE_JSON, scripts: { export: 'next export' } }, @@ -93,7 +93,6 @@ describe('preBuild()', () => { 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 () => {