From b7138c1ec5481c6b8e9a7442b2ca3e350895cd3d Mon Sep 17 00:00:00 2001 From: Paul Gschwendtner Date: Tue, 17 Mar 2020 12:58:17 +0100 Subject: [PATCH] build: remove rollup packaging from dev-infra (#35647) The dev-infra package currently uses rollup for packaging. This has been done initially as a way to workaround manifest paths being used in the AMD JavaScript output. The actual solution to this problem is setting module names that match the `package.json` name. This ensures that the package can be consumed correctly in Bazel, and through NPM. This allows us to get rid of the rollup bundling, and we don't need to hard-code which dependencies should be external or included. Additionally, tools that are part of `dev-infra` can now specify their external dependencies simply in the `package.json`. To reduce version duplication, and out-of-sync versions, a new genrule has been created that syncs the versions with the top-level project `package.json`. PR Close #35647 --- BUILD.bazel | 1 + dev-infra/BUILD.bazel | 30 ++++++------ dev-infra/cli.ts | 1 + dev-infra/package.json | 10 ---- dev-infra/pullapprove/BUILD.bazel | 1 + dev-infra/rollup.config.js | 16 ------- dev-infra/tmpl-package.json | 21 +++++++++ tools/BUILD.bazel | 6 +++ tools/inline-package-json-deps.js | 76 +++++++++++++++++++++++++++++++ 9 files changed, 121 insertions(+), 41 deletions(-) delete mode 100644 dev-infra/package.json delete mode 100644 dev-infra/rollup.config.js create mode 100644 dev-infra/tmpl-package.json create mode 100644 tools/inline-package-json-deps.js diff --git a/BUILD.bazel b/BUILD.bazel index 0454cde4e1..7de233d906 100644 --- a/BUILD.bazel +++ b/BUILD.bazel @@ -8,6 +8,7 @@ exports_files([ "scripts/ci/track-payload-size.sh", "scripts/ci/payload-size.sh", "scripts/ci/payload-size.js", + "package.json", ]) alias( diff --git a/dev-infra/BUILD.bazel b/dev-infra/BUILD.bazel index 11e9640a56..2d70417db3 100644 --- a/dev-infra/BUILD.bazel +++ b/dev-infra/BUILD.bazel @@ -1,38 +1,38 @@ load("@build_bazel_rules_nodejs//:index.bzl", "pkg_npm") load("@npm_bazel_typescript//:index.bzl", "ts_library") -load("@npm_bazel_rollup//:index.bzl", "rollup_bundle") ts_library( name = "cli", srcs = [ "cli.ts", ], + module_name = "@angular/dev-infra-private", deps = [ "//dev-infra/pullapprove", "@npm//@types/node", ], ) -rollup_bundle( - name = "bundle", - config_file = "rollup.config.js", - entry_point = ":cli.ts", - format = "umd", - sourcemap = "hidden", - deps = [ - ":cli", - "@npm//rollup-plugin-commonjs", - "@npm//rollup-plugin-node-resolve", +genrule( + name = "package-json", + srcs = [ + "tmpl-package.json", + "//:package.json", ], + outs = ["package.json"], + cmd = """ + $(location //tools:inline-package-json-deps) $(location tmpl-package.json) \ + $(location //:package.json) $@ + """, + tools = ["//tools:inline-package-json-deps"], ) pkg_npm( name = "npm_package", - srcs = [ - "package.json", - ], visibility = ["//visibility:public"], deps = [ - ":bundle", + ":cli", + ":package-json", + "//dev-infra/ts-circular-dependencies", ], ) diff --git a/dev-infra/cli.ts b/dev-infra/cli.ts index 2d7086b568..b6285c768d 100644 --- a/dev-infra/cli.ts +++ b/dev-infra/cli.ts @@ -1,3 +1,4 @@ +#!/usr/bin/env node /** * @license * Copyright Google Inc. All Rights Reserved. diff --git a/dev-infra/package.json b/dev-infra/package.json deleted file mode 100644 index 6c14408cc8..0000000000 --- a/dev-infra/package.json +++ /dev/null @@ -1,10 +0,0 @@ -{ - "name": "@angular/dev-infra-private", - "version": "0.0.0", - "description": "INTERNAL USE ONLY - Angular internal DevInfra tooling/scripts - INTERNAL USE ONLY", - "license": "MIT", - "private": true, - "bin": { - "ng-dev": "./bundle.js" - } -} diff --git a/dev-infra/pullapprove/BUILD.bazel b/dev-infra/pullapprove/BUILD.bazel index 7da7be1484..5989f338e6 100644 --- a/dev-infra/pullapprove/BUILD.bazel +++ b/dev-infra/pullapprove/BUILD.bazel @@ -5,6 +5,7 @@ ts_library( srcs = [ "verify.ts", ], + module_name = "@angular/dev-infra-private/pullapprove", visibility = ["//dev-infra:__subpackages__"], deps = [ "@npm//@types/minimatch", diff --git a/dev-infra/rollup.config.js b/dev-infra/rollup.config.js deleted file mode 100644 index 8c8713475f..0000000000 --- a/dev-infra/rollup.config.js +++ /dev/null @@ -1,16 +0,0 @@ -const node = require('rollup-plugin-node-resolve'); -const commonjs = require('rollup-plugin-commonjs'); - -module.exports = { - external: ['shelljs', 'minimatch', 'yaml'], - preferBuiltins: true, - output: { - banner: "#!/usr/bin/env node", - }, - plugins: [ - node({ - mainFields: ['browser', 'es2015', 'module', 'jsnext:main', 'main'], - }), - commonjs(), - ], -}; diff --git a/dev-infra/tmpl-package.json b/dev-infra/tmpl-package.json new file mode 100644 index 0000000000..44534e250a --- /dev/null +++ b/dev-infra/tmpl-package.json @@ -0,0 +1,21 @@ +{ + "name": "@angular/dev-infra-private", + "version": "0.0.0", + "description": "INTERNAL USE ONLY - Angular internal DevInfra tooling/scripts - INTERNAL USE ONLY", + "license": "MIT", + "private": true, + "bin": { + "ng-dev": "./cli.js", + "ts-circular-deps": "./ts-circular-dependencies/index.js" + }, + "peerDependencies": { + "chalk": "", + "glob": "", + "minimatch": "", + "shelljs": "", + "typescript": "", + "yaml": "", + "yargs": "", + "tslib": "" + } +} diff --git a/tools/BUILD.bazel b/tools/BUILD.bazel index a4819a92e6..4d03ccc4ab 100644 --- a/tools/BUILD.bazel +++ b/tools/BUILD.bazel @@ -1,6 +1,7 @@ package(default_visibility = ["//visibility:public"]) load("@npm_bazel_typescript//:index.bzl", "ts_config") +load("//tools:defaults.bzl", "nodejs_binary") exports_files([ "tsconfig.json", @@ -13,6 +14,11 @@ ts_config( deps = ["tsconfig.json"], ) +nodejs_binary( + name = "inline-package-json-deps", + entry_point = "inline-package-json-deps.js", +) + platform( name = "rbe_ubuntu1604-angular", parents = ["@rbe_ubuntu1604_angular//config:platform"], diff --git a/tools/inline-package-json-deps.js b/tools/inline-package-json-deps.js new file mode 100644 index 0000000000..58a8f4a354 --- /dev/null +++ b/tools/inline-package-json-deps.js @@ -0,0 +1,76 @@ +#!/usr/bin/env node +/** + * @license + * Copyright Google Inc. All Rights Reserved. + * + * Use of this source code is governed by an MIT-style license that can be + * found in the LICENSE file at https://angular.io/license + */ + +/* + * Script that updates a dependencies in a specified `package.json` file to be + * based on the dependencies of the specified base `package.json`. This allows + * developers to sync dependencies between two `package.json` files without having + * to manually copy versions all the time. + * + * e.g. `/package.json` defines the project dependencies. The `dev-infra/package.json` + * uses a subset of these dependencies and declares these as dependencies for the shared + * package. The dependencies should be the same as the one from `/package.json` as those + * versions are used for testing and development. We don't want mismatching versions. + */ + +const fs = require('fs'); +const args = process.argv.slice(2); +const [inputPackageJsonPath, basePackageJsonPath, outputPath] = args; +const BASE_DEPENDENCY_MARKER = ''; + +if (!inputPackageJsonPath || !basePackageJsonPath || !outputPath) { + console.error('Usage: ./inline-package-json-deps.js '); + process.exit(1); +} + +const inputPackageJson = JSON.parse(fs.readFileSync(inputPackageJsonPath, 'utf8')); +const basePackageJson = JSON.parse(fs.readFileSync(basePackageJsonPath, 'utf8')); +const result = {...inputPackageJson}; + +if (inputPackageJson.dependencies) { + inlineDependenciesFromBase(inputPackageJson.dependencies); +} +if (inputPackageJson.peerDependencies) { + inlineDependenciesFromBase(inputPackageJson.peerDependencies); +} + +fs.writeFileSync(outputPath, JSON.stringify(result, null, 2)); + +/** + * Updates dependencies which have their version set to the base marker, + * to match the version from the base `package.json` file. + */ +function inlineDependenciesFromBase(deps) { + Object.keys(deps).forEach(name => { + const value = deps[name]; + if (value !== BASE_DEPENDENCY_MARKER) { + return; + } + const linkedVersion = getDependency(basePackageJson, name); + if (linkedVersion === null) { + console.error(`Could not find base version for: ${name}`); + console.error( + `Either set a version for ${name} in "${basePackageJsonPath}", or use ` + + `an explicit version in "${inputPackageJson}"`); + process.exit(1); + } + deps[name] = linkedVersion; + }); +} + +/** Gets the version of the specified package from the given package object. */ +function getDependency(packageJson, name) { + if (packageJson.dependencies && packageJson.dependencies[name]) { + return packageJson.dependencies[name]; + } + if (packageJson.devDependencies && packageJson.devDependencies[name]) { + return packageJson.devDependencies[name]; + } + return null; +}