Use custom Babel loader to avoid using separate Babel copies for loader and loader options (vercel#4417)

This resolves the

> .value is not a valid Plugin property

error showing up for people in vercel#4227

cc @timneutkens

Author: loganfsmyth

package.json

@@ -65,7 +65,7 @@
     "@babel/template": "7.0.0-beta.42",
     "ansi-html": "0.0.7",
     "babel-core": "7.0.0-bridge.0",
-    "babel-loader": "8.0.0-beta.2",
+    "babel-loader": "8.0.0-beta.3",
     "babel-plugin-react-require": "3.0.0",
     "babel-plugin-transform-react-remove-prop-types": "0.4.13",
     "case-sensitive-paths-webpack-plugin": "2.1.1",

server/build/loaders/next-babel-loader.js

@@ -0,0 +1,48 @@
+import babelLoader from 'babel-loader'
+
+module.exports = babelLoader.custom(babel => {
+  const presetItem = babel.createConfigItem(require('../babel/preset'), {type: 'preset'})
+  const hotLoaderItem = babel.createConfigItem(require('react-hot-loader/babel'), {type: 'plugin'})
+  const reactJsxSourceItem = babel.createConfigItem(require('@babel/plugin-transform-react-jsx-source'), {type: 'plugin'})
+
+  const configs = new Set()
+
+  return {
+    customOptions (opts) {
+      const custom = {
+        isServer: opts.isServer,
+        dev: opts.dev
+      }
+      const loader = Object.assign({
+        cacheDirectory: true
+      }, opts)
+      delete loader.isServer
+      delete loader.dev
+
+      return { loader, custom }
+    },
+    config (cfg, {customOptions: {isServer, dev}}) {
+      const options = Object.assign({}, cfg.options)
+      if (cfg.hasFilesystemConfig()) {
+        for (const file of [cfg.babelrc, cfg.config]) {
+          if (file && !configs.has(file)) {
+            configs.add(file)
+            console.log(`> Using external babel configuration`)
+            console.log(`> Location: "${file}"`)
+          }
+        }
+      } else {
+        // Add our default preset if the no "babelrc" found.
+        options.presets = [...options.presets, presetItem]
+      }
+
+      options.plugins = [
+        ...options.plugins,
+        dev && !isServer && hotLoaderItem,
+        dev && reactJsxSourceItem
+      ].filter(Boolean)
+
+      return options
+    }
+  }
+})

server/build/webpack.js

@@ -5,7 +5,6 @@ import UglifyJSPlugin from 'uglifyjs-webpack-plugin'
 import CaseSensitivePathPlugin from 'case-sensitive-paths-webpack-plugin'
 import WriteFilePlugin from 'write-file-webpack-plugin'
 import FriendlyErrorsWebpackPlugin from 'friendly-errors-webpack-plugin'
-import {loadPartialConfig, createConfigItem} from '@babel/core'
 import {getPages} from './webpack/utils'
 import PagesPlugin from './plugins/pages-plugin'
 import NextJsSsrImportPlugin from './plugins/nextjs-ssr-import'
@@ -14,10 +13,6 @@ import UnlinkFilePlugin from './plugins/unlink-file-plugin'
 import PagesManifestPlugin from './plugins/pages-manifest-plugin'
 import BuildManifestPlugin from './plugins/build-manifest-plugin'
 
-const presetItem = createConfigItem(require('./babel/preset'), {type: 'preset'})
-const hotLoaderItem = createConfigItem(require('react-hot-loader/babel'), {type: 'plugin'})
-const reactJsxSourceItem = createConfigItem(require('@babel/plugin-transform-react-jsx-source'), {type: 'plugin'})
-
 const nextDir = path.join(__dirname, '..', '..', '..')
 const nextNodeModulesDir = path.join(nextDir, 'node_modules')
 const nextPagesDir = path.join(nextDir, 'pages')
@@ -30,37 +25,6 @@ const interpolateNames = new Map(defaultPages.map((p) => {
   return [path.join(nextPagesDir, p), `dist/bundles/pages/${p}`]
 }))
 
-function babelConfig (dir, {isServer, dev}) {
-  const mainBabelOptions = {
-    cacheDirectory: true,
-    presets: [],
-    plugins: [
-      dev && !isServer && hotLoaderItem,
-      dev && reactJsxSourceItem
-    ].filter(Boolean)
-  }
-
-  const filename = path.join(dir, 'filename.js')
-  const externalBabelConfig = loadPartialConfig({ babelrc: true, filename })
-  if (externalBabelConfig && externalBabelConfig.babelrc) {
-    // Log it out once
-    if (!isServer) {
-      console.log(`> Using external babel configuration`)
-      console.log(`> Location: "${externalBabelConfig.babelrc}"`)
-    }
-    mainBabelOptions.babelrc = true
-  } else {
-    mainBabelOptions.babelrc = false
-  }
-
-  // Add our default preset if the no "babelrc" found.
-  if (!mainBabelOptions.babelrc) {
-    mainBabelOptions.presets.push(presetItem)
-  }
-
-  return mainBabelOptions
-}
-
 function externalsConfig (dir, isServer) {
   const externals = []
 
@@ -96,12 +60,10 @@ function externalsConfig (dir, isServer) {
 }
 
 export default async function getBaseWebpackConfig (dir, {dev = false, isServer = false, buildId, config}) {
-  const babelLoaderOptions = babelConfig(dir, {dev, isServer})
-
   const defaultLoaders = {
     babel: {
-      loader: 'babel-loader',
-      options: babelLoaderOptions
+      loader: 'next-babel-loader',
+      options: {dev, isServer}
     }
   }
 

test/integration/babel/.babelrc

@@ -0,0 +1,6 @@
+{
+  "presets": [
+    "next/babel",
+    "@babel/preset-flow"
+  ]
+}

test/integration/babel/pages/index.js

@@ -0,0 +1,10 @@
+// This page is written in flowtype to test Babel's functionality
+import * as React from 'react'
+
+type Props = {}
+
+export default class MyComponent extends React.Component<Props> {
+  render () {
+    return <div id='text'>Test Babel</div>
+  }
+}

test/integration/babel/test/.babelrc

@@ -0,0 +1,9 @@
+{
+  "presets": [
+    ["next/babel", {
+      "preset-env": {
+        "modules": "commonjs"
+      }
+    }]
+  ]
+}

test/integration/babel/test/index.test.js

@@ -0,0 +1,31 @@
+/* global jasmine, describe, beforeAll, afterAll */
+
+import { join } from 'path'
+import {
+  renderViaHTTP,
+  fetchViaHTTP,
+  findPort,
+  launchApp,
+  killApp
+} from 'next-test-utils'
+
+// test suits
+import rendering from './rendering'
+
+const context = {}
+jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 5
+
+describe('Babel', () => {
+  beforeAll(async () => {
+    context.appPort = await findPort()
+    context.server = await launchApp(join(__dirname, '../'), context.appPort, true)
+
+    // pre-build all pages at the start
+    await Promise.all([
+      renderViaHTTP(context.appPort, '/')
+    ])
+  })
+  afterAll(() => killApp(context.server))
+
+  rendering(context, 'Rendering via HTTP', (p, q) => renderViaHTTP(context.appPort, p, q), (p, q) => fetchViaHTTP(context.appPort, p, q))
+})

test/integration/babel/test/rendering.js

@@ -0,0 +1,17 @@
+/* global describe, it, expect */
+
+import cheerio from 'cheerio'
+
+export default function ({ app }, suiteName, render, fetch) {
+  async function get$ (path, query) {
+    const html = await render(path, query)
+    return cheerio.load(html)
+  }
+
+  describe(suiteName, () => {
+    it('Should compile a page with flowtype correctly', async () => {
+      const $ = await get$('/')
+      expect($('#text').text()).toBe('Test Babel')
+    })
+  })
+}

test/integration/basic/test/hmr.js

@@ -1,6 +1,6 @@
 /* global describe, it, expect */
 import webdriver from 'next-webdriver'
-import { readFileSync, writeFileSync, renameSync } from 'fs'
+import { readFileSync, writeFileSync, renameSync, existsSync } from 'fs'
 import { join } from 'path'
 import { waitFor, check } from 'next-test-utils'
 import cheerio from 'cheerio'
@@ -9,33 +9,39 @@ export default (context, renderViaHTTP) => {
   describe('Hot Module Reloading', () => {
     describe('delete a page and add it back', () => {
       it('should load the page properly', async () => {
-        const browser = await webdriver(context.appPort, '/hmr/contact')
-        const text = await browser
-          .elementByCss('p').text()
-        expect(text).toBe('This is the contact page.')
-
         const contactPagePath = join(__dirname, '../', 'pages', 'hmr', 'contact.js')
         const newContactPagePath = join(__dirname, '../', 'pages', 'hmr', '_contact.js')
 
-        // Rename the file to mimic a deleted page
-        renameSync(contactPagePath, newContactPagePath)
+        try {
+          const browser = await webdriver(context.appPort, '/hmr/contact')
+          const text = await browser
+            .elementByCss('p').text()
+          expect(text).toBe('This is the contact page.')
 
-        // wait until the 404 page comes
-        await check(
-          () => browser.elementByCss('body').text(),
-          /This page could not be found/
-        )
+          // Rename the file to mimic a deleted page
+          renameSync(contactPagePath, newContactPagePath)
 
-        // Rename the file back to the original filename
-        renameSync(newContactPagePath, contactPagePath)
+          // wait until the 404 page comes
+          await check(
+            () => browser.elementByCss('body').text(),
+            /(This page could not be found|ENOENT)/
+          )
 
-        // wait until the page comes back
-        await check(
-          () => browser.elementByCss('body').text(),
-          /This is the contact page/
-        )
+          // Rename the file back to the original filename
+          renameSync(newContactPagePath, contactPagePath)
 
-        browser.close()
+          // wait until the page comes back
+          await check(
+            () => browser.elementByCss('body').text(),
+            /This is the contact page/
+          )
+
+          browser.close()
+        } finally {
+          if (existsSync(newContactPagePath)) {
+            renameSync(newContactPagePath, contactPagePath)
+          }
+        }
       })
     })
 

yarn.lock

@@ -1171,13 +1171,14 @@ babel-jest@21.2.0, babel-jest@^21.2.0:
     babel-plugin-istanbul "^4.0.0"
     babel-preset-jest "^21.2.0"
 
-babel-loader@8.0.0-beta.2:
-  version "8.0.0-beta.2"
-  resolved "https://registry.yarnpkg.com/babel-loader/-/babel-loader-8.0.0-beta.2.tgz#4d5b67c964dc8c9cba866fd13d6b90df3acf8723"
+babel-loader@8.0.0-beta.3:
+  version "8.0.0-beta.3"
+  resolved "https://registry.yarnpkg.com/babel-loader/-/babel-loader-8.0.0-beta.3.tgz#49efeea6e8058d5af860a18a6de88b8c1450645b"
   dependencies:
     find-cache-dir "^1.0.0"
     loader-utils "^1.0.2"
     mkdirp "^0.5.1"
+    util.promisify "^1.0.0"
 
 babel-messages@^6.23.0:
   version "6.23.0"
@@ -2544,7 +2545,7 @@ error-stack-parser@^2.0.0:
   dependencies:
     stackframe "^1.0.3"
 
-es-abstract@^1.7.0:
+es-abstract@^1.5.1, es-abstract@^1.7.0:
   version "1.11.0"
   resolved "https://registry.yarnpkg.com/es-abstract/-/es-abstract-1.11.0.tgz#cce87d518f0496893b1a30cd8461835535480681"
   dependencies:
@@ -5341,6 +5342,13 @@ object.assign@^4.0.4:
     has-symbols "^1.0.0"
     object-keys "^1.0.11"
 
+object.getownpropertydescriptors@^2.0.3:
+  version "2.0.3"
+  resolved "https://registry.yarnpkg.com/object.getownpropertydescriptors/-/object.getownpropertydescriptors-2.0.3.tgz#8758c846f5b407adab0f236e0986f14b051caa16"
+  dependencies:
+    define-properties "^1.1.2"
+    es-abstract "^1.5.1"
+
 object.omit@^2.0.0:
   version "2.0.1"
   resolved "https://registry.yarnpkg.com/object.omit/-/object.omit-2.0.1.tgz#1a9c744829f39dbb858c76ca3579ae2a54ebd1fa"
@@ -7439,6 +7447,13 @@ util-deprecate@^1.0.2, util-deprecate@~1.0.1:
   version "1.0.2"
   resolved "https://registry.yarnpkg.com/util-deprecate/-/util-deprecate-1.0.2.tgz#450d4dc9fa70de732762fbd2d4a28981419a0ccf"
 
+util.promisify@^1.0.0:
+  version "1.0.0"
+  resolved "https://registry.yarnpkg.com/util.promisify/-/util.promisify-1.0.0.tgz#440f7165a459c9a16dc145eb8e72f35687097030"
+  dependencies:
+    define-properties "^1.1.2"
+    object.getownpropertydescriptors "^2.0.3"
+
 util@0.10.3, util@^0.10.3:
   version "0.10.3"
   resolved "https://registry.yarnpkg.com/util/-/util-0.10.3.tgz#7afb1afe50805246489e3db7fe0ed379336ac0f9"