Fix: Pre-rendered index.js page should not cause error

When we copy the pre-rendered pages to `out/`, we do a check to make
sure that the file does not already exist. This is to make sure that we
don't silently overwrite one of the files copied from `public/`, for
example.

But for `index.js` files, Netlify always creates two page entries, one
routing to `/` and one routing to `/index`. So in this case, the first
copying of `index.js` to `out/` goes fine. The second copying fails
because `index.js` already exists.

We fix this by first filtering our list of pages for unique pages and
then performing the copy operation.

Fixes: #2 (comment)

Author: FinnWoelm

README.md

@@ -136,3 +136,4 @@ next-on-netlify has only been tested on NextJS version 9 and above.
 🙌 Big "thank you" to the following people for their contributions:
 - [@spencewood](https://github.com/spencewood)
 - [@alxhghs](https://github.com/alxhghs)
+- [@gamliela](https://github.com/gamliela)

next-on-netlify.js

@@ -69,7 +69,12 @@ copySync(
 
 // 2.3 SSR Setup: Copy SSR pages to functions/_next/pages
 // From there, they can be served by our nextRouter Netlify function.
-ssrPages.forEach(({ file }) => {
+
+// Get unique set of JS files to copy. index.js might be present twice, for
+// example.
+const jsFilesToCopy = [...new Set(ssrPages.map(page => page.file))]
+
+jsFilesToCopy.forEach(file => {
   console.log(`   > ${file}`)
 
   copySync(
@@ -114,7 +119,13 @@ if(existsSync(PUBLIC_PATH)) {
 // These are static, so they do not need to be handled by our nextRouter.
 console.log(`\x1b[1m🔥 Writing pre-rendered HTML pages to ${OUTPUT_PATH}\x1b[22m`)
 
-htmlPages.forEach(({ file }) => {
+// Get unique set of html files to copy.
+// index.html might be present twice, for example. We need to filter one out,
+// otherwise our second attempt to copy the file would crash, since it already
+// exists.
+const htmlFilesToCopy = [...new Set(htmlPages.map(page => page.file))]
+
+htmlFilesToCopy.forEach(file => {
   console.log(`   > ${file}`)
 
   // The path to the file, relative to the pages directory

tests/fixtures/pages-with-prerendered-index/index.js

@@ -0,0 +1,5 @@
+const PreRenderedPage = () => (
+  <p>This page is pre-rendered.</p>
+)
+
+export default PreRenderedPage

tests/fixtures/pages-with-prerendered-index/shows/index.js

@@ -0,0 +1,5 @@
+const PreRenderedPage = () => (
+  <p>This page is pre-rendered.</p>
+)
+
+export default PreRenderedPage

tests/preRenderedIndexPages.test.js

@@ -0,0 +1,114 @@
+// Test that next-on-netlify does not crash when pre-rendering index.js file.
+// See: https://github.com/FinnWoelm/next-on-netlify/issues/2#issuecomment-636415494
+
+const { parse, join } = require('path')
+const { copySync, emptyDirSync, existsSync,
+        readdirSync, readFileSync, readJsonSync } = require('fs-extra')
+const npmRunBuild = require("./helpers/npmRunBuild")
+
+// The name of this test file (without extension)
+const FILENAME = parse(__filename).name
+
+// The directory which will be used for testing.
+// We simulate a NextJS app within that directory, with pages, and a
+// package.json file.
+const PROJECT_PATH = join(__dirname, "builds", FILENAME)
+
+// The directory that contains the fixtures, such as NextJS pages,
+// NextJS config, and package.json
+const FIXTURE_PATH = join(__dirname, "fixtures")
+
+// Capture the output of `npm run build` to verify successful build
+let BUILD_OUTPUT
+
+beforeAll(
+  async () => {
+    // Clear project directory
+    emptyDirSync(PROJECT_PATH)
+    emptyDirSync(join(PROJECT_PATH, "pages"))
+
+    // Copy NextJS pages and config
+    copySync(
+      join(FIXTURE_PATH, "pages-with-prerendered-index"),
+      join(PROJECT_PATH, "pages")
+    )
+    copySync(
+      join(FIXTURE_PATH, "next.config.js"),
+      join(PROJECT_PATH, "next.config.js")
+    )
+
+    // Copy package.json
+    copySync(
+      join(FIXTURE_PATH, "package.json"),
+      join(PROJECT_PATH, "package.json")
+    )
+
+    // Invoke `npm run build`: Build Next and run next-on-netlify
+    const { stdout } = await npmRunBuild({ directory: PROJECT_PATH })
+    BUILD_OUTPUT = stdout
+  },
+  // time out after 30 seconds
+  30 * 1000
+)
+
+describe('Next', () => {
+  test('builds successfully', () => {
+    // NextJS output
+    expect(BUILD_OUTPUT).toMatch("Creating an optimized production build...")
+    expect(BUILD_OUTPUT).toMatch("Automatically optimizing pages...")
+    expect(BUILD_OUTPUT).toMatch("First Load JS shared by all")
+
+    // Next on Netlify output
+    expect(BUILD_OUTPUT).toMatch("Next on Netlify")
+    expect(BUILD_OUTPUT).toMatch("Preparing Netlify Function for SSR pages: functions/nextRouter")
+    expect(BUILD_OUTPUT).toMatch("Writing pre-rendered HTML pages to out/")
+    expect(BUILD_OUTPUT).toMatch("Copying static NextJS assets to out/")
+    expect(BUILD_OUTPUT).toMatch("Setting up redirects")
+    expect(BUILD_OUTPUT).toMatch("Success! All done!")
+  })
+})
+
+describe('Static Pages', () => {
+  test('copies static pages to output directory', () => {
+    const OUTPUT_PATH = join(PROJECT_PATH, "out")
+
+    expect(existsSync(join(OUTPUT_PATH, "index.html"))).toBe(true)
+    expect(existsSync(join(OUTPUT_PATH, "shows.html"))).toBe(true)
+  })
+
+  test('copies static assets to out/_next/ directory', () => {
+    const dirs = readdirSync(join(PROJECT_PATH, "out", "_next", "static"))
+
+    expect(dirs.length).toBe(3)
+    expect(dirs).toContain("chunks")
+    expect(dirs).toContain("runtime")
+  })
+})
+
+describe('404 Page', () => {
+  test('copies 404.html to output directory', () => {
+    const OUTPUT_PATH = join(PROJECT_PATH, "out")
+
+    expect(existsSync(join(OUTPUT_PATH, "404.html"))).toBe(true)
+  })
+
+  // This is required for 404.html to work on netlify-dev
+  test('copies 404.html to directory root', () => {
+    expect(existsSync(join(PROJECT_PATH, "404.html"))).toBe(true)
+  })
+})
+
+describe('Routing',() => {
+  test('creates Netlify redirects', async () => {
+    // Read _redirects file
+    const contents = readFileSync(join(PROJECT_PATH, "out", "_redirects"))
+
+    // Convert contents into an array, each line being one element
+    const redirects = contents.toString().split("\n")
+
+    // Check that routes are present
+    expect(redirects).toContain("/  /index.html  200")
+    expect(redirects).toContain("/index  /index.html  200")
+    expect(redirects).toContain("/shows  /shows.html  200")
+  })
+})