Upgrade Babel 6 to Babel 7 (major) #4050

timneutkens · 2018-03-23T23:00:52Z

Babel 7 does schema validation of .babelrc and doesn't support "babelrc": false.

Meaning that if you want similar behaviour you should just do

{
  "presets": ["next/babel"]
}

I'm not sure what the benefit of babelrc: false is over above 🤔

cc @arunoda

Also, this is pending a release of taskr: lukeed/taskr#305

And styled-jsx seems to work, but can be improved vercel/styled-jsx#362

timneutkens · 2018-03-23T23:09:04Z

Also implemented @xtuc's suggestion here: #3428 (comment)

based on the implemented version here: babel/babel#7472

it'll be a bit faster after babel/babel#7588 lands.

The new API seems to work really well @loganfsmyth (as feedback)

loganfsmyth · 2018-03-23T23:17:20Z

server/build/babel/find-config.js

-  const configList = buildConfigChain(options).filter(i => i.loc !== 'base')
-
-  return configList[0]
+  return loadPartialConfig({ babelrc: true, filename })


I'd recommend against passing babelrc: true, since you pass filename that is already the default, and it's possible we may change the meaning of true still in the beta to be broader than you'd actually want.

loganfsmyth · 2018-03-23T23:21:32Z

server/build/babel/preset.js

