Skip to content

Commit

Permalink
fix: more robust handling of export output (#418)
Browse files Browse the repository at this point in the history
* chore: rename fixtures

* chore: add custom dist export fixture

* fix: more robust handling of export output

* chore: ignore new dist

* chore: fix error snapshot
  • Loading branch information
ascorbic authored Apr 18, 2024
1 parent 1a0542c commit d66e30b
Show file tree
Hide file tree
Showing 16 changed files with 214 additions and 39 deletions.
1 change: 1 addition & 0 deletions .prettierignore
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package.json
edge-runtime/vendor/
deno.lock
tests/fixtures/dist-dir/cool/output
tests/fixtures/output-export-custom-dist/custom-dist
.nx
custom-dist-dir
pnpm.lock
Expand Down
5 changes: 4 additions & 1 deletion src/build/content/static.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,12 @@ export const copyStaticAssets = async (ctx: PluginContext): Promise<void> => {

export const copyStaticExport = async (ctx: PluginContext): Promise<void> => {
await tracer.withActiveSpan('copyStaticExport', async () => {
if (!ctx.exportDetail?.outDirectory) {
ctx.failBuild('Export directory not found')
}
try {
await rm(ctx.staticDir, { recursive: true, force: true })
await cp(ctx.resolveFromSiteDir('out'), ctx.staticDir, { recursive: true })
await cp(ctx.exportDetail.outDirectory, ctx.staticDir, { recursive: true })
} catch (error) {
ctx.failBuild('Failed copying static export', error)
}
Expand Down
63 changes: 61 additions & 2 deletions src/build/plugin-context.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { readFileSync } from 'node:fs'
import { existsSync, readFileSync } from 'node:fs'
import { readFile } from 'node:fs/promises'
// Here we need to actually import `resolve` from node:path as we want to resolve the paths
// eslint-disable-next-line no-restricted-imports
Expand Down Expand Up @@ -32,6 +32,11 @@ export interface RequiredServerFilesManifest {
ignore: string[]
}

export interface ExportDetail {
success: boolean
outDirectory: string
}

export class PluginContext {
utils: NetlifyPluginUtils
netlifyConfig: NetlifyPluginOptions['netlifyConfig']
Expand Down Expand Up @@ -200,6 +205,28 @@ export class PluginContext {
return JSON.parse(await readFile(join(this.publishDir, 'prerender-manifest.json'), 'utf-8'))
}

/**
* Uses various heuristics to try to find the .next dir.
* Works by looking for BUILD_ID, so requires the site to have been built
*/
findDotNext(): string | false {
for (const dir of [
// The publish directory
this.publishDir,
// In the root
resolve(DEFAULT_PUBLISH_DIR),
// The sibling of the publish directory
resolve(this.publishDir, '..', DEFAULT_PUBLISH_DIR),
// In the package dir
resolve(this.constants.PACKAGE_PATH || '', DEFAULT_PUBLISH_DIR),
]) {
if (existsSync(join(dir, 'BUILD_ID'))) {
return dir
}
}
return false
}

/**
* Get Next.js middleware config from the build output
*/
Expand All @@ -215,13 +242,45 @@ export class PluginContext {
/** Get RequiredServerFiles manifest from build output **/
get requiredServerFiles(): RequiredServerFilesManifest {
if (!this._requiredServerFiles) {
let requiredServerFilesJson = join(this.publishDir, 'required-server-files.json')

if (!existsSync(requiredServerFilesJson)) {
const dotNext = this.findDotNext()
if (dotNext) {
requiredServerFilesJson = join(dotNext, 'required-server-files.json')
}
}

this._requiredServerFiles = JSON.parse(
readFileSync(join(this.publishDir, 'required-server-files.json'), 'utf-8'),
readFileSync(requiredServerFilesJson, 'utf-8'),
) as RequiredServerFilesManifest
}
return this._requiredServerFiles
}

#exportDetail: ExportDetail | null = null

/** Get metadata when output = export */
get exportDetail(): ExportDetail | null {
if (this.buildConfig.output !== 'export') {
return null
}
if (!this.#exportDetail) {
const detailFile = join(
this.requiredServerFiles.appDir,
this.buildConfig.distDir,
'export-detail.json',
)
if (!existsSync(detailFile)) {
return null
}
try {
this.#exportDetail = JSON.parse(readFileSync(detailFile, 'utf-8'))
} catch {}
}
return this.#exportDetail
}

/** Get Next Config from build output **/
get buildConfig(): NextConfigComplete {
return this.requiredServerFiles.config
Expand Down
42 changes: 25 additions & 17 deletions src/build/verification.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { existsSync } from 'node:fs'
import { join } from 'node:path'

import { satisfies } from 'semver'

Expand All @@ -10,7 +11,7 @@ const SUPPORTED_NEXT_VERSIONS = '>=13.5.0'
export function verifyPublishDir(ctx: PluginContext) {
if (!existsSync(ctx.publishDir)) {
ctx.failBuild(
`Your publish directory was not found at: ${ctx.publishDir}, please check your build settings`,
`Your publish directory was not found at: ${ctx.publishDir}. Please check your build settings`,
)
}
// for next.js sites the publish directory should never equal the package path which should be
Expand All @@ -22,33 +23,40 @@ export function verifyPublishDir(ctx: PluginContext) {
// that directory will be above packagePath
if (ctx.publishDir === ctx.resolveFromPackagePath('')) {
ctx.failBuild(
`Your publish directory cannot be the same as the base directory of your site, please check your build settings`,
`Your publish directory cannot be the same as the base directory of your site. Please check your build settings`,
)
}
try {
// `PluginContext.buildConfig` is getter and we only test wether it throws
// `PluginContext.buildConfig` is getter and we only test whether it throws
// and don't actually need to use its value
// eslint-disable-next-line no-unused-expressions
ctx.buildConfig
} catch {
ctx.failBuild(
'Your publish directory does not contain expected Next.js build output, please check your build settings',
'Your publish directory does not contain expected Next.js build output. Please check your build settings',
)
}
if (
(ctx.buildConfig.output === 'standalone' || ctx.buildConfig.output === undefined) &&
!existsSync(ctx.standaloneRootDir)
) {
ctx.failBuild(
`Your publish directory does not contain expected Next.js build output, please make sure you are using Next.js version (${SUPPORTED_NEXT_VERSIONS})`,
)
if (ctx.buildConfig.output === 'standalone' || ctx.buildConfig.output === undefined) {
if (!existsSync(join(ctx.publishDir, 'BUILD_ID'))) {
ctx.failBuild(
'Your publish directory does not contain expected Next.js build output. Please check your build settings',
)
}
if (!existsSync(ctx.standaloneRootDir)) {
ctx.failBuild(
`Your publish directory does not contain expected Next.js build output. Please make sure you are using Next.js version (${SUPPORTED_NEXT_VERSIONS})`,
)
}
}
if (ctx.buildConfig.output === 'export' && !existsSync(ctx.resolveFromSiteDir('out'))) {
ctx.failBuild(
`Your export directory was not found at: ${ctx.resolveFromSiteDir(
'out',
)}, please check your build settings`,
)
if (ctx.buildConfig.output === 'export') {
if (!ctx.exportDetail?.success) {
ctx.failBuild(`Your export failed to build. Please check your build settings`)
}
if (!existsSync(ctx.exportDetail?.outDirectory)) {
ctx.failBuild(
`Your export directory was not found at: ${ctx.exportDetail?.outDirectory}. Please check your build settings`,
)
}
}
}

Expand Down
54 changes: 54 additions & 0 deletions tests/e2e/export.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { expect, type Locator } from '@playwright/test'
import { nextVersionSatisfies } from '../utils/next-version-helpers.mjs'
import { test } from '../utils/playwright-helpers.js'

const expectImageWasLoaded = async (locator: Locator) => {
expect(await locator.evaluate((img: HTMLImageElement) => img.naturalHeight)).toBeGreaterThan(0)
}
test('Renders the Home page correctly with output export', async ({ page, outputExport }) => {
const response = await page.goto(outputExport.url)
const headers = response?.headers() || {}

await expect(page).toHaveTitle('Simple Next App')

expect(headers['cache-status']).toBe('"Netlify Edge"; fwd=miss')

const h1 = page.locator('h1')
await expect(h1).toHaveText('Home')

await expectImageWasLoaded(page.locator('img'))
})

test('Renders the Home page correctly with output export and publish set to out', async ({
page,
ouputExportPublishOut,
}) => {
const response = await page.goto(ouputExportPublishOut.url)
const headers = response?.headers() || {}

await expect(page).toHaveTitle('Simple Next App')

expect(headers['cache-status']).toBe('"Netlify Edge"; fwd=miss')

const h1 = page.locator('h1')
await expect(h1).toHaveText('Home')

await expectImageWasLoaded(page.locator('img'))
})

test('Renders the Home page correctly with output export and custom dist dir', async ({
page,
outputExportCustomDist,
}) => {
const response = await page.goto(outputExportCustomDist.url)
const headers = response?.headers() || {}

await expect(page).toHaveTitle('Simple Next App')

expect(headers['cache-status']).toBe('"Netlify Edge"; fwd=miss')

const h1 = page.locator('h1')
await expect(h1).toHaveText('Home')

await expectImageWasLoaded(page.locator('img'))
})
14 changes: 0 additions & 14 deletions tests/e2e/simple-app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,20 +25,6 @@ test('Renders the Home page correctly', async ({ page, simple }) => {
expect(body).toBe('{"words":"hello world"}')
})

test('Renders the Home page correctly with output export', async ({ page, outputExport }) => {
const response = await page.goto(outputExport.url)
const headers = response?.headers() || {}

await expect(page).toHaveTitle('Simple Next App')

expect(headers['cache-status']).toBe('"Netlify Edge"; fwd=miss')

const h1 = page.locator('h1')
await expect(h1).toHaveText('Home')

await expectImageWasLoaded(page.locator('img'))
})

test('Renders the Home page correctly with distDir', async ({ page, distDir }) => {
await page.goto(distDir.url)

Expand Down
1 change: 1 addition & 0 deletions tests/fixtures/output-export-custom-dist/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
custom-dist
12 changes: 12 additions & 0 deletions tests/fixtures/output-export-custom-dist/app/layout.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
export const metadata = {
title: 'Simple Next App',
description: 'Description for Simple Next App',
}

export default function RootLayout({ children }) {
return (
<html lang="en">
<body>{children}</body>
</html>
)
}
8 changes: 8 additions & 0 deletions tests/fixtures/output-export-custom-dist/app/page.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
export default function Home() {
return (
<main>
<h1>Home</h1>
<img src="/squirrel.jpg" alt="a cute squirrel" width="300px" />
</main>
)
}
10 changes: 10 additions & 0 deletions tests/fixtures/output-export-custom-dist/next.config.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
/** @type {import('next').NextConfig} */
const nextConfig = {
output: 'export',
distDir: 'custom-dist',
eslint: {
ignoreDuringBuilds: true,
},
}

module.exports = nextConfig
15 changes: 15 additions & 0 deletions tests/fixtures/output-export-custom-dist/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"name": "output-export",
"version": "0.1.0",
"private": true,
"scripts": {
"postinstall": "next build",
"dev": "next dev",
"build": "next build"
},
"dependencies": {
"next": "latest",
"react": "18.2.0",
"react-dom": "18.2.0"
}
}
1 change: 1 addition & 0 deletions tests/fixtures/output-export-custom-dist/public/next.svg
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
17 changes: 13 additions & 4 deletions tests/integration/simple-app.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,14 +95,14 @@ describe('verification', () => {
test<FixtureTestContext>("Should warn if publish dir doesn't exist", async (ctx) => {
await createFixture('simple', ctx)
expect(() => runPlugin(ctx, { PUBLISH_DIR: 'no-such-directory' })).rejects.toThrowError(
/Your publish directory was not found at: \S+no-such-directory, please check your build settings/,
/Your publish directory was not found at: \S+no-such-directory. Please check your build settings/,
)
})

test<FixtureTestContext>('Should warn if publish dir is root', async (ctx) => {
await createFixture('simple', ctx)
expect(() => runPlugin(ctx, { PUBLISH_DIR: '.' })).rejects.toThrowError(
'Your publish directory cannot be the same as the base directory of your site, please check your build settings',
'Your publish directory cannot be the same as the base directory of your site. Please check your build settings',
)
})

Expand All @@ -111,16 +111,25 @@ describe('verification', () => {
expect(() =>
runPlugin(ctx, { PUBLISH_DIR: 'app/.', PACKAGE_PATH: 'app' }),
).rejects.toThrowError(
'Your publish directory cannot be the same as the base directory of your site, please check your build settings',
'Your publish directory cannot be the same as the base directory of your site. Please check your build settings',
)
})

test<FixtureTestContext>('Should warn if publish dir is not set to Next.js output directory', async (ctx) => {
await createFixture('simple', ctx)
expect(() => runPlugin(ctx, { PUBLISH_DIR: 'public' })).rejects.toThrowError(
'Your publish directory does not contain expected Next.js build output, please check your build settings',
'Your publish directory does not contain expected Next.js build output. Please check your build settings',
)
})
test<FixtureTestContext>('Should not warn if using "out" as publish dir when output is "export"', async (ctx) => {
await createFixture('output-export', ctx)
await expect(runPlugin(ctx, { PUBLISH_DIR: 'out' })).resolves.not.toThrow()
})

test<FixtureTestContext>('Should not throw when using custom distDir and output is "export', async (ctx) => {
await createFixture('output-export-custom-dist', ctx)
await expect(runPlugin(ctx, { PUBLISH_DIR: 'custom-dist' })).resolves.not.toThrow()
})
})

test<FixtureTestContext>('Should add cache-tags to prerendered app pages', async (ctx) => {
Expand Down
2 changes: 1 addition & 1 deletion tests/smoke/deploy.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ describe('version check', () => {
async () => {
// we are not able to get far enough to extract concrete next version, so this error message lack used Next.js version
await expect(selfCleaningFixtureFactories.next12_0_3()).rejects.toThrow(
/Your publish directory does not contain expected Next.js build output, please make sure you are using Next.js version \(>=13.5.0\)/,
/Your publish directory does not contain expected Next.js build output. Please make sure you are using Next.js version \(>=13.5.0\)/,
)
},
)
Expand Down
8 changes: 8 additions & 0 deletions tests/utils/create-e2e-fixture.ts
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,14 @@ async function cleanup(dest: string, deployId?: string): Promise<void> {
export const fixtureFactories = {
simple: () => createE2EFixture('simple'),
outputExport: () => createE2EFixture('output-export'),
ouputExportPublishOut: () =>
createE2EFixture('output-export', {
publishDirectory: 'out',
}),
outputExportCustomDist: () =>
createE2EFixture('output-export-custom-dist', {
publishDirectory: 'custom-dist',
}),
distDir: () =>
createE2EFixture('dist-dir', {
publishDirectory: 'cool/output',
Expand Down

0 comments on commit d66e30b

Please sign in to comment.