De-experimentalize the public/ folder (#8661)

* Remove experimental.publicDirectory config * Error when public is set as an output dir * Remove experimental.publicDirectory checks * Update publicFiles checking in next build
vercel · Oct 6, 2019 · e2d713f · e2d713f
1 parent 1a02dfd
commit e2d713f
Show file tree

Hide file tree

Showing 15 changed files with 97 additions and 49 deletions.
diff --git a/errors/can-not-output-to-public.md b/errors/can-not-output-to-public.md
@@ -0,0 +1,15 @@
+# Can't Override Next Props
+
+#### Why This Error Occurred
+
+Either you set `distDir` to `public` in your `next.config.js` or during `next export` you tried to export to the `public` directory.
+
+This is not allowed due to `public` being a special folder in Next.js used to serve static assets.
+
+#### Possible Ways to Fix It
+
+Use a different `distDir` or export to a different folder.
+
+### Useful Links
+
+- [Static file serving docs](https://nextjs.org/docs#static-file-serving-eg-images)
diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts
@@ -95,6 +95,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
   const publicDir = path.join(dir, 'public')
   const pagesDir = findPagesDir(dir)
   let publicFiles: string[] = []
+  let hasPublicDir = false
 
   let backgroundWork: (Promise<any> | undefined)[] = []
   backgroundWork.push(
@@ -104,7 +105,12 @@ export default async function build(dir: string, conf = null): Promise<void> {
 
   await verifyTypeScriptSetup(dir, pagesDir)
 
-  if (config.experimental.publicDirectory) {
+  try {
+    await fsStat(publicDir)
+    hasPublicDir = true
+  } catch (_) {}
+
+  if (hasPublicDir) {
     publicFiles = await recursiveReadDir(publicDir, /.*/)
   }
 
@@ -131,10 +137,12 @@ export default async function build(dir: string, conf = null): Promise<void> {
   const entrypoints = createEntrypoints(mappedPages, target, buildId, config)
   const conflictingPublicFiles: string[] = []
 
-  try {
-    await fsStat(path.join(publicDir, '_next'))
-    throw new Error(PUBLIC_DIR_MIDDLEWARE_CONFLICT)
-  } catch (err) {}
+  if (hasPublicDir) {
+    try {
+      await fsStat(path.join(publicDir, '_next'))
+      throw new Error(PUBLIC_DIR_MIDDLEWARE_CONFLICT)
+    } catch (err) {}
+  }
 
   for (let file of publicFiles) {
     file = file

diff --git a/packages/next/export/index.ts b/packages/next/export/index.ts
@@ -153,6 +153,13 @@ export default async function(
 
   // Initialize the output directory
   const outDir = options.outdir
+
+  if (outDir === join(dir, 'public')) {
+    throw new Error(
+      `The 'public' directory is reserved in Next.js and can not be used as the export out directory. https://err.sh/zeit/next.js/can-not-output-to-public`
+    )
+  }
+
   await recursiveDelete(join(outDir))
   await mkdirp(join(outDir, '_next', buildId))
 
@@ -243,12 +250,7 @@ export default async function(
 
   const publicDir = join(dir, CLIENT_PUBLIC_FILES_PATH)
   // Copy public directory
-  if (
-    !options.buildExport &&
-    nextConfig.experimental &&
-    nextConfig.experimental.publicDirectory &&
-    existsSync(publicDir)
-  ) {
+  if (!options.buildExport && existsSync(publicDir)) {
     log('  copying "public" directory')
     await recursiveCopy(publicDir, outDir, {
       filter(path) {

diff --git a/packages/next/next-server/server/config.ts b/packages/next/next-server/server/config.ts
@@ -77,6 +77,12 @@ function assignDefaults(userConfig: { [key: string]: any }) {
       experimentalWarning()
     }
 
+    if (key === 'distDir' && userConfig[key] === 'public') {
+      throw new Error(
+        `The 'public' directory is reserved in Next.js and can not be set as the 'distDir'. https://err.sh/zeit/next.js/can-not-output-to-public`
+      )
+    }
+
     const maybeObject = userConfig[key]
     if (!!maybeObject && maybeObject.constructor === Object) {
       userConfig[key] = {

diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts
@@ -294,10 +294,7 @@ export default class Server {
       },
     ]
 
-    if (
-      this.nextConfig.experimental.publicDirectory &&
-      fs.existsSync(this.publicDir)
-    ) {
+    if (fs.existsSync(this.publicDir)) {
       routes.push(...this.generatePublicRoutes())
     }
 

diff --git a/packages/next/server/next-dev-server.ts b/packages/next/server/next-dev-server.ts
@@ -229,24 +229,21 @@ export default class DevServer extends Server {
 
     // check for a public file, throwing error if there's a
     // conflicting page
-    if (this.nextConfig.experimental.publicDirectory) {
-      if (await this.hasPublicFile(pathname!)) {
-        const pageFile = await findPageFile(
-          this.pagesDir!,
+    if (await this.hasPublicFile(pathname!)) {
+      const pageFile = await findPageFile(
+        this.pagesDir!,
+        normalizePagePath(pathname!),
+        this.nextConfig.pageExtensions
+      )
 
-          normalizePagePath(pathname!),
-          this.nextConfig.pageExtensions
+      if (pageFile) {
+        const err = new Error(
+          `A conflicting public file and page file was found for path ${pathname} https://err.sh/zeit/next.js/conflicting-public-file-page`
         )
-
-        if (pageFile) {
-          const err = new Error(
-            `A conflicting public file and page file was found for path ${pathname} https://err.sh/zeit/next.js/conflicting-public-file-page`
-          )
-          res.statusCode = 500
-          return this.renderError(err, req, res, pathname!, {})
-        }
-        return this.servePublic(req, res, pathname!)
+        res.statusCode = 500
+        return this.renderError(err, req, res, pathname!, {})
       }
+      return this.servePublic(req, res, pathname!)
     }
 
     const { finished } = (await this.hotReloader!.run(req, res, parsedUrl)) || {

diff --git a/test/integration/basic/next.config.js b/test/integration/basic/next.config.js
@@ -4,9 +4,6 @@ module.exports = {
     // Make sure entries are not getting disposed.
     maxInactiveAge: 1000 * 60 * 60
   },
-  experimental: {
-    publicDirectory: true
-  },
   webpack (config) {
     config.module.rules.push({
       test: /pages[\\/]hmr[\\/]about/,

diff --git a/test/integration/config-experimental-warning/test/index.test.js b/test/integration/config-experimental-warning/test/index.test.js
@@ -39,7 +39,7 @@ describe('Promise in next config', () => {
       module.exports = {
         target: 'server',
         experimental: {
-          publicDirectory: true
+          something: true
         }
       }
     `)
@@ -52,7 +52,7 @@ describe('Promise in next config', () => {
       module.exports = (phase) => ({
         target: 'server',
         experimental: {
-          publicDirectory: true
+          something: true
         }
       })
     `)

diff --git a/test/integration/errors-on-output-to-public/pages/index.js b/test/integration/errors-on-output-to-public/pages/index.js
@@ -0,0 +1 @@
+export default () => 'hi'
diff --git a/test/integration/errors-on-output-to-public/test/index.test.js b/test/integration/errors-on-output-to-public/test/index.test.js
@@ -0,0 +1,37 @@
+/* eslint-env jest */
+/* global jasmine */
+import path from 'path'
+import fs from 'fs-extra'
+import { nextBuild, nextExport } from 'next-test-utils'
+
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 1
+const appDir = path.join(__dirname, '..')
+const nextConfig = path.join(appDir, 'next.config.js')
+
+describe('Errors on output to public', () => {
+  it('Throws error when `distDir` is set to public', async () => {
+    await fs.writeFile(nextConfig, `module.exports = { distDir: 'public' }`)
+    const results = await nextBuild(appDir, [], { stdout: true, stderr: true })
+    expect(results.stdout + results.stderr).toMatch(
+      /The 'public' directory is reserved in Next.js and can not be set as/
+    )
+    await fs.remove(nextConfig)
+  })
+
+  it('Throws error when export out dir is public', async () => {
+    await fs.remove(nextConfig)
+    await nextBuild(appDir)
+    const outdir = path.join(appDir, 'public')
+    const results = await nextExport(
+      appDir,
+      { outdir },
+      {
+        stdout: true,
+        stderr: true
+      }
+    )
+    expect(results.stdout + results.stderr).toMatch(
+      /The 'public' directory is reserved in Next.js and can not be used as/
+    )
+  })
+})
diff --git a/test/integration/export/next.config.js b/test/integration/export/next.config.js
@@ -9,9 +9,6 @@ module.exports = phase => {
     serverRuntimeConfig: {
       bar: 'bar'
     },
-    experimental: {
-      publicDirectory: true
-    },
     exportTrailingSlash: true,
     exportPathMap: function () {
       return {

diff --git a/test/integration/production/next.config.js b/test/integration/production/next.config.js
@@ -2,8 +2,5 @@ module.exports = {
   onDemandEntries: {
     // Make sure entries are not getting disposed.
     maxInactiveAge: 1000 * 60 * 60
-  },
-  experimental: {
-    publicDirectory: true
   }
 }
diff --git a/test/integration/serverless-trace/next.config.js b/test/integration/serverless-trace/next.config.js
@@ -4,9 +4,6 @@ module.exports = {
     // Make sure entries are not getting disposed.
     maxInactiveAge: 1000 * 60 * 60
   },
-  experimental: {
-    publicDirectory: true
-  },
   // make sure error isn't thrown from empty publicRuntimeConfig
   publicRuntimeConfig: {}
 }
diff --git a/test/integration/serverless/next.config.js b/test/integration/serverless/next.config.js
@@ -4,9 +4,6 @@ module.exports = {
     // Make sure entries are not getting disposed.
     maxInactiveAge: 1000 * 60 * 60
   },
-  experimental: {
-    publicDirectory: true
-  },
   // make sure error isn't thrown from empty publicRuntimeConfig
   publicRuntimeConfig: {}
 }
diff --git a/test/lib/next-test-utils.js b/test/lib/next-test-utils.js
@@ -180,8 +180,8 @@ export function nextBuild (dir, args = [], opts = {}) {
   return runNextCommand(['build', dir, ...args], opts)
 }
 
-export function nextExport (dir, { outdir }) {
-  return runNextCommand(['export', dir, '--outdir', outdir])
+export function nextExport (dir, { outdir }, opts = {}) {
+  return runNextCommand(['export', dir, '--outdir', outdir], opts)
 }
 
 export function nextStart (dir, port, opts = {}) {