From 055d5fab129ae5bf713268c47aa26c3f5846aa42 Mon Sep 17 00:00:00 2001 From: Greg Magolan Date: Mon, 8 Apr 2019 19:31:04 -0700 Subject: [PATCH] Move `scripts` to NodeModuleSources provider & remove ScriptsProvider --- internal/common/node_module_info.bzl | 1 + internal/common/providers.bzl | 17 ---------- internal/common/sources_aspect.bzl | 11 +++--- internal/npm_install/generate_build_file.js | 32 +++++++++++++---- internal/npm_install/node_module_library.bzl | 34 ++++++------------- .../@angular/core/BUILD.bazel.golden | 6 ++-- .../@gregmagolan/test-a/BUILD.bazel.golden | 5 +-- .../@gregmagolan/test-b/BUILD.bazel.golden | 5 +-- .../node_modules/ajv/BUILD.bazel.golden | 5 +-- .../node_modules/jasmine/BUILD.bazel.golden | 5 +-- .../node_modules/rxjs/BUILD.bazel.golden | 5 +-- .../node_modules/unidiff/BUILD.bazel.golden | 5 +-- .../node_modules/zone.js/BUILD.bazel.golden | 5 +-- 13 files changed, 65 insertions(+), 71 deletions(-) delete mode 100644 internal/common/providers.bzl diff --git a/internal/common/node_module_info.bzl b/internal/common/node_module_info.bzl index 0617e87458..a8e72c919b 100644 --- a/internal/common/node_module_info.bzl +++ b/internal/common/node_module_info.bzl @@ -25,6 +25,7 @@ NodeModuleInfo = provider( NodeModuleSources = provider( doc = "Provides sources for npm dependencies installed with yarn_install and npm_install rules", fields = { + "scripts": "Source files that are javascript named-UMD or named-AMD modules for use in rules such as ts_devserver", "sources": "Source files that are npm dependencies", "workspace": "The workspace name that the npm dependencies are provided from", }, diff --git a/internal/common/providers.bzl b/internal/common/providers.bzl deleted file mode 100644 index 7645e92ea1..0000000000 --- a/internal/common/providers.bzl +++ /dev/null @@ -1,17 +0,0 @@ -# Copyright 2019 The Bazel Authors. All rights reserved. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -# Provider for "node-module-like" rules such as `node_module_library`. -# It tells downstream rules how to consume Javascript files in the NPM package. -ScriptsProvider = provider(fields = ["scripts"]) diff --git a/internal/common/sources_aspect.bzl b/internal/common/sources_aspect.bzl index 4268dcff4b..99f1640a09 100644 --- a/internal/common/sources_aspect.bzl +++ b/internal/common/sources_aspect.bzl @@ -15,8 +15,7 @@ """Aspect to collect es5 js sources and scripts from deps. """ -load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo") -load("@build_bazel_rules_nodejs//internal/common:providers.bzl", "ScriptsProvider") +load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo", "NodeModuleSources") def _sources_aspect_impl(target, ctx): # TODO(kyliau): node_sources here is a misnomer because it implies that @@ -27,17 +26,15 @@ def _sources_aspect_impl(target, ctx): node_sources = depset() # dev_scripts is a collection of "scripts" from "node-module-like" targets - # such as `node_module_library`. Note that nothing is collected from the default - # filegroup target for generic node modules because it does not have the - # `scripts` provider nor does it have the `deps` attribute. + # such as `node_module_library` dev_scripts = depset() # Note layering: until we have JS interop providers, this needs to know how to # get TypeScript outputs. if hasattr(target, "typescript"): node_sources = depset(transitive = [node_sources, target.typescript.es5_sources]) - elif ScriptsProvider in target: - dev_scripts = depset(transitive = [dev_scripts, target[ScriptsProvider].scripts]) + elif NodeModuleSources in target: + dev_scripts = depset(transitive = [dev_scripts, target[NodeModuleSources].scripts]) elif hasattr(target, "files") and not NodeModuleInfo in target: # Sources from npm fine grained deps should not be included node_sources = depset( diff --git a/internal/npm_install/generate_build_file.js b/internal/npm_install/generate_build_file.js index 5add0f00ae..2b68b87d87 100644 --- a/internal/npm_install/generate_build_file.js +++ b/internal/npm_install/generate_build_file.js @@ -631,11 +631,11 @@ function printJson(pkg) { } /** - * A filter function for files in an npm package. + * A filter function for files in an npm package. Comparison is case-insensitive. * @param files array of files to filter - * @param exts list of white listed extensions; if empty, no filter is done on extensions; - * '' empty string denotes to allow files with no extensions, other extensions - * are listed with '.ext' notation such as '.d.ts'. + * @param exts list of white listed case-insensitive extensions; if empty, no filter is + * done on extensions; '' empty string denotes to allow files with no extensions, + * other extensions are listed with '.ext' notation such as '.d.ts'. */ function filterFiles(files, exts = []) { // Files with spaces (\x20) or unicode characters (<\x20 && >\x7E) are not allowed in @@ -647,8 +647,9 @@ function filterFiles(files, exts = []) { // include files with no extensions if noExt is true if (allowNoExts && !path.extname(f)) return true; // filter files in exts + const lc = f.toLowerCase(); for (const e of exts) { - if (e && f.endsWith(e)) { + if (e && lc.endsWith(e.toLowerCase())) { return true; } } @@ -677,12 +678,26 @@ function isNgApfPackage(pkg) { }); } +/** + * If the package is in the Angular package format returns list + * of package files that end with `.umd.js`, `.ngfactory.js` and `.ngsummary.js`. + */ +function getNgApfScripts(pkg) { + return isNgApfPackage(pkg) ? + filterFiles(pkg._files, ['.umd.js', '.ngfactory.js', '.ngsummary.js']) : + []; +} + /** * Given a pkg, return the skylark `node_module_library` targets for the package. */ function printPackage(pkg) { const sources = filterFiles(pkg._files, INCLUDED_FILES); const dtsSources = filterFiles(pkg._files, ['.d.ts']); + // TODO(gmagolan): add UMD & AMD scripts to scripts even if not an APF package _but_ only if they + // are named? + const scripts = getNgApfScripts(pkg); + const pkgDeps = pkg._dependencies.filter(dep => dep !== pkg && !dep._isNested); let result = `load("@build_bazel_rules_nodejs//:defs.bzl", "nodejs_binary") @@ -702,15 +717,18 @@ node_module_library( ${ pkgDeps.map(dep => `"//node_modules/${dep._dir}:${dep._name}__files",`).join('\n ')} ], - is_apf = ${isNgApfPackage(pkg) ? 'True' : 'False'}, ) +# ${pkg._name}__files target is used as dep for other package targets to prevent +# circular dependencies errors node_module_library( name = "${pkg._name}__files", srcs = [ ${sources.map(f => `":${f}",`).join('\n ')} ], - is_apf = ${isNgApfPackage(pkg) ? 'True' : 'False'}, + scripts = [ + ${scripts.map(f => `":${f}",`).join('\n ')} + ], ) node_module_library( diff --git a/internal/npm_install/node_module_library.bzl b/internal/npm_install/node_module_library.bzl index 479db50457..d7adcd87bc 100644 --- a/internal/npm_install/node_module_library.bzl +++ b/internal/npm_install/node_module_library.bzl @@ -13,27 +13,15 @@ # limitations under the License. load("@build_bazel_rules_nodejs//internal/common:node_module_info.bzl", "NodeModuleInfo", "NodeModuleSources") -load("@build_bazel_rules_nodejs//internal/common:providers.bzl", "ScriptsProvider") def _node_module_library_impl(ctx): - apf_umds = [] - apf_factories = [] - apf_summaries = [] - - # If this npm package is in the Angular package format then collect - # umd bundles, ngfactory files & ngsummary files and provide them - # via the ScriptsProvider - if ctx.attr.is_apf: - for file in ctx.files.srcs: - if file.basename.endswith(".umd.js"): - apf_umds.append(file) - elif file.basename.endswith(".ngfactory.js"): - apf_factories.append(file) - elif file.basename.endswith(".ngsummary.js"): - apf_summaries.append(file) - - sources = depset(transitive = [src.files for src in ctx.attr.srcs] + [dep.files for dep in ctx.attr.deps]) workspace = ctx.label.workspace_root.split("/")[1] if ctx.label.workspace_root else ctx.workspace_name + sources = depset(ctx.files.srcs, transitive = [dep.files for dep in ctx.attr.deps]) + + scripts = depset() + for dep in ctx.attr.scripts: + scripts = depset(transitive = [scripts, dep.files]) + scripts = depset(ctx.files.scripts, transitive = [scripts]) return [ DefaultInfo( @@ -44,11 +32,9 @@ def _node_module_library_impl(ctx): ), NodeModuleSources( sources = sources, + scripts = scripts, workspace = workspace, ), - ScriptsProvider( - scripts = depset(apf_umds + apf_factories + apf_summaries), - ), ] node_module_library = rule( @@ -58,9 +44,9 @@ node_module_library = rule( doc = "The list of files that comprise the package", allow_files = True, ), - "is_apf": attr.bool( - default = False, - doc = "True if this npm package is in the Angular package format", + "scripts": attr.label_list( + doc = "A subset of srcs that are javascript named-UMD or named-AMD scripts for use in rules such as ts_devserver", + allow_files = True, ), "deps": attr.label_list( doc = "Transitive dependencies of the package", diff --git a/internal/npm_install/test/golden/node_modules/@angular/core/BUILD.bazel.golden b/internal/npm_install/test/golden/node_modules/@angular/core/BUILD.bazel.golden index 7345bbca24..6c7475ddd7 100644 --- a/internal/npm_install/test/golden/node_modules/@angular/core/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/node_modules/@angular/core/BUILD.bazel.golden @@ -12,7 +12,6 @@ node_module_library( "//node_modules/rxjs:rxjs__files", "//node_modules/zone.js:zone.js__files", ], - is_apf = True, ) node_module_library( name = "core__files", @@ -629,7 +628,10 @@ node_module_library( ":testing/testing.d.ts", ":testing/testing.metadata.json", ], - is_apf = True, + scripts = [ + ":bundles/core-testing.umd.js", + ":bundles/core.umd.js", + ], ) node_module_library( name = "core__typings", diff --git a/internal/npm_install/test/golden/node_modules/@gregmagolan/test-a/BUILD.bazel.golden b/internal/npm_install/test/golden/node_modules/@gregmagolan/test-a/BUILD.bazel.golden index 3a50293b78..71eb172542 100644 --- a/internal/npm_install/test/golden/node_modules/@gregmagolan/test-a/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/node_modules/@gregmagolan/test-a/BUILD.bazel.golden @@ -10,7 +10,6 @@ node_module_library( deps = [ ], - is_apf = False, ) node_module_library( name = "test-a__files", @@ -19,7 +18,9 @@ node_module_library( ":main.js", ":package.json", ], - is_apf = False, + scripts = [ + + ], ) node_module_library( name = "test-a__typings", diff --git a/internal/npm_install/test/golden/node_modules/@gregmagolan/test-b/BUILD.bazel.golden b/internal/npm_install/test/golden/node_modules/@gregmagolan/test-b/BUILD.bazel.golden index fc274df3b5..13530d88a0 100644 --- a/internal/npm_install/test/golden/node_modules/@gregmagolan/test-b/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/node_modules/@gregmagolan/test-b/BUILD.bazel.golden @@ -10,7 +10,6 @@ node_module_library( deps = [ ], - is_apf = False, ) node_module_library( name = "test-b__files", @@ -20,7 +19,9 @@ node_module_library( ":node_modules/@gregmagolan/test-a/package.json", ":package.json", ], - is_apf = False, + scripts = [ + + ], ) node_module_library( name = "test-b__typings", diff --git a/internal/npm_install/test/golden/node_modules/ajv/BUILD.bazel.golden b/internal/npm_install/test/golden/node_modules/ajv/BUILD.bazel.golden index 3a650c2b57..42c2af409f 100644 --- a/internal/npm_install/test/golden/node_modules/ajv/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/node_modules/ajv/BUILD.bazel.golden @@ -13,7 +13,6 @@ node_module_library( "//node_modules/fast-json-stable-stringify:fast-json-stable-stringify__files", "//node_modules/json-schema-traverse:json-schema-traverse__files", ], - is_apf = False, ) node_module_library( name = "ajv__files", @@ -107,7 +106,9 @@ node_module_library( ":scripts/prepare-tests", ":scripts/travis-gh-pages", ], - is_apf = False, + scripts = [ + + ], ) node_module_library( name = "ajv__typings", diff --git a/internal/npm_install/test/golden/node_modules/jasmine/BUILD.bazel.golden b/internal/npm_install/test/golden/node_modules/jasmine/BUILD.bazel.golden index 2d22bf989f..a09d8b3627 100644 --- a/internal/npm_install/test/golden/node_modules/jasmine/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/node_modules/jasmine/BUILD.bazel.golden @@ -21,7 +21,6 @@ node_module_library( "//node_modules/path-is-absolute:path-is-absolute__files", "//node_modules/jasmine-core:jasmine-core__files", ], - is_apf = False, ) node_module_library( name = "jasmine__files", @@ -41,7 +40,9 @@ node_module_library( ":package.json", ":tasks/jasmine.js", ], - is_apf = False, + scripts = [ + + ], ) node_module_library( name = "jasmine__typings", diff --git a/internal/npm_install/test/golden/node_modules/rxjs/BUILD.bazel.golden b/internal/npm_install/test/golden/node_modules/rxjs/BUILD.bazel.golden index 8404795b48..39bb199cec 100644 --- a/internal/npm_install/test/golden/node_modules/rxjs/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/node_modules/rxjs/BUILD.bazel.golden @@ -10,7 +10,6 @@ node_module_library( deps = [ "//node_modules/tslib:tslib__files", ], - is_apf = False, ) node_module_library( name = "rxjs__files", @@ -3573,7 +3572,9 @@ node_module_library( ":webSocket/index.js.map", ":webSocket/package.json", ], - is_apf = False, + scripts = [ + ":bundles/rxjs.umd.js", + ], ) node_module_library( name = "rxjs__typings", diff --git a/internal/npm_install/test/golden/node_modules/unidiff/BUILD.bazel.golden b/internal/npm_install/test/golden/node_modules/unidiff/BUILD.bazel.golden index cfd7ea5e96..c6bda34b02 100644 --- a/internal/npm_install/test/golden/node_modules/unidiff/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/node_modules/unidiff/BUILD.bazel.golden @@ -10,7 +10,6 @@ node_module_library( deps = [ "//node_modules/diff:diff__files", ], - is_apf = False, ) node_module_library( name = "unidiff__files", @@ -26,7 +25,9 @@ node_module_library( ":test/test_unidiff.js", ":unidiff.js", ], - is_apf = False, + scripts = [ + + ], ) node_module_library( name = "unidiff__typings", diff --git a/internal/npm_install/test/golden/node_modules/zone.js/BUILD.bazel.golden b/internal/npm_install/test/golden/node_modules/zone.js/BUILD.bazel.golden index fdfd80f724..0e1e0ea02f 100644 --- a/internal/npm_install/test/golden/node_modules/zone.js/BUILD.bazel.golden +++ b/internal/npm_install/test/golden/node_modules/zone.js/BUILD.bazel.golden @@ -10,7 +10,6 @@ node_module_library( deps = [ ], - is_apf = False, ) node_module_library( name = "zone.js__files", @@ -144,7 +143,9 @@ node_module_library( ":lib/zone.ts", ":package.json", ], - is_apf = False, + scripts = [ + + ], ) node_module_library( name = "zone.js__typings",