From 85fbc06cb46aac592002914af88349f34f57e908 Mon Sep 17 00:00:00 2001 From: Raghu Simha Date: Tue, 23 Feb 2021 23:10:29 -0500 Subject: [PATCH] =?UTF-8?q?=F0=9F=8F=97=20Rewrite=20`dep-check`=20with=20`?= =?UTF-8?q?esbuild`=20and=20`babel`=20(#32845)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .gitignore | 2 +- build-system/babel-config/dep-check-config.js | 3 + build-system/tasks/clean.js | 2 +- build-system/tasks/dep-check.js | 210 ++++++++---------- package-lock.json | 22 -- package.json | 3 +- 6 files changed, 100 insertions(+), 142 deletions(-) diff --git a/.gitignore b/.gitignore index e245c6eba25f..54a0daf1f73e 100644 --- a/.gitignore +++ b/.gitignore @@ -2,7 +2,7 @@ .g4ignore build/ .karma-cache* -.amp-build +.amp-dep-check c /dist dist.3p diff --git a/build-system/babel-config/dep-check-config.js b/build-system/babel-config/dep-check-config.js index e2da969e2e88..e49803518d5a 100644 --- a/build-system/babel-config/dep-check-config.js +++ b/build-system/babel-config/dep-check-config.js @@ -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', diff --git a/build-system/tasks/clean.js b/build-system/tasks/clean.js index d134607c2730..849e8f26311d 100644 --- a/build-system/tasks/clean.js +++ b/build-system/tasks/clean.js @@ -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', diff --git a/build-system/tasks/dep-check.js b/build-system/tasks/dep-check.js index f6cdcba1fe85..090dcf8b53fa 100644 --- a/build-system/tasks/dep-check.js +++ b/build-system/tasks/dep-check.js @@ -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 {{ @@ -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>} + * 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} + * @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} */ -function getEntryModule(extensionFolder) { +function getEntryPoint(extensionFolder) { const extension = path.basename(extensionFolder); return fs .readdirSync(extensionFolder) @@ -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} 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)); @@ -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); @@ -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); } /** diff --git a/package-lock.json b/package-lock.json index 5a8ee47fd361..f9018f0207d8 100644 --- a/package-lock.json +++ b/package-lock.json @@ -31659,28 +31659,6 @@ } } }, - "vinyl-source-stream": { - "version": "2.0.0", - "resolved": "https://registry.npmjs.org/vinyl-source-stream/-/vinyl-source-stream-2.0.0.tgz", - "integrity": "sha1-84pa+53R6Ttl1VBGmsYYKsT1S44=", - "dev": true, - "requires": { - "through2": "^2.0.3", - "vinyl": "^2.1.0" - }, - "dependencies": { - "through2": { - "version": "2.0.5", - "resolved": "https://registry.npmjs.org/through2/-/through2-2.0.5.tgz", - "integrity": "sha512-/mrRod8xqpA+IHSLyGCQ2s8SPHiCDEeQJSep1jqLYeEUClOFG2Qsh+4FU6G9VeqpZnGW/Su8LQGc4YKni5rYSQ==", - "dev": true, - "requires": { - "readable-stream": "~2.3.6", - "xtend": "~4.0.1" - } - } - } - }, "vinyl-sourcemap": { "version": "1.1.0", "resolved": "https://registry.npmjs.org/vinyl-sourcemap/-/vinyl-sourcemap-1.1.0.tgz", diff --git a/package.json b/package.json index 3a0a7a317473..9a1048d07e47 100644 --- a/package.json +++ b/package.json @@ -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", @@ -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" }