@@ -36,18 +36,18 @@ const plugins = envPlugins[process.env.NODE_ENV] || envPlugins['development']

 module.exports = (context, opts = {}) => ({
  presets: [
-    [require.resolve('babel-preset-env'), {
+    [require.resolve('@babel/preset-env'), {


Out of curiosity, is there a specific motivation for using paths here instead of calling require() on its own?

Since you're passing absolute paths it safe to do this, but it does seem more roundabout since Babel has to process the string into a path.

loganfsmyth · 2018-03-23T23:25:30Z

server/build/babel/find-config.js

@@ -1,16 +1,9 @@
 import { join } from 'path'
-import buildConfigChain from 'babel-core/lib/transformation/file/options/build-config-chain'
+import {loadPartialConfig} from '@babel/core'

 export default function findBabelConfig (dir) {
  // We need to provide a location of a filename inside the `dir`.
  // For the name of the file, we could be provide anything.
  const filename = join(dir, 'filename.js')


This is a little concerning. We've got path-based config processing now from babel/babel#7091, so by hard-coding a filename you're essentially making that feature unusable.

If this is done as a performance optimization, I'd caution against worrying too much there. Babel's config processing has a lot more caching now and should be very fast, even when loadPartialConfig is called for every single file.

If this is done as a performance optimization, I'd caution against worrying too much there. Babel's config processing has a lot more caching now and should be very fast, even when loadPartialConfig is called for every single file.

Basically it's not to pre-optimize, but simply to output which path babel is using when starting the Next process, to be transparent to the user.

Also, it's used to include the next/babel preset automatically when no babelrc is available.

loganfsmyth · 2018-03-23T23:29:56Z

server/build/webpack.js

-    // That's why we need to do this.
-    const { options } = externalBabelConfig
-    mainBabelOptions.babelrc = options.babelrc !== false
+    mainBabelOptions.babelrc = true
  } else {
    mainBabelOptions.babelrc = false
  }


I'd recommend using the createConfigItem utility on the mainBabelOptions.presets.push(require.resolve('./babel/preset')) line below this, rather than just passing the string, otherwise this will end up doing a lot more work.

loganfsmyth · 2018-03-23T23:30:38Z

server/build/webpack.js

@@ -33,17 +33,13 @@ function babelConfig (dir, {isServer, dev}) {
  }


Same as below about babel.createConfigItem for the hot loader listing above this.

loganfsmyth · 2018-03-23T23:37:54Z

server/build/webpack.js

-    // That's why we need to do this.
-    const { options } = externalBabelConfig
-    mainBabelOptions.babelrc = options.babelrc !== false
+    mainBabelOptions.babelrc = true


If you change up this logic to just be per-filename like I'd recommend, you shouldn't need to assign this to anything. e.g.

const presetItem = createConfigItem(require.resolve('./babel/preset')); const hotLoaderItem = createConfigItem(require.resolve('react-hot-loader/babel')); function getBabelConfig(filename, {isServer, dev}) { const config = loadPartialConfig({ filename, plugins: [ dev && !isServer && hotLoaderItem ].filter(Boolean), }); if (!config) return null; let options = config.options; if (config.babelrc) { if (!isServer) { // Maybe just have some extra logic to avoid printing this multiple times? console.log(`> Using external babel configuration`) console.log(`> Location: "${config.babelrc}"`) } } else { options.presets.push(presetItem); } return options; }

Hmmm in that case babel-loader needs to be changed to support a function that has the file path as argument and resolves to a babel config right 🤔

I like the idea though 👍

Ahh, I'd totally forgotten that this is used via babel-loader. That makes this a lot clearer.

Hmmm in that case babel-loader needs to be changed to support a function that has the file path as argument and resolves to a babel config right

Maybe that's something we should explore. I hadn't thought about it but it may well make sense.

Unrelated to this specific comment, I'd remove the mainBabelOptions.babelrc = true line, since searching for the config file is already the default.

I've removed the line 👍

@loganfsmyth created an issue here: babel/babel-loader#598

loganfsmyth · 2018-03-23T23:39:15Z

I had more comments than I originally expected! Hope they help though :D

loganfsmyth · 2018-03-24T17:21:46Z

.babelrc

  ],
  "env": {
    "test": {
      "presets": [
-        "es2015",
+        "@babel/preset-es2015",


Obviously an existing issue, but since you're using preset-env already, I'd be very surprised if this was necessary.

Good catch, we can solve it differently 👍

loganfsmyth · 2018-03-24T23:37:15Z

.babelrc

  ],
  "env": {
    "test": {
      "presets": [
-        "es2015",
-        "./babel"
+        ["./babel", {


So we're on the same page, I'm a little confused about the expectations you have for this vs the presets you have in the top-level presets array. As it is, this test env will just run both sets of presets. What specifically is the goal of the test env here? It'll add a few extra plugins around styling and react it looks like, but it also doubles up on a ton of the stuff you're already specifying in the top level config.

Right, this should probably be broken out into .babelrc in the test directory. Will do that after this PR lands. Since it's not directly related to babel 7 👍

arunoda

Since babel is removing the babelrc:false support, there's no point in having it.

BTW: It'd be amazing if you could separate out lint fixes to another PR.

loganfsmyth · 2018-03-27T15:33:38Z

@arunoda

Since babel is removing the babelrc:false support, there's no point in having it.

It's an option that you can programmatically pass to Babel, but it's not an option that is available in config files, even in Babel 6. It would have been a no-op where Next was using it in the .babelrc, it's just that because you were parsing the file yourselves, you could add additional custom behavior based on it. Babel 7 adds validation and now properly states that it is not expected in config files.

timneutkens · 2018-03-27T17:34:28Z

@arunoda

BTW: It'd be amazing if you could separate out lint fixes to another PR.

Unfortunately the older standard breaks because of babel 7. So I had to upgrade it. I'll try to send another PR to merge before this one 👍

timneutkens · 2018-03-27T17:39:36Z

Cherry picked the commit: #4064

# Conflicts: # yarn.lock

# Conflicts: # .babelrc # package.json # server/build/webpack.js # yarn.lock

timneutkens · 2018-04-03T12:18:02Z

Thank you very much for all the reviews @loganfsmyth very much appreciated ❤️

oliviertassinari · 2018-04-03T17:03:58Z

package.json

@@ -77,7 +78,7 @@
    "fresh": "0.5.2",
    "friendly-errors-webpack-plugin": "1.6.1",
    "glob": "7.1.2",
-    "hoist-non-react-statics": "2.5.0",
+    "hoist-non-react-statics": "2.3.1",


loganfsmyth · 2018-04-03T17:17:24Z

package.json

@@ -1,6 +1,6 @@
 {
  "name": "next",
-  "version": "6.0.0-canary.2",
+  "version": "5.0.1-canary.17",


Was this accidentally reverted?

yeah fixed already

It's release as 6.0.0-canary.3

Refactor for babel 7

bab70b1

loganfsmyth reviewed Mar 23, 2018

View reviewed changes

Require modules directly, use createConfigItem for additional config

9b848a0

loganfsmyth reviewed Mar 24, 2018

View reviewed changes

timneutkens added 2 commits March 24, 2018 20:59

Use commonjs from preset-env

df71be2

Remove babelrc = true

ecc8eb0

timneutkens mentioned this pull request Mar 24, 2018

Support for loadPartialConfig by user babel/babel-loader#598

Closed

Use babel7 version of @task/babel

541e933

loganfsmyth reviewed Mar 24, 2018

View reviewed changes

timneutkens added 6 commits March 25, 2018 15:54

Use babel-core bridge for babel-jest

98581a5

Add test for fragment syntax

bbb164b

Merge branch 'canary' of github.com:zeit/next.js into add/babel-7

41bf9a2

Temporarily disable eslint for files using class properties

eb07d7d

yarn.lock was stale

efe0bef

Upgrade standard.js

d3dadcf

arunoda approved these changes Mar 27, 2018

View reviewed changes

timneutkens mentioned this pull request Mar 27, 2018

Upgrade standard.js #4064

Merged

timneutkens added 2 commits March 27, 2018 20:40

Merge branch 'canary' of github.com:zeit/next.js into add/babel-7

90b6765

# Conflicts: # yarn.lock

Update yarn.lock

a133915

timneutkens mentioned this pull request Apr 2, 2018

Support for Babel 7 - Export a function instead of an object threepointone/markdown-in-js#18

Merged

timneutkens added 3 commits April 2, 2018 16:06

Merge branch 'canary' of github.com:zeit/next.js into add/babel-7

13eaa21

# Conflicts: # .babelrc # package.json # server/build/webpack.js # yarn.lock

Add @babel/preset-flow

809b2e8

Remove unused modules

b3d4803

timneutkens changed the title ~~Refactor for Babel 7~~ Upgrade Babel 6 to Babel 7 (major) Apr 3, 2018

timneutkens merged commit 2d8c19a into vercel:canary Apr 3, 2018

timneutkens deleted the add/babel-7 branch April 3, 2018 07:34

timneutkens mentioned this pull request Apr 3, 2018

Speed up tests #4102

Merged

oliviertassinari reviewed Apr 3, 2018

View reviewed changes

loganfsmyth reviewed Apr 3, 2018

View reviewed changes

This was referenced Apr 3, 2018

Redo #4090 #4104

Merged

[core] Upgrade Babel 6 to Babel 7 mui/material-ui#10964

Merged

lock bot locked as resolved and limited conversation to collaborators Apr 3, 2019

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Upgrade Babel 6 to Babel 7 (major) #4050

Upgrade Babel 6 to Babel 7 (major) #4050

timneutkens commented Mar 23, 2018

timneutkens commented Mar 23, 2018

loganfsmyth Mar 23, 2018

loganfsmyth Mar 23, 2018

loganfsmyth Mar 23, 2018 •

edited

Loading

timneutkens Mar 24, 2018 •

edited

Loading

loganfsmyth Mar 23, 2018

loganfsmyth Mar 23, 2018

loganfsmyth Mar 23, 2018 •

edited

Loading

timneutkens Mar 24, 2018

timneutkens Mar 24, 2018

loganfsmyth Mar 24, 2018

loganfsmyth Mar 24, 2018

timneutkens Mar 24, 2018

timneutkens Mar 24, 2018

loganfsmyth commented Mar 23, 2018

loganfsmyth Mar 24, 2018

timneutkens Mar 24, 2018

timneutkens Mar 24, 2018

loganfsmyth Mar 24, 2018

timneutkens Mar 25, 2018

arunoda left a comment

loganfsmyth commented Mar 27, 2018

timneutkens commented Mar 27, 2018 •

edited

Loading

timneutkens commented Mar 27, 2018

timneutkens commented Apr 3, 2018

oliviertassinari Apr 3, 2018

loganfsmyth Apr 3, 2018

timneutkens Apr 3, 2018

timneutkens Apr 3, 2018

		@@ -33,17 +33,13 @@ function babelConfig (dir, {isServer, dev}) {
		}

Upgrade Babel 6 to Babel 7 (major) #4050

Upgrade Babel 6 to Babel 7 (major) #4050

Conversation

timneutkens commented Mar 23, 2018

timneutkens commented Mar 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loganfsmyth Mar 23, 2018 • edited Loading

Choose a reason for hiding this comment

timneutkens Mar 24, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loganfsmyth Mar 23, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loganfsmyth commented Mar 23, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

arunoda left a comment

Choose a reason for hiding this comment

loganfsmyth commented Mar 27, 2018

timneutkens commented Mar 27, 2018 • edited Loading

timneutkens commented Mar 27, 2018

timneutkens commented Apr 3, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

loganfsmyth Mar 23, 2018 •

edited

Loading

timneutkens Mar 24, 2018 •

edited

Loading

loganfsmyth Mar 23, 2018 •

edited

Loading

timneutkens commented Mar 27, 2018 •

edited

Loading