Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(builtin): new js_library rule #2109

Merged
merged 47 commits into from
Aug 17, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
1daa8bb
feat(typescript): include module_mappings_aspect on ts_project deps
mistic Aug 12, 2020
3f21290
chore(builtin): remove old js_library rule
mistic Aug 12, 2020
a05de59
chore(builtin): rename node_module_library rule into new js_library rule
mistic Aug 12, 2020
b55aa4a
feat(builtin): new js_library rule proposal implementation
mistic Aug 12, 2020
a508486
feat(builtin): include old amd_names from legacy js_library
mistic Aug 13, 2020
14a706f
refactor(builtin): update usages of js_library to use targets from co…
mistic Aug 13, 2020
00943c4
refactor(builtin): update usages of node_module_library into the new …
mistic Aug 13, 2020
3bba742
fix(builtin): wrong import of copy_to_bin bzl file
mistic Aug 13, 2020
ba7deb3
fix(builtin): wrong import of copy_to_bin bzl file. Now importing fro…
mistic Aug 13, 2020
ff7c268
fix(builtin): correctly setup copy_to_bin on gjtorikian/isBinaryFile …
mistic Aug 13, 2020
c38b2ef
chore(builtin): lint fix the entire code base
mistic Aug 13, 2020
1962095
chore(builtin): lint fix the entire code base for imports
mistic Aug 13, 2020
11acd00
fix(builtin): gjtorikian/isBinaryFile to use deps composition on the …
mistic Aug 13, 2020
9ec1861
fix(builtin): js_library usage on grpc_web/BUILD.bazel
mistic Aug 13, 2020
622c9cc
fix(builtin): js_library usage on grpc_web/BUILD.bazel
mistic Aug 13, 2020
0b0bf78
test(builtin): fix js_binary usages in a couple of places across tests
mistic Aug 13, 2020
68d8531
chore(builtin): run lint fix
mistic Aug 13, 2020
81faea0
chore(builtin): include some changes according PR review
mistic Aug 13, 2020
4f516f5
feat(builtin): only include declarationinfo provider in case there ar…
mistic Aug 13, 2020
b806e17
fix(builtin): remove legacy string-typed typescript provider
mistic Aug 13, 2020
580c947
chore(builtin): include some changes according PR review
mistic Aug 13, 2020
a65fb7e
fix(builtin): typo when appending declarationinfo provider
mistic Aug 13, 2020
11d2983
fix(builtin): isBinaryFile js_library usage
mistic Aug 13, 2020
d6e1907
refactor(builtin): remove non needed provider filtering on the rule d…
mistic Aug 14, 2020
1be5fa4
refactor(builtin): remove non needed provider filtering on the rule d…
mistic Aug 14, 2020
269d303
refactor(builtin): remove non needed provider filtering on the rule d…
mistic Aug 14, 2020
f57b958
refactor(builtin): update files on linkable package info
mistic Aug 14, 2020
1e1391e
refactor(builtin): update files on linkable package info
mistic Aug 14, 2020
e35955f
refactor(builtin): simplify providers on js_library
mistic Aug 14, 2020
fbd2034
refactor(builtin): reworked implementation of js_library
mistic Aug 14, 2020
3cc1fec
chore(builtin): apply lint fix to all changes rules
mistic Aug 14, 2020
36c502a
fix(builtin): remove bad import for js_library
mistic Aug 14, 2020
10b2e67
fix(builtin): declare deps on isBinaryFile as internal only as it imp…
mistic Aug 14, 2020
18c4ad9
chore(builtin): remove unused import on isBinaryFile package
mistic Aug 15, 2020
017f19a
chore(builtin): remove unused deps from isBinaryFile package
mistic Aug 15, 2020
daaa6cb
docs(builtin): complete documentation for js_library
mistic Aug 15, 2020
0880d88
docs(builtin): complete documentation for js_library arguments
mistic Aug 15, 2020
8a9920a
chore(builtin): fixed linting problems on js_library
mistic Aug 15, 2020
03e7fe3
feat(builtin): include sources from deps that are not node_modules
mistic Aug 15, 2020
28d7ed0
feat(builtin): recover old behaviour of auto copy into bin
mistic Aug 15, 2020
f67b293
chore(builtin): reorganize imports
mistic Aug 15, 2020
ba2e13c
feat(builtin): include every file in the list of files depsets
mistic Aug 17, 2020
97ebf68
refactor(builtin): don't rely on extra src variable to copy files int…
mistic Aug 17, 2020
9ae2712
docs(builtin): add note about include_npm_package_info check
mistic Aug 17, 2020
98599f8
chore(builtin): remove every copy_to_bin rule previously added
mistic Aug 17, 2020
b590bfa
fix(builtin): workaround for examples/user_managed_deps failing test
mistic Aug 17, 2020
0b54193
docs(builtin): explain why we need one copy_to_bin left
mistic Aug 17, 2020
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions WORKSPACE
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,9 @@ yarn_install(
# The @npm//:node_modules_filegroup generated by manual_build_file_contents
# is used in the //packages/typescript/test/reference_types_directive:tsconfig_types
# test. For now we're still supporting node_modules as a filegroup tho this may
# change in the future. The default generated //:node_modules target is a node_module_library
# rule which provides NpmPackageInfo but that rule is not yet in the public API and we
# have not yet dropped support for filegroup based node_modules target.
# change in the future. The default generated //:node_modules target is a js_library
# rule which provides NpmPackageInfo but we have not yet dropped support for
# filegroup based node_modules target.
manual_build_file_contents = """
filegroup(
name = "node_modules_filegroup",
Expand Down
195 changes: 160 additions & 35 deletions internal/js_library/js_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -12,13 +12,27 @@
# See the License for the specific language governing permissions and
# limitations under the License.

"""js_library allows defining a set of javascript sources and assigning a package_name.
"""js_library can be used to expose and share any library package.

DO NOT USE - this is not fully designed, and exists only to enable testing within this repo.
DO NOT USE - this is not fully designed yet and it is a work in progress.
"""

load("//:providers.bzl", "LinkablePackageInfo", "declaration_info", "js_module_info")
load("//third_party/github.com/bazelbuild/bazel-skylib:rules/private/copy_file_private.bzl", "copy_bash", "copy_cmd")
load(
"//:providers.bzl",
"DeclarationInfo",
"JSModuleInfo",
"JSNamedModuleInfo",
"LinkablePackageInfo",
"NpmPackageInfo",
"declaration_info",
"js_module_info",
"js_named_module_info",
)
load(
"//third_party/github.com/bazelbuild/bazel-skylib:rules/private/copy_file_private.bzl",
"copy_bash",
"copy_cmd",
)

_AMD_NAMES_DOC = """Mapping from require module names to global variables.
This allows devmode JS sources to load unnamed UMD bundles from third-party libraries."""
Expand Down Expand Up @@ -50,90 +64,201 @@ def write_amd_names_shim(actions, amd_names_shim, targets):
actions.write(amd_names_shim, amd_names_shim_content)

def _impl(ctx):
files = []
input_files = ctx.files.srcs + ctx.files.named_module_srcs
all_files = []
typings = []
js_files = []
named_module_files = []
include_npm_package_info = False

for src in ctx.files.srcs:
if src.is_source and not src.path.startswith("external/"):
dst = ctx.actions.declare_file(src.basename, sibling = src)
for idx, f in enumerate(input_files):
file = f

# copy files into bin if needed
if file.is_source and not file.path.startswith("external/"):
dst = ctx.actions.declare_file(file.basename, sibling = file)
if ctx.attr.is_windows:
copy_cmd(ctx, src, dst)
else:
copy_bash(ctx, src, dst)
if dst.basename.endswith(".d.ts"):
typings.append(dst)
copy_cmd(ctx, file, dst)
else:
files.append(dst)
elif src.basename.endswith(".d.ts"):
typings.append(src)
else:
files.append(src)
copy_bash(ctx, file, dst)

# re-assign file to the one now copied into the bin folder
file = dst

# register js files
if file.basename.endswith(".js") or file.basename.endswith(".js.map") or file.basename.endswith(".json"):
mistic marked this conversation as resolved.
Show resolved Hide resolved
js_files.append(file)

# register typings
if (
(
file.path.endswith(".d.ts") or
file.path.endswith(".d.ts.map") or
# package.json may be required to resolve "typings" key
file.path.endswith("/package.json")
) and
# exclude eg. external/npm/node_modules/protobufjs/node_modules/@types/node/index.d.ts
# these would be duplicates of the typings provided directly in another dependency.
# also exclude all /node_modules/typescript/lib/lib.*.d.ts files as these are determined by
# the tsconfig "lib" attribute
len(file.path.split("/node_modules/")) < 3 and file.path.find("/node_modules/typescript/lib/lib.") == -1
):
typings.append(file)

for p in files:
if p.basename.endswith(".js") or p.basename.endswith(".js.map") or p.basename.endswith(".json"):
js_files.append(p)
# auto detect if it entirely an npm package
#
# NOTE: it probably can be removed once we support node_modules from more than
# a single workspace
if file.is_source and file.path.startswith("external/"):
# We cannot always expose the NpmPackageInfo as the linker
# only allow us to reference node modules from a single workspace at a time.
# Here we are automatically decide if we should or not including that provider
# by running through the sources and check if we have a src coming from an external
# workspace which indicates we should include the provider.
include_npm_package_info = True

files_depset = depset(files)
# ctx.files.named_module_srcs are merged after ctx.files.srcs
if idx >= len(ctx.files.srcs):
named_module_files.append(file)

# every single file on bin should be added here
all_files.append(file)

files_depset = depset(all_files)
js_files_depset = depset(js_files)
named_module_files_depset = depset(named_module_files)
typings_depset = depset(typings)

files_depsets = [files_depset]
npm_sources_depsets = [files_depset]
direct_sources_depsets = [files_depset]
direct_named_module_sources_depsets = [named_module_files_depset]
typings_depsets = [typings_depset]
js_files_depsets = [js_files_depset]

for dep in ctx.attr.deps:
mistic marked this conversation as resolved.
Show resolved Hide resolved
if NpmPackageInfo in dep:
npm_sources_depsets.append(dep[NpmPackageInfo].sources)
else:
if JSModuleInfo in dep:
js_files_depsets.append(dep[JSModuleInfo].direct_sources)
direct_sources_depsets.append(dep[JSModuleInfo].direct_sources)
if JSNamedModuleInfo in dep:
direct_named_module_sources_depsets.append(dep[JSNamedModuleInfo].direct_sources)
direct_sources_depsets.append(dep[JSNamedModuleInfo].direct_sources)
if DeclarationInfo in dep:
typings_depsets.append(dep[DeclarationInfo].declarations)
direct_sources_depsets.append(dep[DeclarationInfo].declarations)
if DefaultInfo in dep:
files_depsets.append(dep[DefaultInfo].files)

providers = [
DefaultInfo(
files = files_depset,
runfiles = ctx.runfiles(files = ctx.files.srcs),
files = depset(transitive = files_depsets),
runfiles = ctx.runfiles(
files = all_files,
transitive_files = depset(transitive = files_depsets),
),
),
AmdNamesInfo(names = ctx.attr.amd_names),
js_module_info(depset(js_files)),
js_module_info(
sources = depset(transitive = js_files_depsets),
deps = ctx.attr.deps,
),
js_named_module_info(
sources = depset(transitive = direct_named_module_sources_depsets),
deps = ctx.attr.deps,
),
]

if ctx.attr.package_name:
path = "/".join([p for p in [ctx.bin_dir.path, ctx.label.workspace_root, ctx.label.package] if p])
providers.append(LinkablePackageInfo(
package_name = ctx.attr.package_name,
path = path,
files = files_depset,
files = depset(transitive = direct_sources_depsets),
))

if include_npm_package_info:
workspace_name = ctx.label.workspace_name if ctx.label.workspace_name else ctx.workspace_name
providers.append(NpmPackageInfo(
direct_sources = depset(transitive = direct_sources_depsets),
sources = depset(transitive = npm_sources_depsets),
workspace = workspace_name,
))

# Don't provide DeclarationInfo if there are no typings to provide.
# Improves error messaging downstream if DeclarationInfo is required.
if len(typings):
providers.append(declaration_info(depset(typings)))
providers.append(declaration_info(
declarations = depset(transitive = typings_depsets),
deps = ctx.attr.deps,
))

return providers

_js_library = rule(
implementation = _impl,
attrs = {
"amd_names": attr.string_dict(doc = _AMD_NAMES_DOC),
"is_windows": attr.bool(mandatory = True, doc = "Automatically set by macro"),
"amd_names": attr.string_dict(
doc = _AMD_NAMES_DOC,
),
"deps": attr.label_list(
doc = """Transitive dependencies of the package.
It should include fine grained npm dependencies from the sources
or other targets we want to include in the library but also propagate their own deps.""",
),
"is_windows": attr.bool(
doc = "Automatically set by macro",
mandatory = True,
),
# module_name for legacy ts_library module_mapping support
# which is still being used in a couple of tests
# TODO: remove once legacy module_mapping is removed
"module_name": attr.string(),
"package_name": attr.string(),
"module_name": attr.string(
doc = "Internal use only. It will be removed soon.",
),
"named_module_srcs": attr.label_list(
doc = """A subset of srcs that are javascript named-UMD or
named-AMD for use in rules such as ts_devserver.
They will be copied into the package bin folder if needed.""",
allow_files = True,
),
"package_name": attr.string(
doc = """Optional package_name that this package may be imported as.""",
),
"srcs": attr.label_list(
doc = """The list of files that comprise the package.
They will be copied into the package bin folder if needed.""",
allow_files = True,
mandatory = True,
),
},
doc = "Defines a js_library package",
)

def js_library(
name,
srcs,
srcs = [],
amd_names = {},
package_name = None,
deps = [],
named_module_srcs = [],
**kwargs):
"""Internal use only. May be published to the public API in a future release."""
"""Internal use only yet. It will be released into a public API in a future release."""
module_name = kwargs.pop("module_name", None)
if module_name:
fail("use package_name instead of module_name in target //%s:%s" % (native.package_name(), name))
if kwargs.pop("is_windows", None):
fail("is_windows is set by the js_library macro and should not be set explicitely")
_js_library(
name = name,
srcs = srcs,
amd_names = amd_names,
srcs = srcs,
named_module_srcs = named_module_srcs,
deps = deps,
package_name = package_name,
# module_name for legacy ts_library module_mapping support
# which is still being used in a couple of tests
# TODO: remove once legacy module_mapping is removed
module_name = package_name,
is_windows = select({
Expand Down
4 changes: 3 additions & 1 deletion internal/node/test/lib2/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,10 @@ js_library(
name = "lib2",
package_name = "lib2",
srcs = [
"main.js",
"package.json",
"src/some.js",
],
deps = [
":tsconfig",
],
)
27 changes: 13 additions & 14 deletions internal/npm_install/generate_build_file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
/**
* @fileoverview This script generates BUILD.bazel files by analyzing
* the node_modules folder layed out by yarn or npm. It generates
* fine grained Bazel `node_module_library` targets for each root npm package
* fine grained Bazel `js_library` targets for each root npm package
* and all files for that package and its transitive deps are included
* in the target. For example, `@<workspace>//jasmine` would
* include all files in the jasmine npm package and all of its
Expand All @@ -28,7 +28,7 @@
* target will be generated for the `jasmine` binary in the `jasmine`
* npm package.
*
* Additionally, a `@<workspace>//:node_modules` `node_module_library`
* Additionally, a `@<workspace>//:node_modules` `js_library`
* is generated that includes all packages under node_modules
* as well as the .bin folder.
*
Expand Down Expand Up @@ -158,16 +158,16 @@ function generateRootBuildFile(pkgs: Dep[]) {
})});

let buildFile = BUILD_FILE_HEADER +
`load("@build_bazel_rules_nodejs//internal/npm_install:node_module_library.bzl", "node_module_library")
`load("@build_bazel_rules_nodejs//internal/js_library:js_library.bzl", "js_library")

exports_files([
${exportsStarlark}])

# The node_modules directory in one catch-all node_module_library.
# The node_modules directory in one catch-all js_library.
# NB: Using this target may have bad performance implications if
# there are many files in target.
# See https://github.com/bazelbuild/bazel/issues/5153.
node_module_library(
js_library(
name = "node_modules",${pkgFilesStarlark}${depsStarlark}
)

Expand Down Expand Up @@ -861,7 +861,7 @@ function findFile(pkg: Dep, m: string) {
}

/**
* Given a pkg, return the skylark `node_module_library` targets for the package.
* Given a pkg, return the skylark `js_library` targets for the package.
*/
function printPackage(pkg: Dep) {
function starlarkFiles(attr: string, files: string[], comment: string = '') {
Expand Down Expand Up @@ -919,8 +919,7 @@ function printPackage(pkg: Dep) {
const depsStarlark =
deps.map(dep => `"//${dep._dir}:${dep._name}__contents",`).join('\n ');

let result =
`load("@build_bazel_rules_nodejs//internal/npm_install:node_module_library.bzl", "node_module_library")
let result = `load("@build_bazel_rules_nodejs//internal/js_library:js_library.bzl", "js_library")

# Generated targets for npm package "${pkg._dir}"
${printJson(pkg)}
Expand Down Expand Up @@ -955,7 +954,7 @@ filegroup(
)

# The primary target for this package for use in rule deps
node_module_library(
js_library(
name = "${pkg._name}",
# direct sources listed for strict deps support
srcs = [":${pkg._name}__files"],
Expand All @@ -967,14 +966,14 @@ node_module_library(
)

# Target is used as dep for main targets to prevent circular dependencies errors
node_module_library(
js_library(
name = "${pkg._name}__contents",
srcs = [":${pkg._name}__files", ":${pkg._name}__nested_node_modules"],${namedSourcesStarlark}
visibility = ["//:__subpackages__"],
)

# Typings files that are part of the npm package not including nested node_modules
node_module_library(
js_library(
name = "${pkg._name}__typings",${dtsStarlark}
)

Expand Down Expand Up @@ -1137,7 +1136,7 @@ type Dep = {
}

/**
* Given a scope, return the skylark `node_module_library` target for the scope.
* Given a scope, return the skylark `js_library` target for the scope.
*/
function printScope(scope: string, pkgs: Dep[]) {
pkgs = pkgs.filter(pkg => !pkg._isNested && pkg._dir.startsWith(`${scope}/`));
Expand Down Expand Up @@ -1168,10 +1167,10 @@ function printScope(scope: string, pkgs: Dep[]) {
],`;
}

return `load("@build_bazel_rules_nodejs//internal/npm_install:node_module_library.bzl", "node_module_library")
return `load("@build_bazel_rules_nodejs//internal/js_library:js_library.bzl", "js_library")

# Generated target for npm scope ${scope}
node_module_library(
js_library(
name = "${scope}",${pkgFilesStarlark}${depsStarlark}
)

Expand Down
Loading