Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

create babel-preset and babel-register modules #13973

Merged
merged 6 commits into from
Sep 20, 2017
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Gruntfile.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
require('./src/optimize/babel/register');
require('./src/babel-register');

module.exports = function (grunt) {
// set the config once before calling load-grunt-config
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -89,13 +89,13 @@
"ansicolors": "0.3.2",
"autoprefixer": "6.5.4",
"autoprefixer-loader": "2.0.0",
"babel-cli": "6.18.0",
"babel-core": "6.21.0",
"babel-jest": "21.0.0",
"babel-loader": "6.2.10",
"babel-plugin-add-module-exports": "0.2.1",
"babel-plugin-transform-async-generator-functions": "6.24.1",
"babel-plugin-transform-class-properties": "6.24.1",
"babel-plugin-transform-define": "1.3.0",
"babel-plugin-transform-object-rest-spread": "6.23.0",
"babel-polyfill": "6.20.0",
"babel-preset-env": "1.4.0",
Expand Down
2 changes: 1 addition & 1 deletion scripts/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ node scripts/{{script name}} --help

This directory is excluded from the build and tools within it should help users discover their capabilities. Each script in this directory must:

- include the `../src/optimize/babel/register` module to bootstrap babel
- require `src/babel-register` to bootstrap babel
- call out to source code that is in the `src` directory
- react to the `--help` flag
- run everywhere OR check and fail fast when a required OS or toolchain is not available
2 changes: 1 addition & 1 deletion scripts/docs.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
require('../src/optimize/babel/register');
require('../src/babel-register');
require('../src/docs/cli');
2 changes: 1 addition & 1 deletion scripts/es_archiver.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
require('../src/optimize/babel/register');
require('../src/babel-register');
require('../src/es_archiver/cli');
2 changes: 1 addition & 1 deletion scripts/functional_test_runner.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
require('../src/optimize/babel/register');
require('../src/babel-register');
require('../src/functional_test_runner/cli');
3 changes: 1 addition & 2 deletions scripts/jest.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,5 @@
//
// See all cli options in https://facebook.github.io/jest/docs/cli.html

require('../src/optimize/babel/register');
require('../src/babel-register');
require('../src/jest/cli');

2 changes: 1 addition & 1 deletion scripts/mocha.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
require('../src/optimize/babel/register');
require('../src/babel-register');
require('../test/scripts/run_mocha');
13 changes: 13 additions & 0 deletions src/babel-preset/common.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
module.exports = {
presets: [
require.resolve('babel-preset-react'),
],
plugins: [
require.resolve('babel-plugin-add-module-exports'),
// stage 3
require.resolve('babel-plugin-transform-async-generator-functions'),
require.resolve('babel-plugin-transform-object-rest-spread'),
// stage 2
require.resolve('babel-plugin-transform-class-properties'),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know this is copy-pasted, just thought I'd make sure we add a comment here: the class properties proposal was merged with the private fields proposal into the "class fields" proposal, which is currently stage 3. I don't see a specific Babel transform for that proposal yet, though, but there is babel/proposals#12 which tracks this proposal. Maybe add a link to that Babel proposal?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, that's a good point. I imagine once there is a solid class fields proposal it will go into babel-preset-env and collide with this transform, but I shall leave a comment nonetheless

],
};
22 changes: 22 additions & 0 deletions src/babel-preset/node.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,22 @@
module.exports = {
presets: [
[
require.resolve('babel-preset-env'),
{
targets: {
node: 'current',
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe add a comment above this line about what current means? (I had to check because I didn't know if it was "latest" or something like that). Maybe:

// Transforms for the Node.js version that's used to run Babel

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any thoughts on pulling this from package.json instead? I mention that only as a single-source-of-truth thing, as I honestly can't think of a situation where current would be a problem.

Copy link
Contributor Author

@spalger spalger Sep 18, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I request that we keep "current" because I take advantage of the fact that I can run Kibana server in node v8 and babel-preset-env doesn't transpile async/await and makes debugging so much easier

},
useBuiltIns: true,
},
],
require('./common'),
],
plugins: [
[
require.resolve('babel-plugin-transform-define'),
{
'typeof BUILT_WITH_BABEL': 'true'
}
]
]
};
18 changes: 18 additions & 0 deletions src/babel-preset/webpack.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
module.exports = {
presets: [
[
require.resolve('babel-preset-env'),
{
targets: {
browsers: [
'last 2 versions',
'> 5%',
'Safari 7', // for PhantomJS support
],
},
useBuiltIns: true,
},
],
require('./common'),
]
};
5 changes: 5 additions & 0 deletions src/babel-register/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
// register and polyfill need to happen in this
// order and in separate files. Checkout each file
// for a much more detailed explaination
require('./register');
require('./polyfill');
10 changes: 10 additions & 0 deletions src/babel-register/polyfill.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
// `babel-preset-env` looks for and rewrites the following import
// statement into a list of import statements based on the polyfills
// necessary for our target environment (the current version of node)
// but since it does that during compilation, `import 'babel-polyfill'`
// must be in a file that is loaded with `require()` AFTER `babel-register`
// is configured.
//
// This is why we have this single statement in it's own file and require
// it from ./index.js
import 'babel-polyfill';
29 changes: 29 additions & 0 deletions src/babel-register/register.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
const { resolve } = require('path');

// this must happen before `require('babel-register')` and can't be changed
// once the module has been loaded
if (!process.env.BABEL_CACHE_PATH) {
process.env.BABEL_CACHE_PATH = resolve(__dirname, '../../optimize/.babelcache.json');
}

// paths that babel-register should ignore
const ignore = [
/[\\\/](node_modules|bower_components)[\\\/]/,
];

if (typeof BUILT_WITH_BABEL !== 'undefined') {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nitpick: Could this be process.env.BUILT_WITH_BABEL instead? It just feels "unnatural" to have typeof something return true.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Haha, yeah, fair 👍

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, now that I think about it, I kind of like that it looks unnatural, since it totally is. But I'll switch over to process.env.BUILT_WITH_BABEL and leave a comment that the environment variable should never be set... idk...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I can just replace typeof BUILT_WITH_BABEL with the string "boolean" or something? I just don't want people to get the idea that they need to set some environment variable, and I really don't want to read an environment variable here just because... or maybe if (global.__BUILT_WITH_BABEL__)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea with the global, definitely a better choice than my process.env idea 👍

// in "production" builds we define `typeof BUILT_WITH_BABEL` as `true`
// which indicates that all of the code in the `src` directory is already
// built and can be ignored by `babel-register`
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, not sure I follow the logic and struggled with the build, so I'll ask here instead: Why do we still run babel-register in prod? Can't it just run normal require (e.g. if we moved the require('babel-register') into an if instead of having it always run)?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We pre-transpile the server code but plugins are still transpiled in prod.

ignore.push(resolve(__dirname, '../../src'));
}

// modifies all future calls to require() to automatically
// compile the required source with babel
require('babel-register')({
ignore,
babelrc: false,
presets: [
require.resolve('../babel-preset/node')
],
});
2 changes: 1 addition & 1 deletion src/cli/index.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
require('../optimize/babel/register');
require('../babel-register');
require('./cli');
2 changes: 1 addition & 1 deletion src/cli_plugin/index.js
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
require('../optimize/babel/register');
require('../babel-register');
require('./cli');
9 changes: 5 additions & 4 deletions src/jest/babelTransform.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const babelJest = require('babel-jest');
const options = require('../optimize/babel/options');

const babelOptions = options.node;

module.exports = babelJest.createTransformer(babelOptions);
module.exports = babelJest.createTransformer({
presets: [
require.resolve('../babel-preset/node')
]
});
8 changes: 0 additions & 8 deletions src/optimize/babel/.eslintrc

This file was deleted.

61 changes: 0 additions & 61 deletions src/optimize/babel/helpers.js

This file was deleted.

20 changes: 0 additions & 20 deletions src/optimize/babel/options.build.js

This file was deleted.

24 changes: 0 additions & 24 deletions src/optimize/babel/options.js

This file was deleted.

2 changes: 0 additions & 2 deletions src/optimize/babel/polyfills.js

This file was deleted.

3 changes: 0 additions & 3 deletions src/optimize/babel/register.js

This file was deleted.

7 changes: 5 additions & 2 deletions src/optimize/base_optimizer.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import UglifyJsPlugin from 'webpack/lib/optimize/UglifyJsPlugin';
import { defaults, transform } from 'lodash';

import { fromRoot } from '../utils';
import babelOptions from './babel/options';
import pkg from '../../package.json';
import { setLoaderQueryParam, makeLoaderString } from './loaders';

Expand Down Expand Up @@ -133,7 +132,11 @@ export default class BaseOptimizer {
test: /\.js$/,
exclude: babelExclude.concat(this.env.noParse),
loader: 'babel-loader',
query: babelOptions.webpack
query: {
presets: [
require.resolve('../babel-preset/webpack')
]
}
},
],
postLoaders: this.env.postLoaders || [],
Expand Down
14 changes: 0 additions & 14 deletions tasks/build/babel_options.js

This file was deleted.

1 change: 0 additions & 1 deletion tasks/build/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ module.exports = function (grunt) {
'copy:devSource',
'clean:devSourceForTestbed',
'babel:build',
'_build:babelOptions',
'_build:plugins',
'_build:data',
'_build:verifyTranslations',
Expand Down
8 changes: 5 additions & 3 deletions tasks/config/babel.js
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import babelOptions from '../../src/optimize/babel/options';

module.exports = {
build: {
options: babelOptions.node,
options: {
presets: [
require.resolve('../../src/babel-preset/node')
]
},
src: [
'build/kibana/**/*.js',
'!**/public/**',
Expand Down
2 changes: 1 addition & 1 deletion test/mocha_setup.js
Original file line number Diff line number Diff line change
@@ -1 +1 @@
require('../src/optimize/babel/register');
require('../src/babel-register');