Skip to content

Commit

Permalink
🏗 Rewrite dep-check with esbuild and babel (#32845)
Browse files Browse the repository at this point in the history
  • Loading branch information
rsimha committed Feb 24, 2021
1 parent d6ba94c commit 85fbc06
Show file tree
Hide file tree
Showing 6 changed files with 100 additions and 142 deletions.
2 changes: 1 addition & 1 deletion .gitignore
Expand Up @@ -2,7 +2,7 @@
.g4ignore
build/
.karma-cache*
.amp-build
.amp-dep-check
c
/dist
dist.3p
Expand Down
3 changes: 3 additions & 0 deletions build-system/babel-config/dep-check-config.js
Expand Up @@ -39,6 +39,9 @@ function getDepCheckConfig() {
},
];
const depCheckPlugins = [
'./build-system/babel-plugins/babel-plugin-transform-json-import',
'./build-system/babel-plugins/babel-plugin-transform-json-configuration',
'./build-system/babel-plugins/babel-plugin-transform-jss',
'./build-system/babel-plugins/babel-plugin-transform-fix-leading-comments',
'./build-system/babel-plugins/babel-plugin-transform-promise-resolve',
'@babel/plugin-transform-react-constant-elements',
Expand Down
2 changes: 1 addition & 1 deletion build-system/tasks/clean.js
Expand Up @@ -28,7 +28,7 @@ const ROOT_DIR = path.resolve(__dirname, '../../');
*/
async function clean() {
const pathsToDelete = [
'.amp-build',
'.amp-dep-check',
'.karma-cache*',
'build',
'build-system/server/new-server/transforms/dist',
Expand Down
210 changes: 94 additions & 116 deletions build-system/tasks/dep-check.js
Expand Up @@ -15,26 +15,22 @@
*/
'use strict';

const babelify = require('babelify');
const browserify = require('browserify');
const babel = require('@babel/core');
const depCheckConfig = require('../test-configs/dep-check-config');
const esbuild = require('esbuild');
const fs = require('fs-extra');
const gulp = require('gulp');
const minimatch = require('minimatch');
const path = require('path');
const source = require('vinyl-source-stream');
const through = require('through2');
const {
createCtrlcHandler,
exitCtrlcHandler,
} = require('../common/ctrlcHandler');
const {compileJison} = require('./compile-jison');
const {css} = require('./css');
const {cyan, red, yellow} = require('kleur/colors');
const {cyan, green, red, yellow} = require('kleur/colors');
const {log, logLocalDev} = require('../common/logging');

const root = process.cwd();
const absPathRegExp = new RegExp(`^${root}/`);
const depCheckDir = '.amp-dep-check';

/**
* @typedef {{
Expand Down Expand Up @@ -155,90 +151,88 @@ Rule.prototype.matchBadDeps = function (moduleName, deps) {
const rules = depCheckConfig.rules.map((config) => new Rule(config));

/**
* Returns a list of entryPoint modules.
* extensions/{$extension}/{$version}/{$extension}.js
* src/amp.js
* 3p/integration.js
*
* @return {!Promise<!Array<string>>}
* Returns a single module that contains a list of entry points to these files:
* - extensions/{$extension}/{$version}/{$extension}.js
* - src/amp.js
* - 3p/integration.js
* @return {string}
*/
function getSrcs() {
return fs
.readdir('extensions')
.then((dirItems) => {
// Look for extension entry points
return flatten(
dirItems
.map((x) => `extensions/${x}`)
.filter((x) => fs.statSync(x).isDirectory())
.map(getEntryModule)
// Concat the core binary and integration binary as entry points.
.concat('src/amp.js', '3p/integration.js')
);
})
.then((files) => {
// Write all the entry modules into a single file so they can be processed
// together.
fs.mkdirpSync('./.amp-build');
const filename = './.amp-build/gulp-dep-check-collection.js';
fs.writeFileSync(
filename,
files
.map((file) => {
return `import '../${file}';`;
})
.join('\n')
);
return [filename];
});
async function getEntryPointModule() {
const coreBinaries = ['src/amp.js', '3p/integration.js'];
const extensions = await fs.promises.readdir('extensions');
const extensionEntryPoints = extensions
.map((x) => `extensions/${x}`)
.filter((x) => fs.statSync(x).isDirectory())
.map(getEntryPoint);
const allEntryPoints = flatten(extensionEntryPoints).concat(coreBinaries);
await fs.ensureDir(depCheckDir);
const entryPointModule = path.join(depCheckDir, 'entry-point-module.js');
const entryPointData = allEntryPoints
.map((file) => `import '../${file}';`)
.join('\n');
await fs.promises.writeFile(entryPointModule, entryPointData);
logLocalDev('Added all entry points to', cyan(entryPointModule));
return entryPointModule;
}

/**
* @param {string} entryModule
* @return {!Promise<!ModuleDef>}
* @param {string} entryPointModule
* @return {!ModuleDef}
*/
function getGraph(entryModule) {
let resolve;
const promise = new Promise((r) => {
resolve = r;
});
const module = Object.create(null);
module.name = entryModule;
module.deps = [];

// TODO(erwinm): Try and work this in with `gulp build` so that
// we're not running browserify twice during CI.
const bundler = browserify(entryModule, {
debug: true,
fast: true,
}).transform(babelify, {caller: {name: 'dep-check'}, global: true});

bundler.pipeline.get('deps').push(
through.obj(function (row, _enc, next) {
module.deps.push({
name: row.file.replace(absPathRegExp, ''),
deps: row.deps,
async function getModuleGraph(entryPointModule) {
const esbuildBabelPlugin = {
name: 'babel',
async setup(build) {
const transformContents = async ({file, contents}) => {
const babelOptions = babel.loadOptions({
caller: {name: 'dep-check'},
filename: file.path,
sourceFileName: path.relative(process.cwd(), file.path),
});
const result = await babel.transformAsync(contents, babelOptions);
return {contents: result.code};
};

build.onLoad({filter: /.*/, namespace: ''}, async (file) => {
const contents = await fs.promises.readFile(file.path, 'utf-8');
const transformed = await transformContents({file, contents});
return transformed;
});
this.push(row);
next();
})
);
bundler
.bundle()
.pipe(source(entryModule))
// Unfortunately we need to write the files out.
.pipe(gulp.dest('./.amp-build'))
.on('end', () => {
resolve(module);
},
};

const bundleFile = path.join(depCheckDir, 'entry-point-bundle.js');
const moduleGraphFile = path.join(depCheckDir, 'module-graph.json');
await esbuild.build({
entryPoints: [entryPointModule],
bundle: true,
outfile: bundleFile,
metafile: moduleGraphFile,
plugins: [esbuildBabelPlugin],
});
logLocalDev('Bundled all entry points into', cyan(bundleFile));

const moduleGraphJson = await fs.readJson(moduleGraphFile);
const entryPoints = moduleGraphJson.inputs;
const moduleGraph = Object.create(null);
moduleGraph.name = entryPointModule;
moduleGraph.deps = [];

for (const entryPoint in entryPoints) {
moduleGraph.deps.push({
name: entryPoint,
deps: entryPoints[entryPoint].imports.map((dep) => dep.path),
});
return promise;
}
logLocalDev('Extracted module graph from', cyan(moduleGraphFile));
return moduleGraph;
}

/**
* @param {string} extensionFolder
* @return {!Array<!ModuleDef>}
*/
function getEntryModule(extensionFolder) {
function getEntryPoint(extensionFolder) {
const extension = path.basename(extensionFolder);
return fs
.readdirSync(extensionFolder)
Expand All @@ -250,25 +244,17 @@ function getEntryModule(extensionFolder) {
}

/**
* Flattens the graph to easily run through the Rules. Original graph
* would be nested ModuleDef's wherein the top level are entry points
* with nested dependencies. We flatten it so all we have are individual
* modules and their imports as well as making the entries unique.
* Flattens the module dependency graph and makes its entries unique. This
* serves as the input on which all rules are tested.
*
* @param {!Array<!ModuleDef>} entryPoints
* @return {!ModuleDef}
*/
function flattenGraph(entryPoints) {
// Flatten the graph by just getting all the deps from all
// the entry points.
entryPoints = entryPoints.map((entryPoint) => entryPoint.deps);
// Now make the graph have unique entries
return flatten(entryPoints).reduce((acc, cur) => {
return flatten(entryPoints.deps).reduce((acc, cur) => {
const {name} = cur;
if (!acc[name]) {
acc[name] = Object.keys(cur.deps)
// Get rid of the absolute path for minimatch'ing
.map((x) => cur.deps[x].replace(absPathRegExp, ''));
acc[name] = Object.values(cur.deps);
}
return acc;
}, Object.create(null));
Expand All @@ -277,12 +263,12 @@ function flattenGraph(entryPoints) {
/**
* Run Module dependency graph against the rules.
*
* @param {!ModuleDef} modules
* @param {!ModuleDef} moduleGraph
* @return {boolean} true if violations were discovered.
*/
function runRules(modules) {
function runRules(moduleGraph) {
const errors = [];
Object.entries(modules).forEach(([moduleName, deps]) => {
Object.entries(moduleGraph).forEach(([moduleName, deps]) => {
// Run Rules against the modules and flatten for reporting.
const results = rules.flatMap((rule) => rule.run(moduleName, deps));
errors.push(...results);
Expand All @@ -306,28 +292,20 @@ async function depCheck() {
await css();
await compileJison();
logLocalDev('Checking dependencies...');
return getSrcs()
.then((entryPoints) => {
// This check is for extension folders that actually dont have
// an extension entry point module yet.
entryPoints = entryPoints.filter((x) => fs.existsSync(x));
return Promise.all(entryPoints.map(getGraph));
})
.then(flattenGraph)
.then(runRules)
.then((errorsFound) => {
if (errorsFound) {
log(
yellow('NOTE:'),
'Valid dependencies should be added whereas unused ones should be deleted. Please fix',
cyan('build-system/test-configs/dep-check-config.js')
);
const reason = new Error('Dependency checks failed');
reason.showStack = false;
return Promise.reject(reason);
}
})
.then(() => exitCtrlcHandler(handlerProcess));
const entryPointModule = await getEntryPointModule();
const moduleGraph = await getModuleGraph(entryPointModule);
const errorsFound = runRules(flattenGraph(moduleGraph));
if (errorsFound) {
log(
yellow('NOTE:'),
'Invalid dependencies must be removed, while valid ones must be added to',
cyan('build-system/test-configs/dep-check-config.js')
);
throw new Error('Dependency checks failed');
} else {
logLocalDev(green('SUCCESS:'), 'Checked all dependencies.');
}
exitCtrlcHandler(handlerProcess);
}

/**
Expand Down
22 changes: 0 additions & 22 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions package.json
Expand Up @@ -53,9 +53,9 @@
"@chiragrupani/karma-chromium-edge-launcher": "2.1.0",
"@jest/core": "26.6.3",
"@sinonjs/fake-timers": "7.0.2",
"@types/node": "14.14.28",
"@types/fancy-log": "1.3.1",
"@types/minimist": "1.2.1",
"@types/node": "14.14.28",
"acorn-globals": "6.0.0",
"amphtml-validator": "1.0.34",
"ast-replace": "1.1.3",
Expand Down Expand Up @@ -190,7 +190,6 @@
"tsickle": "0.39.1",
"typescript": "4.1.3",
"uglifyify": "5.0.2",
"vinyl-source-stream": "2.0.0",
"vinyl-sourcemaps-apply": "0.2.1",
"watchify": "4.0.0"
}
Expand Down

0 comments on commit 85fbc06

Please sign in to comment.