Skip to content

Commit

Permalink
Use global transform for babelify (#22927)
Browse files Browse the repository at this point in the history
* Attempt to use babelify.global.

* Oops, no ES6 in build-system/.

* Simplify some code.

* Set ignore for babelify in karma.conf.js.

* Add comment.

* Fix comment.
  • Loading branch information
William Chou committed Jun 20, 2019
1 parent ecf86b9 commit 6093a86
Show file tree
Hide file tree
Showing 6 changed files with 32 additions and 48 deletions.
2 changes: 1 addition & 1 deletion build-system/compile/compile.js
Expand Up @@ -238,7 +238,7 @@ function compile(entryModuleFilenames, outputDir, outputFilename, options) {
'node_modules/web-animations-js/web-animations.install.js',
'node_modules/web-activities/activity-ports.js',
'node_modules/@ampproject/animations/dist/animations.mjs',
'node_modules/@ampproject/worker-dom/dist/amp.main.mjs',
'node_modules/@ampproject/worker-dom/dist/amp/main.mjs',
'node_modules/document-register-element/build/' +
'document-register-element.patched.js',
// 'node_modules/core-js/modules/**.js',
Expand Down
5 changes: 3 additions & 2 deletions build-system/tasks/dep-check.js
Expand Up @@ -29,6 +29,7 @@ const source = require('vinyl-source-stream');
const through = require('through2');
const {createCtrlcHandler, exitCtrlcHandler} = require('../ctrlcHandler');
const {css} = require('./css');
const {devDependencies} = require('./helpers');
const {isTravisBuild} = require('../travis');

const root = process.cwd();
Expand Down Expand Up @@ -207,9 +208,9 @@ function getGraph(entryModule) {
// we're not running browserify twice on travis.
const bundler = browserify(entryModule, {debug: true}).transform(babelify, {
compact: false,
// Transform files in node_modules since deps use ES6 export.
// https://github.com/babel/babelify#why-arent-files-in-node_modules-being-transformed
// Transform "node_modules/", but ignore devDependencies.
global: true,
ignore: devDependencies(),
});

bundler.pipeline.get('deps').push(
Expand Down
18 changes: 17 additions & 1 deletion build-system/tasks/helpers.js
Expand Up @@ -404,6 +404,17 @@ function finishBundle(srcFilename, destDir, destFilename, options) {
}
}

/**
* Returns array of relative paths to "devDependencies" defined in package.json.
* @return {!Array<string>}
*/
function devDependencies() {
const file = fs.readFileSync('package.json', 'utf8');
const packageJson = JSON.parse(file);
const devDependencies = Object.keys(packageJson['devDependencies']);
return devDependencies.map(p => `./node_modules/${p}`);
}

/**
* Transforms a given JavaScript file entry point with browserify, and watches
* it for changes (if required).
Expand All @@ -422,7 +433,11 @@ function compileUnminifiedJs(srcDir, srcFilename, destDir, options) {
let bundler = browserify({
entries: entryPoint,
debug: true,
}).transform(babelify);
}).transform(babelify, {
// Transform "node_modules/", but ignore devDependencies.
global: true,
ignore: devDependencies(),
});

if (options.watch) {
bundler = watchify(bundler);
Expand Down Expand Up @@ -751,6 +766,7 @@ module.exports = {
compileAllUnminifiedTargets,
compileJs,
compileTs,
devDependencies,
enableLocalTesting,
endBuildStep,
hostname,
Expand Down
11 changes: 10 additions & 1 deletion build-system/tasks/karma.conf.js
Expand Up @@ -15,6 +15,7 @@
*/
'use strict';

const {devDependencies} = require('./helpers');
const {gitCommitterEmail} = require('../git');
const {isTravisBuild, travisJobNumber} = require('../travis');

Expand Down Expand Up @@ -68,7 +69,15 @@ module.exports = {
debug: true,
basedir: __dirname + '/../../',
transform: [
['babelify', {'global': isTravisBuild(), 'sourceMapsAbsolute': true}],
[
'babelify',
{
// Transform "node_modules/", but ignore devDependencies (on Travis).
'global': isTravisBuild(),
'ignore': devDependencies(),
'sourceMapsAbsolute': true,
},
],
],
// Prevent "cannot find module" errors on Travis. See #14166.
bundleDelay: isTravisBuild() ? 5000 : 1200,
Expand Down
42 changes: 0 additions & 42 deletions build-system/tasks/update-packages.js
Expand Up @@ -118,46 +118,6 @@ function patchRegisterElement() {
);
}

/**
* transformEs6Packages() doesn't work on subdirectories of dist/,
* and I can't figure out which option enables it.
* Just move "dist/amp/main.mjs" to "dist/amp.main.mjs" as a workaround.
*/
function moveWorkerDom() {
const dir = 'node_modules/@ampproject/worker-dom/dist/';
fs.copyFileSync(dir + 'amp/main.mjs', dir + 'amp.main.mjs');
}

/**
* Makes sure ES6 packages in node_modules that are used by the runtime will be
* transformed by babelify. The list of packages is dynamically generated by
* reading the `dependencies` section of the package.json in the project root.
* This is a no-op if transforms are already enabled for a package.
* See https://github.com/babel/babelify#why-arent-files-in-node_modules-being-transformed
*/
function transformEs6Packages() {
const rootPackageJsonFile = 'package.json';
const rootPackageJsonContents = fs.readFileSync(rootPackageJsonFile, 'utf8');
const rootPackageJson = JSON.parse(rootPackageJsonContents);
const es6Packages = Object.keys(rootPackageJson['dependencies']);
es6Packages.forEach(es6Package => {
const packageJsonFile = 'node_modules/' + es6Package + '/package.json';
const packageJsonContents = fs.readFileSync(packageJsonFile, 'utf8');
const packageJson = JSON.parse(packageJsonContents);
if (!packageJson['browserify']) {
packageJson['browserify'] = {'transform': ['babelify']};
const updatedPackageJson = JSON.stringify(packageJson, null, 2);
fs.writeFileSync(packageJsonFile, updatedPackageJson, 'utf8');
if (!isTravisBuild()) {
log(
colors.green('Enabled ES6 transforms for runtime dependency'),
colors.cyan(es6Package)
);
}
}
});
}

/**
* Installs custom lint rules from build-system/eslint-rules to node_modules.
*/
Expand Down Expand Up @@ -226,8 +186,6 @@ async function updatePackages() {
}
patchWebAnimations();
patchRegisterElement();
moveWorkerDom();
transformEs6Packages();
}

module.exports = {
Expand Down
2 changes: 1 addition & 1 deletion extensions/amp-script/0.1/amp-script.js
Expand Up @@ -35,7 +35,7 @@ import {
} from '../../../src/service/origin-experiments-impl';
import {isExperimentOn} from '../../../src/experiments';
import {rewriteAttributeValue} from '../../../src/url-rewrite';
import {upgrade} from '@ampproject/worker-dom/dist/amp.main.mjs';
import {upgrade} from '@ampproject/worker-dom/dist/amp/main.mjs';

/** @const {string} */
const TAG = 'amp-script';
Expand Down

0 comments on commit 6093a86

Please sign in to comment.