Skip to content

fix: force serverless target #343

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
May 25, 2021
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 4 additions & 10 deletions helpers/doesNotNeedPlugin.js
Original file line number Diff line number Diff line change
@@ -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
1 change: 0 additions & 1 deletion helpers/getNextConfig.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
'use strict'

const { cwd: getCwd } = require('process')
const { resolve } = require('path')

const moize = require('moize')

Expand Down
23 changes: 0 additions & 23 deletions helpers/hasCorrectNextConfig.js

This file was deleted.

38 changes: 38 additions & 0 deletions helpers/verifyBuildTarget.js
Original file line number Diff line number Diff line change
@@ -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.`,

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

NICE

)

/* 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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

love this 🙏

30 changes: 8 additions & 22 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,19 +1,13 @@
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
Expand All @@ -29,21 +23,13 @@ module.exports = {
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)
}
if (doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild })) {
return
}

// Populates the correct config if needed
await verifyBuildTarget({ netlifyConfig, packageJson, failBuild })

// Because we memoize nextConfig, we need to do this after the write file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we can prob remove this comment now i think?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

double ping on this one?

Suggested change
// Because we memoize nextConfig, we need to do this after the write file

const nextConfig = await getNextConfig(utils.failBuild)

Expand All @@ -64,7 +50,7 @@ module.exports = {
}) {
const { failBuild } = utils.build

if (await doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild })) {
if (doesNotNeedPlugin({ netlifyConfig, packageJson, failBuild })) {
return
}

Expand All @@ -76,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
}

Expand Down
49 changes: 15 additions & 34 deletions test/index.js
Original file line number Diff line number Diff line change
@@ -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`
Expand Down Expand Up @@ -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()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:O omg. we couldve done this the whole time...................

const { path, cleanup } = await getTmpDir({ unsafeCleanup: true })
const restoreCwd = changeCwd(path)
Object.assign(this, { cleanup, restoreCwd })
Expand All @@ -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' } },
Expand All @@ -89,14 +85,15 @@ describe('preBuild()', () => {
})

test('run plugin if the app has next export in an unused script', async () => {
process.env.hi = 'ok'

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hi

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hah. oops

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()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
// expect(await pathExists('next.config.js')).toBeTruthy()

})

test('do nothing if app has static html export in toml/ntl config', async () => {
Expand All @@ -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 () => {
Expand All @@ -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 () => {
Expand All @@ -133,7 +130,7 @@ describe('preBuild()', () => {
utils,
})

expect(await pathExists('next.config.js')).toBeFalsy()
expect(process.env.NEXT_PRIVATE_TARGET).toBeUndefined()

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

brilliant

})

test('fail build if the app has no package.json', async () => {
Expand Down Expand Up @@ -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' },
Expand All @@ -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()
Expand Down