From 1429347c97095920f7dc8c29d579a3a013ce4e5b Mon Sep 17 00:00:00 2001 From: Kim Joar Bekkelund Date: Mon, 12 Mar 2018 12:01:11 +0100 Subject: [PATCH] [kbn-pm] Include Kibana's transitive _projects_ in build (#16813) (#17097) --- packages/eslint-config-kibana/package.json | 5 -- .../eslint-plugin-kibana-custom/package.json | 5 -- packages/kbn-pm/dist/index.js | 62 +++++++++++-------- .../__snapshots__/bootstrap.test.ts.snap | 10 +++ .../production/build_production_projects.ts | 37 ++++++----- .../__fixtures__/package.json | 15 +++++ .../__fixtures__/packages/baz/package.json | 1 + .../packages/shouldskip/package.json | 12 ++++ .../packages/shouldskip/src/index.js | 5 ++ .../build_production_projects.test.ts | 2 +- packages/kbn-pm/src/utils/project.ts | 15 ++--- packages/kbn-pm/src/utils/projects.test.ts | 44 +++++++++++++ packages/kbn-pm/src/utils/projects.ts | 29 +++++++++ tasks/build/packages.js | 8 +++ 14 files changed, 185 insertions(+), 65 deletions(-) create mode 100644 packages/kbn-pm/src/production/integration_tests/__fixtures__/package.json create mode 100644 packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/shouldskip/package.json create mode 100644 packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/shouldskip/src/index.js diff --git a/packages/eslint-config-kibana/package.json b/packages/eslint-config-kibana/package.json index ae803dd0ea1e..0eb02dd893ae 100644 --- a/packages/eslint-config-kibana/package.json +++ b/packages/eslint-config-kibana/package.json @@ -1,10 +1,5 @@ { "name": "@elastic/eslint-config-kibana", - "kibana": { - "build": { - "skip": true - } - }, "version": "0.14.0", "description": "The eslint config used by the kibana team", "main": ".eslintrc.js", diff --git a/packages/eslint-plugin-kibana-custom/package.json b/packages/eslint-plugin-kibana-custom/package.json index 734cab910ebc..0ffa600125ef 100644 --- a/packages/eslint-plugin-kibana-custom/package.json +++ b/packages/eslint-plugin-kibana-custom/package.json @@ -1,10 +1,5 @@ { "name": "@elastic/eslint-plugin-kibana-custom", - "kibana": { - "build": { - "skip": true - } - }, "version": "1.1.0", "description": "Custom ESLint rules for Kibana", "repository": { diff --git a/packages/kbn-pm/dist/index.js b/packages/kbn-pm/dist/index.js index 254f960265f3..5019c70aa524 100644 --- a/packages/kbn-pm/dist/index.js +++ b/packages/kbn-pm/dist/index.js @@ -6066,6 +6066,7 @@ let getProjects = exports.getProjects = (() => { exports.buildProjectGraph = buildProjectGraph; exports.topologicallyBatchProjects = topologicallyBatchProjects; +exports.includeTransitiveProjects = includeTransitiveProjects; var _glob = __webpack_require__(7); @@ -6156,6 +6157,22 @@ function topologicallyBatchProjects(projectsToBatch, projectGraph) { } return batches; } +function includeTransitiveProjects(subsetOfProjects, allProjects, { onlyProductionDependencies = false } = {}) { + const dependentProjects = new Map(); + // the current list of packages we are expanding using breadth-first-search + const toProcess = [...subsetOfProjects]; + while (toProcess.length > 0) { + const project = toProcess.shift(); + const dependencies = onlyProductionDependencies ? project.productionDependencies : project.allDependencies; + Object.keys(dependencies).forEach(dep => { + if (allProjects.has(dep)) { + toProcess.push(allProjects.get(dep)); + } + }); + dependentProjects.set(project.name, project); + } + return dependentProjects; +} /***/ }), /* 11 */ @@ -8869,7 +8886,9 @@ class Project { this.packageJsonLocation = _path2.default.resolve(this.path, 'package.json'); this.nodeModulesLocation = _path2.default.resolve(this.path, 'node_modules'); this.targetLocation = _path2.default.resolve(this.path, 'target'); - this.allDependencies = _extends({}, this.json.devDependencies || {}, this.json.dependencies || {}); + this.productionDependencies = this.json.dependencies || {}; + this.devDependencies = this.json.devDependencies || {}; + this.allDependencies = _extends({}, this.devDependencies, this.productionDependencies); this.scripts = this.json.scripts || {}; } get name() { @@ -8896,12 +8915,6 @@ class Project { getBuildConfig() { return this.json.kibana && this.json.kibana.build || {}; } - /** - * Whether a package should not be included in the Kibana build artifact. - */ - skipFromBuild() { - return this.getBuildConfig().skip === true; - } /** * Returns the directory that should be copied into the Kibana build artifact. * This config can be specified to only include the project's build artifacts @@ -35244,11 +35257,7 @@ exports.buildProductionProjects = undefined; let buildProductionProjects = exports.buildProductionProjects = (() => { var _ref = _asyncToGenerator(function* ({ kibanaRoot, buildRoot }) { - const projectPaths = (0, _config.getProjectPaths)(kibanaRoot, { - 'skip-kibana': true, - 'skip-kibana-extra': true - }); - const projects = yield getProductionProjects(kibanaRoot, projectPaths); + const projects = yield getProductionProjects(kibanaRoot); const projectGraph = (0, _projects.buildProjectGraph)(projects); const batchedProjects = (0, _projects.topologicallyBatchProjects)(projects, projectGraph); const projectNames = [...projects.values()].map(function (project) { @@ -35269,23 +35278,24 @@ let buildProductionProjects = exports.buildProductionProjects = (() => { }; })(); /** - * Returns only the projects that should be built into the production bundle + * Returns the subset of projects that should be built into the production + * bundle. As we copy these into Kibana's `node_modules` during the build step, + * and let Kibana's build process be responsible for installing dependencies, + * we only include Kibana's transitive _production_ dependencies. */ let getProductionProjects = (() => { - var _ref2 = _asyncToGenerator(function* (kibanaRoot, projectPaths) { - const projects = yield (0, _projects.getProjects)(kibanaRoot, projectPaths); - const buildProjects = new Map(); - for (const [name, project] of projects.entries()) { - if (!project.skipFromBuild()) { - buildProjects.set(name, project); - } - } - return buildProjects; + var _ref2 = _asyncToGenerator(function* (rootPath) { + const projectPaths = (0, _config.getProjectPaths)(rootPath, {}); + const projects = yield (0, _projects.getProjects)(rootPath, projectPaths); + const productionProjects = (0, _projects.includeTransitiveProjects)([projects.get('kibana')], projects, { onlyProductionDependencies: true }); + // We remove Kibana, as we're already building Kibana + productionProjects.delete('kibana'); + return productionProjects; }); - return function getProductionProjects(_x2, _x3) { + return function getProductionProjects(_x2) { return _ref2.apply(this, arguments); }; })(); @@ -35298,7 +35308,7 @@ let deleteTarget = (() => { } }); - return function deleteTarget(_x4) { + return function deleteTarget(_x3) { return _ref3.apply(this, arguments); }; })(); @@ -35310,7 +35320,7 @@ let buildProject = (() => { } }); - return function buildProject(_x5) { + return function buildProject(_x4) { return _ref4.apply(this, arguments); }; })(); @@ -35350,7 +35360,7 @@ let copyToBuild = (() => { yield (0, _package_json.writePackageJson)(buildProjectPath, preparedPackageJson); }); - return function copyToBuild(_x6, _x7, _x8) { + return function copyToBuild(_x5, _x6, _x7) { return _ref5.apply(this, arguments); }; })(); diff --git a/packages/kbn-pm/src/commands/__snapshots__/bootstrap.test.ts.snap b/packages/kbn-pm/src/commands/__snapshots__/bootstrap.test.ts.snap index 57c70ce24d64..39f99235aaff 100644 --- a/packages/kbn-pm/src/commands/__snapshots__/bootstrap.test.ts.snap +++ b/packages/kbn-pm/src/commands/__snapshots__/bootstrap.test.ts.snap @@ -8,6 +8,7 @@ Array [ "allDependencies": Object { "bar": "link:packages/bar", }, + "devDependencies": Object {}, "json": Object { "dependencies": Object { "bar": "link:packages/bar", @@ -18,11 +19,15 @@ Array [ "nodeModulesLocation": "/packages/kbn-pm/src/commands/node_modules", "packageJsonLocation": "/packages/kbn-pm/src/commands/package.json", "path": "/packages/kbn-pm/src/commands", + "productionDependencies": Object { + "bar": "link:packages/bar", + }, "scripts": Object {}, "targetLocation": "/packages/kbn-pm/src/commands/target", }, "bar" => Project { "allDependencies": Object {}, + "devDependencies": Object {}, "json": Object { "name": "bar", "scripts": Object { @@ -33,6 +38,7 @@ Array [ "nodeModulesLocation": "/packages/kbn-pm/src/commands/packages/bar/node_modules", "packageJsonLocation": "/packages/kbn-pm/src/commands/packages/bar/package.json", "path": "/packages/kbn-pm/src/commands/packages/bar", + "productionDependencies": Object {}, "scripts": Object { "kbn:bootstrap": "node ./bar.js", }, @@ -43,6 +49,7 @@ Array [ "kibana" => Array [ Project { "allDependencies": Object {}, + "devDependencies": Object {}, "json": Object { "name": "bar", "scripts": Object { @@ -53,6 +60,7 @@ Array [ "nodeModulesLocation": "/packages/kbn-pm/src/commands/packages/bar/node_modules", "packageJsonLocation": "/packages/kbn-pm/src/commands/packages/bar/package.json", "path": "/packages/kbn-pm/src/commands/packages/bar", + "productionDependencies": Object {}, "scripts": Object { "kbn:bootstrap": "node ./bar.js", }, @@ -72,6 +80,7 @@ Array [ Array [], Project { "allDependencies": Object {}, + "devDependencies": Object {}, "json": Object { "name": "bar", "scripts": Object { @@ -82,6 +91,7 @@ Array [ "nodeModulesLocation": "/packages/kbn-pm/src/commands/packages/bar/node_modules", "packageJsonLocation": "/packages/kbn-pm/src/commands/packages/bar/package.json", "path": "/packages/kbn-pm/src/commands/packages/bar", + "productionDependencies": Object {}, "scripts": Object { "kbn:bootstrap": "node ./bar.js", }, diff --git a/packages/kbn-pm/src/production/build_production_projects.ts b/packages/kbn-pm/src/production/build_production_projects.ts index 6d06cee145b4..7e107e24c134 100644 --- a/packages/kbn-pm/src/production/build_production_projects.ts +++ b/packages/kbn-pm/src/production/build_production_projects.ts @@ -8,6 +8,7 @@ import { buildProjectGraph, topologicallyBatchProjects, ProjectMap, + includeTransitiveProjects, } from '../utils/projects'; import { createProductionPackageJson, @@ -24,12 +25,7 @@ export async function buildProductionProjects({ kibanaRoot: string; buildRoot: string; }) { - const projectPaths = getProjectPaths(kibanaRoot, { - 'skip-kibana': true, - 'skip-kibana-extra': true, - }); - - const projects = await getProductionProjects(kibanaRoot, projectPaths); + const projects = await getProductionProjects(kibanaRoot); const projectGraph = buildProjectGraph(projects); const batchedProjects = topologicallyBatchProjects(projects, projectGraph); @@ -46,22 +42,25 @@ export async function buildProductionProjects({ } /** - * Returns only the projects that should be built into the production bundle + * Returns the subset of projects that should be built into the production + * bundle. As we copy these into Kibana's `node_modules` during the build step, + * and let Kibana's build process be responsible for installing dependencies, + * we only include Kibana's transitive _production_ dependencies. */ -async function getProductionProjects( - kibanaRoot: string, - projectPaths: string[] -) { - const projects = await getProjects(kibanaRoot, projectPaths); +async function getProductionProjects(rootPath: string) { + const projectPaths = getProjectPaths(rootPath, {}); + const projects = await getProjects(rootPath, projectPaths); - const buildProjects: ProjectMap = new Map(); - for (const [name, project] of projects.entries()) { - if (!project.skipFromBuild()) { - buildProjects.set(name, project); - } - } + const productionProjects = includeTransitiveProjects( + [projects.get('kibana')!], + projects, + { onlyProductionDependencies: true } + ); - return buildProjects; + // We remove Kibana, as we're already building Kibana + productionProjects.delete('kibana'); + + return productionProjects; } async function deleteTarget(project: Project) { diff --git a/packages/kbn-pm/src/production/integration_tests/__fixtures__/package.json b/packages/kbn-pm/src/production/integration_tests/__fixtures__/package.json new file mode 100644 index 000000000000..60e84ee21815 --- /dev/null +++ b/packages/kbn-pm/src/production/integration_tests/__fixtures__/package.json @@ -0,0 +1,15 @@ +{ + "name": "kibana", + "version": "1.0.0", + "private": true, + "dependencies": { + "@elastic/foo": "link:packages/foo", + "@elastic/baz": "link:packages/baz" + }, + "devDependencies": { + + }, + "scripts": { + "build": "echo 'should not be called'; false" + } +} \ No newline at end of file diff --git a/packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/baz/package.json b/packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/baz/package.json index aab2b2f06cfd..f33caad0824e 100644 --- a/packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/baz/package.json +++ b/packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/baz/package.json @@ -4,6 +4,7 @@ "private": true, "main": "./code.js", "dependencies": { + "@elastic/quux": "link:../quux", "noop3": "999.999.999" }, "devDependencies": { diff --git a/packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/shouldskip/package.json b/packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/shouldskip/package.json new file mode 100644 index 000000000000..ee7406d9ac85 --- /dev/null +++ b/packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/shouldskip/package.json @@ -0,0 +1,12 @@ +{ + "name": "@elastic/shouldskip", + "version": "1.0.0", + "private": true, + "main": "./target/index.js", + "dependencies": { + "@elastic/bar": "link:../bar" + }, + "scripts": { + "build": "echo 'should not be called'; false" + } +} \ No newline at end of file diff --git a/packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/shouldskip/src/index.js b/packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/shouldskip/src/index.js new file mode 100644 index 000000000000..bbe0c2322a49 --- /dev/null +++ b/packages/kbn-pm/src/production/integration_tests/__fixtures__/packages/shouldskip/src/index.js @@ -0,0 +1,5 @@ +import bar from '@elastic/bar'; // eslint-disable-line import/no-unresolved + +export default function(val) { + return 'test [' + val + '] (' + bar(val) + ')'; +} diff --git a/packages/kbn-pm/src/production/integration_tests/build_production_projects.test.ts b/packages/kbn-pm/src/production/integration_tests/build_production_projects.test.ts index e3e92f3e4d68..338a8fd1259a 100644 --- a/packages/kbn-pm/src/production/integration_tests/build_production_projects.test.ts +++ b/packages/kbn-pm/src/production/integration_tests/build_production_projects.test.ts @@ -23,7 +23,7 @@ describe('kbn-pm production', function() { dot: true, }); - const projects = await getProjects(tmpDir, ['./packages/*']); + const projects = await getProjects(tmpDir, ['.', './packages/*']); for (const project of projects.values()) { // This will both install dependencies and generate `yarn.lock` files diff --git a/packages/kbn-pm/src/utils/project.ts b/packages/kbn-pm/src/utils/project.ts index 2f6d1c649d7a..0f138937c579 100644 --- a/packages/kbn-pm/src/utils/project.ts +++ b/packages/kbn-pm/src/utils/project.ts @@ -33,6 +33,8 @@ export class Project { public readonly targetLocation: string; public readonly path: string; public readonly allDependencies: PackageDependencies; + public readonly productionDependencies: PackageDependencies; + public readonly devDependencies: PackageDependencies; public readonly scripts: PackageScripts; constructor(packageJson: PackageJson, projectPath: string) { @@ -43,9 +45,11 @@ export class Project { this.nodeModulesLocation = path.resolve(this.path, 'node_modules'); this.targetLocation = path.resolve(this.path, 'target'); + this.productionDependencies = this.json.dependencies || {}; + this.devDependencies = this.json.devDependencies || {}; this.allDependencies = { - ...(this.json.devDependencies || {}), - ...(this.json.dependencies || {}), + ...this.devDependencies, + ...this.productionDependencies, }; this.scripts = this.json.scripts || {}; @@ -95,13 +99,6 @@ export class Project { return (this.json.kibana && this.json.kibana.build) || {}; } - /** - * Whether a package should not be included in the Kibana build artifact. - */ - skipFromBuild() { - return this.getBuildConfig().skip === true; - } - /** * Returns the directory that should be copied into the Kibana build artifact. * This config can be specified to only include the project's build artifacts diff --git a/packages/kbn-pm/src/utils/projects.test.ts b/packages/kbn-pm/src/utils/projects.test.ts index 0a52c2da4d4b..2f737d03c641 100644 --- a/packages/kbn-pm/src/utils/projects.test.ts +++ b/packages/kbn-pm/src/utils/projects.test.ts @@ -4,6 +4,7 @@ import { getProjects, buildProjectGraph, topologicallyBatchProjects, + includeTransitiveProjects, } from './projects'; import { Project } from './project'; import { getProjectPaths } from '../config'; @@ -105,3 +106,46 @@ describe('#topologicallyBatchProjects', () => { expect(expectedBatches).toMatchSnapshot(); }); }); + +describe('#includeTransitiveProjects', () => { + test('includes transitive dependencies for Kibana package', async () => { + const projects = await getProjects(rootPath, ['.', 'packages/*']); + + const kibana = projects.get('kibana')!; + const withTransitive = includeTransitiveProjects([kibana], projects); + + expect([...withTransitive.keys()]).toEqual(['kibana', 'foo']); + }); + + test('handles multiple projects with same transitive dep', async () => { + const projects = await getProjects(rootPath, ['.', 'packages/*']); + + const kibana = projects.get('kibana')!; + const bar = projects.get('bar')!; + const withTransitive = includeTransitiveProjects([kibana, bar], projects); + + expect([...withTransitive.keys()]).toEqual(['kibana', 'bar', 'foo']); + }); + + test('handles projects with no deps', async () => { + const projects = await getProjects(rootPath, ['.', 'packages/*']); + + const foo = projects.get('foo')!; + const withTransitive = includeTransitiveProjects([foo], projects); + + expect([...withTransitive.keys()]).toEqual(['foo']); + }); + + test('includes dependencies of dependencies', async () => { + const projects = await getProjects(rootPath, [ + '.', + 'packages/*', + '../plugins/*', + ]); + + const quux = projects.get('quux')!; + const withTransitive = includeTransitiveProjects([quux], projects); + + expect([...withTransitive.keys()]).toEqual(['quux', 'bar', 'baz', 'foo']); + }); +}); diff --git a/packages/kbn-pm/src/utils/projects.ts b/packages/kbn-pm/src/utils/projects.ts index dd89b49dd458..20cccabd4cff 100644 --- a/packages/kbn-pm/src/utils/projects.ts +++ b/packages/kbn-pm/src/utils/projects.ts @@ -143,3 +143,32 @@ export function topologicallyBatchProjects( return batches; } + +export function includeTransitiveProjects( + subsetOfProjects: Project[], + allProjects: ProjectMap, + { onlyProductionDependencies = false } = {} +) { + const dependentProjects: ProjectMap = new Map(); + + // the current list of packages we are expanding using breadth-first-search + const toProcess = [...subsetOfProjects]; + + while (toProcess.length > 0) { + const project = toProcess.shift()!; + + const dependencies = onlyProductionDependencies + ? project.productionDependencies + : project.allDependencies; + + Object.keys(dependencies).forEach(dep => { + if (allProjects.has(dep)) { + toProcess.push(allProjects.get(dep)!); + } + }); + + dependentProjects.set(project.name, project); + } + + return dependentProjects; +} diff --git a/tasks/build/packages.js b/tasks/build/packages.js index 30ceccac0fda..c2af8963498c 100644 --- a/tasks/build/packages.js +++ b/tasks/build/packages.js @@ -37,6 +37,14 @@ import { buildProductionProjects } from '@kbn/pm'; * will be located within the top-level `node_modules` folder, which means * normal module resolution will apply and you can `require(...)` any of these * packages when running Kibana in production. + * + * ## Known limitations + * + * - This process _only_ include packages that used by Kibana or any of its + * transitive packages, as it depends on only running `yarn` at the top level. + * That means a Kibana plugin can only depend on Kibana packages that are used + * in some way by Kibana itself in production, as it won't otherwise be + * included in the production build. */ module.exports = function (grunt) { grunt.registerTask('_build:packages', async function () {