From 75c3c5e12a99c8fdb57475ef7e852bdc422f1908 Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Wed, 13 Jul 2022 14:44:39 -0400 Subject: [PATCH 1/2] fix: add better error messaging for builds --- package.json | 1 + plugin/src/helpers/verification.ts | 18 +++++-- test/helpers/verification.spec.ts | 80 ++++++++++++++++++++++++++++++ 3 files changed, 94 insertions(+), 5 deletions(-) create mode 100644 test/helpers/verification.spec.ts diff --git a/package.json b/package.json index 8bf0582cce..e0abd6eef0 100644 --- a/package.json +++ b/package.json @@ -82,6 +82,7 @@ "jest": { "testMatch": [ "**/test/**/*.js", + "**/test/**/*.ts", "!**/test/fixtures/**", "!**/test/sample/**" ], diff --git a/plugin/src/helpers/verification.ts b/plugin/src/helpers/verification.ts index cf8ecc4086..3d66eea151 100644 --- a/plugin/src/helpers/verification.ts +++ b/plugin/src/helpers/verification.ts @@ -62,10 +62,18 @@ export const checkNextSiteHasBuilt = ({ failBuild: FailBuild }): void | never => { if (!existsSync(path.join(publish, 'BUILD_ID'))) { - const outWarning = - path.basename(publish) === 'out' - ? `Your publish directory is set to "out", but in most cases it should be ".next".` - : `In most cases it should be set to ".next", unless you have chosen a custom "distDir" in your Next config.` + let outWarning + + if (path.basename(publish) === 'out') { + outWarning = `Your publish directory is set to "out", but in most cases it should be ".next".` + } else if (path.basename(publish) !== '.next' && existsSync(path.join('.next', 'BUILD_ID'))) { + outWarning = outdent` + However, a '.next' directory was found with a production build. + Consider changing your 'publish' directory to '.next' + ` + } else { + outWarning = `In most cases it should be set to ".next", unless you have chosen a custom "distDir" in your Next config.` + } return failBuild(outdent` The directory "${publish}" does not contain a Next.js production build. Perhaps the build command was not run, or you specified the wrong publish directory. @@ -74,7 +82,7 @@ export const checkNextSiteHasBuilt = ({ `) } if (existsSync(path.join(publish, 'export-detail.json'))) { - failBuild(outdent` + return failBuild(outdent` Detected that "next export" was run, but site is incorrectly publishing the ".next" directory. The publish directory should be set to "out", and you should set the environment variable NETLIFY_NEXT_PLUGIN_SKIP to "true". `) diff --git a/test/helpers/verification.spec.ts b/test/helpers/verification.spec.ts new file mode 100644 index 0000000000..9b6662241b --- /dev/null +++ b/test/helpers/verification.spec.ts @@ -0,0 +1,80 @@ +import { checkNextSiteHasBuilt } from '../../plugin/src/helpers/verification' +import { outdent } from 'outdent' + +import type { NetlifyPluginUtils } from '@netlify/build' +type FailBuild = NetlifyPluginUtils['build']['failBuild'] + +jest.mock('fs', () => { + return { + existsSync: jest.fn() + } +}) + +describe('checkNextSiteHasBuilt', () => { + let failBuildMock + let { existsSync } = require('fs') + + beforeEach(() => { + failBuildMock = (jest.fn() as unknown) as FailBuild + }) + + afterEach(() => { + jest.clearAllMocks() + jest.resetAllMocks() + }) + + it('returns error message about incorrectly publishing the ".next" directory when "next export" was run', () => { + existsSync.mockReturnValue(true) + + const expectedFailureMessage = outdent` + Detected that "next export" was run, but site is incorrectly publishing the ".next" directory. + The publish directory should be set to "out", and you should set the environment variable NETLIFY_NEXT_PLUGIN_SKIP to "true". + ` + + checkNextSiteHasBuilt({ publish: '.next', failBuild: failBuildMock }) + + expect(failBuildMock).toHaveBeenCalledWith(expectedFailureMessage) + }) + + it('returns error message prompt to change publish directory to ".next"', () => { + // False for not initially finding the specified 'publish' directory, + // True for successfully finding a '.next' directory with a production build + existsSync.mockReturnValueOnce(false).mockReturnValueOnce(true) + + const expectedFailureMessage = outdent` + The directory "someCustomDir" does not contain a Next.js production build. Perhaps the build command was not run, or you specified the wrong publish directory. + However, a '.next' directory was found with a production build. + Consider changing your 'publish' directory to '.next' + If you are using "next export" then you should set the environment variable NETLIFY_NEXT_PLUGIN_SKIP to "true". + ` + + checkNextSiteHasBuilt({ publish: 'someCustomDir', failBuild: failBuildMock }) + + expect(failBuildMock).toHaveBeenCalledWith(expectedFailureMessage) + }) + + it('returns error message prompt when publish directory is set to "out"', () => { + existsSync.mockReturnValue(false) + + const expectedFailureMessage = outdent` + The directory "out" does not contain a Next.js production build. Perhaps the build command was not run, or you specified the wrong publish directory. + Your publish directory is set to "out", but in most cases it should be ".next". + If you are using "next export" then you should set the environment variable NETLIFY_NEXT_PLUGIN_SKIP to "true". + ` + checkNextSiteHasBuilt({ publish: 'out', failBuild: failBuildMock }) + + expect(failBuildMock).toHaveBeenCalledWith(expectedFailureMessage) + }) + + it('returns default error message when production build was not found', () => { + existsSync.mockReturnValue(false) + const expectedFailureMessage = outdent` + The directory ".next" does not contain a Next.js production build. Perhaps the build command was not run, or you specified the wrong publish directory. + In most cases it should be set to ".next", unless you have chosen a custom "distDir" in your Next config. + If you are using "next export" then you should set the environment variable NETLIFY_NEXT_PLUGIN_SKIP to "true". + ` + checkNextSiteHasBuilt({ publish: '.next', failBuild: failBuildMock }) + + expect(failBuildMock).toHaveBeenCalledWith(expectedFailureMessage) + }) +}) From 5655f3fddca164f13d944c117ef8003105bbb940 Mon Sep 17 00:00:00 2001 From: Erica Pisani Date: Wed, 13 Jul 2022 14:59:43 -0400 Subject: [PATCH 2/2] test: cleanup --- test/helpers/verification.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/helpers/verification.spec.ts b/test/helpers/verification.spec.ts index 9b6662241b..ec06262bb3 100644 --- a/test/helpers/verification.spec.ts +++ b/test/helpers/verification.spec.ts @@ -12,7 +12,7 @@ jest.mock('fs', () => { describe('checkNextSiteHasBuilt', () => { let failBuildMock - let { existsSync } = require('fs') + const { existsSync } = require('fs') beforeEach(() => { failBuildMock = (jest.fn() as unknown) as FailBuild