Skip to content

Commit 9a32ace

Browse files
erezrokaherezrokah
authored
fix: backport update failure points in plugin to do nothing instead to v1 (#146)
1 parent 19228c4 commit 9a32ace

6 files changed

+161
-113
lines changed

‎helpers/doesNotNeedPlugin.js

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
const findUp = require('find-up')
2+
3+
// Checks all the cases for which the plugin should do nothing
4+
const isStaticExportProject = require('./isStaticExportProject')
5+
const doesSiteUseNextOnNetlify = require('./doesSiteUseNextOnNetlify')
6+
const hasCorrectNextConfig = require('./hasCorrectNextConfig')
7+
8+
const doesNotNeedPlugin = async ({ netlifyConfig, packageJson }) => {
9+
const { build } = netlifyConfig
10+
const { name, scripts = {} } = packageJson
11+
const nextConfigPath = await findUp('next.config.js')
12+
13+
return (
14+
isStaticExportProject({ build, scripts }) ||
15+
doesSiteUseNextOnNetlify({ packageJson }) ||
16+
!hasCorrectNextConfig(nextConfigPath)
17+
)
18+
}
19+
20+
module.exports = doesNotNeedPlugin

‎helpers/doesSiteUseNextOnNetlify.js

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
// Checks if site is already using next-on-netlify
2+
const { name: pluginName } = require('../package.json')
3+
4+
const doesSiteUseNextOnNetlify = ({ packageJson }) => {
5+
const { name, scripts = {}, dependencies = {} } = packageJson
6+
7+
const hasNextOnNetlifyInstalled = dependencies['next-on-netlify'] !== undefined
8+
const hasNextOnNetlifyPostbuildScript =
9+
typeof scripts.postbuild === 'string' && scripts.postbuild.includes('next-on-netlify')
10+
const isUsingNextOnNetlify = (hasNextOnNetlifyInstalled || hasNextOnNetlifyPostbuildScript) && pluginName !== name
11+
if (isUsingNextOnNetlify) {
12+
console.log(
13+
`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.`,
14+
)
15+
}
16+
17+
return isUsingNextOnNetlify
18+
}
19+
20+
module.exports = doesSiteUseNextOnNetlify

‎helpers/hasCorrectNextConfig.js

+27
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
const path = require('path')
2+
3+
// Checks if site has the correct next.cofig.js
4+
const hasCorrectNextConfig = (nextConfigPath) => {
5+
// In the plugin's case, no config is valid because we'll make it ourselves
6+
if (nextConfigPath === undefined) return true
7+
8+
// We cannot load `next` at the top-level because we validate whether the
9+
// site is using `next` inside `onPreBuild`.
10+
const { PHASE_PRODUCTION_BUILD } = require('next/constants')
11+
const { default: loadConfig } = require('next/dist/next-server/server/config')
12+
13+
// If the next config exists, log warning if target isnt in acceptableTargets
14+
const acceptableTargets = ['serverless', 'experimental-serverless-trace']
15+
const nextConfig = loadConfig(PHASE_PRODUCTION_BUILD, path.resolve('.'))
16+
const isValidTarget = acceptableTargets.includes(nextConfig.target)
17+
if (!isValidTarget) {
18+
console.log(
19+
`Your next.config.js must set the "target" property to one of: ${acceptableTargets.join(', ')}. Update the
20+
target property to allow this plugin to run.`,
21+
)
22+
}
23+
24+
return isValidTarget
25+
}
26+
27+
module.exports = hasCorrectNextConfig

‎helpers/isStaticExportProject.js

-1
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
// Takes 1. Netlify config's build details and
22
// 2. the project's package.json scripts to determine if
33
// the Next.js app uses static HTML export
4-
54
const isStaticExportProject = ({ build, scripts }) => {
65
const NEXT_EXPORT_COMMAND = 'next export'
76

‎index.js

+13-42
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,11 @@
11
const fs = require('fs')
22
const path = require('path')
33
const util = require('util')
4-
5-
const findUp = require('find-up')
64
const makeDir = require('make-dir')
5+
const findUp = require('find-up')
76

8-
const { name: pluginName } = require('./package.json')
9-
const isStaticExportProject = require('./helpers/isStaticExportProject')
107
const validateNextUsage = require('./helpers/validateNextUsage')
8+
const doesNotNeedPlugin = require('./helpers/doesNotNeedPlugin')
119

1210
const pWriteFile = util.promisify(fs.writeFile)
1311

@@ -21,58 +19,31 @@ module.exports = {
2119

2220
validateNextUsage(failBuild)
2321

24-
if (Object.keys(packageJson).length === 0) {
25-
return failBuild(`Could not find a package.json for this project`)
22+
const hasNoPackageJson = Object.keys(packageJson).length === 0
23+
if (hasNoPackageJson) {
24+
return failBuild('Could not find a package.json for this project')
2625
}
2726

28-
const { build } = netlifyConfig
29-
const { name, scripts = {}, dependencies = {} } = packageJson
30-
31-
if (isStaticExportProject({ build, scripts })) {
32-
return failBuild(
33-
`Static HTML export Next.js projects do not require this plugin. Check your project's build command for 'next export'.`,
34-
)
35-
}
36-
37-
const hasNextOnNetlifyInstalled = dependencies['next-on-netlify'] !== undefined
38-
const hasNextOnNetlifyPostbuildScript =
39-
typeof scripts.postbuild === 'string' && scripts.postbuild.includes('next-on-netlify')
40-
const isAlreadyUsingNextOnNetlify =
41-
(hasNextOnNetlifyInstalled || hasNextOnNetlifyPostbuildScript) && pluginName !== name
42-
if (isAlreadyUsingNextOnNetlify) {
43-
return failBuild(
44-
`This plugin does not support sites that manually use next-on-netlify. Uninstall next-on-netlify as a dependency to resolve.`,
45-
)
27+
if (await doesNotNeedPlugin({ netlifyConfig, packageJson })) {
28+
return
4629
}
4730

4831
const nextConfigPath = await findUp('next.config.js')
49-
if (nextConfigPath !== undefined) {
50-
// We cannot load `next` at the top-level because we validate whether the
51-
// site is using `next` inside `onPreBuild`.
52-
const { PHASE_PRODUCTION_BUILD } = require('next/constants')
53-
const { default: loadConfig } = require('next/dist/next-server/server/config')
54-
55-
// If the next config exists, fail build if target isnt in acceptableTargets
56-
const acceptableTargets = ['serverless', 'experimental-serverless-trace']
57-
const nextConfig = loadConfig(PHASE_PRODUCTION_BUILD, path.resolve('.'))
58-
const isValidTarget = acceptableTargets.includes(nextConfig.target)
59-
if (!isValidTarget) {
60-
return failBuild(
61-
`Your next.config.js must set the "target" property to one of: ${acceptableTargets.join(', ')}`,
62-
)
63-
}
64-
} else {
32+
if (nextConfigPath === undefined) {
6533
// Create the next config file with target set to serverless by default
6634
const nextConfig = `
6735
module.exports = {
6836
target: 'serverless'
6937
}
7038
`
7139
await pWriteFile('next.config.js', nextConfig)
72-
console.log(`** Adding next.config.js with target set to 'serverless' **`)
7340
}
7441
},
75-
async onBuild({ constants: { PUBLISH_DIR, FUNCTIONS_SRC = DEFAULT_FUNCTIONS_SRC } }) {
42+
async onBuild({ netlifyConfig, packageJson, constants: { PUBLISH_DIR, FUNCTIONS_SRC = DEFAULT_FUNCTIONS_SRC } }) {
43+
if (await doesNotNeedPlugin({ netlifyConfig, packageJson })) {
44+
return
45+
}
46+
7647
console.log(`** Running Next on Netlify package **`)
7748

7849
await makeDir(PUBLISH_DIR)

‎test/index.js

+81-70
Original file line numberDiff line numberDiff line change
@@ -61,71 +61,74 @@ const DUMMY_PACKAGE_JSON = { name: 'dummy', version: '1.0.0' }
6161
const netlifyConfig = { build: {} }
6262

6363
describe('preBuild()', () => {
64-
test('fail build if the app has static html export in npm script', async () => {
65-
await expect(
66-
plugin.onPreBuild({
67-
netlifyConfig: { build: { command: 'npm run build' } },
68-
packageJson: { ...DUMMY_PACKAGE_JSON, scripts: { build: 'next export' } },
69-
utils,
70-
constants: { FUNCTIONS_SRC: 'out_functions' },
71-
}),
72-
).rejects.toThrow(
73-
`Static HTML export Next.js projects do not require this plugin. Check your project's build command for 'next export'.`,
74-
)
64+
test('create next.config.js with correct target if file does not exist', async () => {
65+
await plugin.onPreBuild({
66+
netlifyConfig,
67+
packageJson: DUMMY_PACKAGE_JSON,
68+
utils,
69+
constants: { FUNCTIONS_SRC: 'out_functions' },
70+
})
71+
72+
expect(await pathExists('next.config.js')).toBeTruthy()
7573
})
7674

77-
test('do not fail build if the app has next export in an unused script', async () => {
78-
await expect(
79-
plugin.onPreBuild({
80-
netlifyConfig,
81-
packageJson: { ...DUMMY_PACKAGE_JSON, scripts: { export: 'next export' } },
82-
utils,
83-
constants: {},
84-
}),
85-
).resolves
75+
test('do nothing if the app has static html export in npm script', async () => {
76+
await plugin.onPreBuild({
77+
netlifyConfig: { build: { command: 'npm run build' } },
78+
packageJson: { ...DUMMY_PACKAGE_JSON, scripts: { build: 'next export' } },
79+
utils,
80+
constants: { FUNCTIONS_SRC: 'out_functions' },
81+
})
82+
83+
expect(await pathExists('next.config.js')).toBeFalsy()
8684
})
8785

88-
test('fail build if the app has static html export in toml/ntl config', async () => {
89-
await expect(
90-
plugin.onPreBuild({
91-
netlifyConfig: { build: { command: 'next build && next export' } },
92-
packageJson: DUMMY_PACKAGE_JSON,
93-
utils,
94-
constants: { FUNCTIONS_SRC: 'out_functions' },
95-
}),
96-
).rejects.toThrow(
97-
`Static HTML export Next.js projects do not require this plugin. Check your project's build command for 'next export'.`,
98-
)
86+
test('run plugin if the app has next export in an unused script', async () => {
87+
await plugin.onPreBuild({
88+
netlifyConfig,
89+
packageJson: { ...DUMMY_PACKAGE_JSON, scripts: { export: 'next export' } },
90+
utils,
91+
constants: {},
92+
})
93+
94+
expect(await pathExists('next.config.js')).toBeTruthy()
9995
})
10096

101-
test('fail build if app has next-on-netlify installed', async () => {
97+
test('do nothing if app has static html export in toml/ntl config', async () => {
98+
await plugin.onPreBuild({
99+
netlifyConfig: { build: { command: 'next build && next export' } },
100+
packageJson: DUMMY_PACKAGE_JSON,
101+
utils,
102+
constants: { FUNCTIONS_SRC: 'out_functions' },
103+
})
104+
105+
expect(await pathExists('next.config.js')).toBeFalsy()
106+
})
107+
108+
test('do nothing if app has next-on-netlify installed', async () => {
102109
const packageJson = {
103110
dependencies: { 'next-on-netlify': '123' },
104111
}
105-
await expect(
106-
plugin.onPreBuild({
107-
netlifyConfig,
108-
packageJson,
109-
utils,
110-
}),
111-
).rejects.toThrow(
112-
`This plugin does not support sites that manually use next-on-netlify. Uninstall next-on-netlify as a dependency to resolve.`,
113-
)
112+
await plugin.onPreBuild({
113+
netlifyConfig,
114+
packageJson,
115+
utils,
116+
})
117+
118+
expect(await pathExists('next.config.js')).toBeFalsy()
114119
})
115120

116-
test('fail build if app has next-on-netlify postbuild script', async () => {
121+
test('do nothing if app has next-on-netlify postbuild script', async () => {
117122
const packageJson = {
118123
scripts: { postbuild: 'next-on-netlify' },
119124
}
120-
await expect(
121-
plugin.onPreBuild({
122-
netlifyConfig,
123-
packageJson,
124-
utils,
125-
}),
126-
).rejects.toThrow(
127-
`This plugin does not support sites that manually use next-on-netlify. Uninstall next-on-netlify as a dependency to resolve.`,
128-
)
125+
await plugin.onPreBuild({
126+
netlifyConfig,
127+
packageJson,
128+
utils,
129+
})
130+
131+
expect(await pathExists('next.config.js')).toBeFalsy()
129132
})
130133

131134
test('fail build if the app has no package.json', async () => {
@@ -138,42 +141,48 @@ describe('preBuild()', () => {
138141
}),
139142
).rejects.toThrow(`Could not find a package.json for this project`)
140143
})
144+
})
141145

142-
test('create next.config.js with correct target if file does not exist', async () => {
143-
await plugin.onPreBuild({
146+
describe('onBuild()', () => {
147+
test('does not run onBuild if using next-on-netlify', async () => {
148+
const packageJson = {
149+
scripts: { postbuild: 'next-on-netlify' },
150+
}
151+
await useFixture('publish_copy_files')
152+
await moveNextDist()
153+
const PUBLISH_DIR = 'publish'
154+
await plugin.onBuild({
144155
netlifyConfig,
145-
packageJson: DUMMY_PACKAGE_JSON,
146-
utils,
147-
constants: { FUNCTIONS_SRC: 'out_functions' },
156+
packageJson,
157+
constants: {},
148158
})
149159

150-
expect(await pathExists('next.config.js')).toBeTruthy()
160+
expect(await pathExists(`${PUBLISH_DIR}/index.html`)).toBeFalsy()
151161
})
152162

153163
test.each(['invalid_next_config', 'deep_invalid_next_config'])(
154-
`fail build if the app's next config has an invalid target`,
164+
`do nothing if the app's next config has an invalid target`,
155165
async (fixtureName) => {
156166
await useFixture(fixtureName)
157-
await expect(
158-
plugin.onPreBuild({
159-
netlifyConfig,
160-
packageJson: DUMMY_PACKAGE_JSON,
161-
utils,
162-
constants: { FUNCTIONS_SRC: 'out_functions' },
163-
}),
164-
).rejects.toThrow(
165-
`Your next.config.js must set the "target" property to one of: serverless, experimental-serverless-trace`,
166-
)
167+
const PUBLISH_DIR = 'publish'
168+
await plugin.onBuild({
169+
netlifyConfig,
170+
packageJson: DUMMY_PACKAGE_JSON,
171+
utils,
172+
constants: { FUNCTIONS_SRC: 'out_functions' },
173+
})
174+
175+
expect(await pathExists(`${PUBLISH_DIR}/index.html`)).toBeFalsy()
167176
},
168177
)
169-
})
170178

171-
describe('onBuild()', () => {
172179
test('copy files to the publish directory', async () => {
173180
await useFixture('publish_copy_files')
174181
await moveNextDist()
175182
const PUBLISH_DIR = 'publish'
176183
await plugin.onBuild({
184+
netlifyConfig,
185+
packageJson: DUMMY_PACKAGE_JSON,
177186
constants: {
178187
PUBLISH_DIR,
179188
FUNCTIONS_SRC: 'functions',
@@ -191,6 +200,8 @@ describe('onBuild()', () => {
191200
await useFixture('functions_copy_files')
192201
await moveNextDist()
193202
await plugin.onBuild({
203+
netlifyConfig,
204+
packageJson: DUMMY_PACKAGE_JSON,
194205
constants: {
195206
FUNCTIONS_SRC,
196207
PUBLISH_DIR: '.',

0 commit comments

Comments
 (0)