Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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 (e) {
return failBuild('Error loading your next.config.js.')
}
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