From 31659579eeee2c644f186540cbb78af29c7f3bb8 Mon Sep 17 00:00:00 2001 From: Logan Smyth Date: Wed, 23 May 2018 11:26:57 -0700 Subject: [PATCH] Use custom Babel loader to avoid using separate Babel copies for loader and loader options (#4417) This resolves the > .value is not a valid Plugin property error showing up for people in https://github.com/zeit/next.js/issues/4227 cc @timneutkens --- package.json | 2 +- server/build/loaders/next-babel-loader.js | 48 +++++++++++++++++++++++ server/build/webpack.js | 42 +------------------- test/integration/babel/.babelrc | 6 +++ test/integration/babel/pages/index.js | 10 +++++ test/integration/babel/test/.babelrc | 9 +++++ test/integration/babel/test/index.test.js | 31 +++++++++++++++ test/integration/babel/test/rendering.js | 17 ++++++++ test/integration/basic/test/hmr.js | 48 +++++++++++++---------- yarn.lock | 23 +++++++++-- 10 files changed, 170 insertions(+), 66 deletions(-) create mode 100644 server/build/loaders/next-babel-loader.js create mode 100644 test/integration/babel/.babelrc create mode 100644 test/integration/babel/pages/index.js create mode 100644 test/integration/babel/test/.babelrc create mode 100644 test/integration/babel/test/index.test.js create mode 100644 test/integration/babel/test/rendering.js diff --git a/package.json b/package.json index 4954a8a5ebebb..2da3dbe8bc469 100644 --- a/package.json +++ b/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", diff --git a/server/build/loaders/next-babel-loader.js b/server/build/loaders/next-babel-loader.js new file mode 100644 index 0000000000000..c468bb7c88f19 --- /dev/null +++ b/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 + } + } +}) diff --git a/server/build/webpack.js b/server/build/webpack.js index 8b231fcf53e35..43a9047a05dd5 100644 --- a/server/build/webpack.js +++ b/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} } } diff --git a/test/integration/babel/.babelrc b/test/integration/babel/.babelrc new file mode 100644 index 0000000000000..1493ad65f8636 --- /dev/null +++ b/test/integration/babel/.babelrc @@ -0,0 +1,6 @@ +{ + "presets": [ + "next/babel", + "@babel/preset-flow" + ] +} \ No newline at end of file diff --git a/test/integration/babel/pages/index.js b/test/integration/babel/pages/index.js new file mode 100644 index 0000000000000..2aff7efd2c5c7 --- /dev/null +++ b/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 { + render () { + return
Test Babel
+ } +} diff --git a/test/integration/babel/test/.babelrc b/test/integration/babel/test/.babelrc new file mode 100644 index 0000000000000..27c7237aeb802 --- /dev/null +++ b/test/integration/babel/test/.babelrc @@ -0,0 +1,9 @@ +{ + "presets": [ + ["next/babel", { + "preset-env": { + "modules": "commonjs" + } + }] + ] +} \ No newline at end of file diff --git a/test/integration/babel/test/index.test.js b/test/integration/babel/test/index.test.js new file mode 100644 index 0000000000000..dc359a9aae190 --- /dev/null +++ b/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)) +}) diff --git a/test/integration/babel/test/rendering.js b/test/integration/babel/test/rendering.js new file mode 100644 index 0000000000000..3047058b969d5 --- /dev/null +++ b/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') + }) + }) +} diff --git a/test/integration/basic/test/hmr.js b/test/integration/basic/test/hmr.js index 14f4f7d937436..12467e4b942c3 100644 --- a/test/integration/basic/test/hmr.js +++ b/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) + } + } }) }) diff --git a/yarn.lock b/yarn.lock index dc6aac5d2faab..4faff5e3b6af2 100644 --- a/yarn.lock +++ b/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" @@ -7447,6 +7455,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"