Skip to content

fail build if plugin cant load next.config.js #99

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 2 commits into from
Feb 17, 2021
Merged
Show file tree
Hide file tree
Changes from all 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
2 changes: 1 addition & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ test/sample/netlify/functions
test/sample/my-publish-dir
test/sample/.next
test/sample/.netlify
next.config.js
/next.config.js

# Logs
logs
Expand Down
4 changes: 2 additions & 2 deletions helpers/doesNotNeedPlugin.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ const isStaticExportProject = require('./isStaticExportProject')
const doesSiteUseNextOnNetlify = require('./doesSiteUseNextOnNetlify')
const hasCorrectNextConfig = require('./hasCorrectNextConfig')

const doesNotNeedPlugin = async ({ netlifyConfig, packageJson }) => {
const doesNotNeedPlugin = async ({ netlifyConfig, packageJson, utils }) => {

Choose a reason for hiding this comment

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

[dust] should we pass only failBuild

Copy link
Author

Choose a reason for hiding this comment

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

i was trying to keep it on the same level as netlifyConfig and packageJson, as in they're all things that get passed to the different event handlers (i.e onBuild) so i think it's ok? will keep in mind tho going forward

const { build } = netlifyConfig
const { name, scripts = {} } = packageJson
const nextConfigPath = await findUp('next.config.js')

return (
isStaticExportProject({ build, scripts }) ||
doesSiteUseNextOnNetlify({ packageJson }) ||
!hasCorrectNextConfig(nextConfigPath)
!hasCorrectNextConfig({ nextConfigPath, failBuild: utils.build.failBuild })
)
}

Expand Down
9 changes: 7 additions & 2 deletions helpers/hasCorrectNextConfig.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
const path = require('path')

// Checks if site has the correct next.cofig.js
const hasCorrectNextConfig = (nextConfigPath) => {
const hasCorrectNextConfig = ({ nextConfigPath, failBuild }) => {
// In the plugin's case, no config is valid because we'll make it ourselves
if (nextConfigPath === undefined) return true

Expand All @@ -12,7 +12,12 @@ const hasCorrectNextConfig = (nextConfigPath) => {

// 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('.'))
let nextConfig
try {
nextConfig = loadConfig(PHASE_PRODUCTION_BUILD, path.resolve('.'))
} catch (error) {
return failBuild('Error loading your next.config.js.', { error })
}
const isValidTarget = acceptableTargets.includes(nextConfig.target)
if (!isValidTarget) {
console.log(
Expand Down
11 changes: 8 additions & 3 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ module.exports = {
return failBuild('Could not find a package.json for this project')
}

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

Expand All @@ -39,8 +39,13 @@ module.exports = {
await pWriteFile('next.config.js', nextConfig)
}
},
async onBuild({ netlifyConfig, packageJson, constants: { PUBLISH_DIR, FUNCTIONS_SRC = DEFAULT_FUNCTIONS_SRC } }) {
if (await doesNotNeedPlugin({ netlifyConfig, packageJson })) {
async onBuild({
netlifyConfig,
packageJson,
constants: { PUBLISH_DIR, FUNCTIONS_SRC = DEFAULT_FUNCTIONS_SRC },
utils,
}) {
if (await doesNotNeedPlugin({ netlifyConfig, packageJson, utils })) {
return
}

Expand Down
6 changes: 6 additions & 0 deletions test/fixtures/broken_next_config/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
module.exports = {
target: "serverless",
i18n: {
locales: ["en", "fr"
}
};
17 changes: 17 additions & 0 deletions test/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,19 @@ describe('preBuild()', () => {
}),
).rejects.toThrow(`Could not find a package.json for this project`)
})

test('fail build if the app cant load the next.config.js', async () => {
await useFixture('broken_next_config')

await expect(
plugin.onPreBuild({
netlifyConfig,
packageJson: DUMMY_PACKAGE_JSON,
utils,
constants: { FUNCTIONS_SRC: 'out_functions' },
}),
).rejects.toThrow(`Error loading your next.config.js.`)
})
})

describe('onBuild()', () => {
Expand All @@ -155,6 +168,7 @@ describe('onBuild()', () => {
netlifyConfig,
packageJson,
constants: {},
utils,
})

expect(await pathExists(`${PUBLISH_DIR}/index.html`)).toBeFalsy()
Expand All @@ -170,6 +184,7 @@ describe('onBuild()', () => {
packageJson: DUMMY_PACKAGE_JSON,
utils,
constants: { FUNCTIONS_SRC: 'out_functions' },
utils,
})

expect(await pathExists(`${PUBLISH_DIR}/index.html`)).toBeFalsy()
Expand All @@ -187,6 +202,7 @@ describe('onBuild()', () => {
PUBLISH_DIR,
FUNCTIONS_SRC: 'functions',
},
utils,
})

expect(await pathExists(`${PUBLISH_DIR}/_redirects`)).toBeTruthy()
Expand All @@ -206,6 +222,7 @@ describe('onBuild()', () => {
FUNCTIONS_SRC,
PUBLISH_DIR: '.',
},
utils,
})

expect(await pathExists(`${resolvedFunctions}/next_api_test/next_api_test.js`)).toBeTruthy()
Expand Down