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 17 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
8 changes: 7 additions & 1 deletion e2e/nodejs_image/foolib/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,8 +1,14 @@
load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin")
load("@build_bazel_rules_nodejs//internal/js_library:js_library.bzl", "js_library")

copy_to_bin(
mistic marked this conversation as resolved.
Show resolved Hide resolved
name = "srcs",
srcs = ["index.js"],
)

js_library(
name = "foolib",
package_name = "@foo/lib",
srcs = ["index.js"],
srcs = [":srcs"],
visibility = ["//visibility:public"],
)
186 changes: 109 additions & 77 deletions internal/js_library/js_library.bzl
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Copyright 2017 The Bazel Authors. All rights reserved.
# Copyright 2020 The Bazel Authors. All rights reserved.
mistic marked this conversation as resolved.
Show resolved Hide resolved
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
Expand All @@ -12,13 +12,19 @@
# 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.

DO NOT USE - this is not fully designed, and exists only to enable testing within this repo.
"""Contains the js_library which can be used to expose any library package.
"""

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(
"@build_bazel_rules_nodejs//:providers.bzl",
"DeclarationInfo",
"JSModuleInfo",
"JSNamedModuleInfo",
"LinkablePackageInfo",
"NpmPackageInfo",
"js_module_info",
"js_named_module_info",
)

_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 All @@ -30,9 +36,7 @@ AmdNamesInfo = provider(

def write_amd_names_shim(actions, amd_names_shim, targets):
"""Shim AMD names for UMD bundles that were shipped anonymous.

These are collected from our bootstrap deps (the only place global scripts should appear)

Args:
actions: skylark rule execution context.actions
amd_names_shim: File where the shim is written
Expand All @@ -49,96 +53,124 @@ def write_amd_names_shim(actions, amd_names_shim, targets):
amd_names_shim_content += "define(\"%s\", function() { return %s });\n" % n
actions.write(amd_names_shim, amd_names_shim_content)

def _impl(ctx):
files = []
typings = []
js_files = []
def _js_library_impl(ctx):
direct_sources = depset(ctx.files.srcs)
direct_named_module_sources = depset(ctx.files.named_module_srcs)
sources_depsets = [direct_sources]
named_module_sources_depsets = [direct_named_module_sources]

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)
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)
else:
files.append(dst)
elif src.basename.endswith(".d.ts"):
typings.append(src)
else:
files.append(src)

for p in files:
if p.basename.endswith(".js") or p.basename.endswith(".js.map") or p.basename.endswith(".json"):
js_files.append(p)

files_depset = depset(files)
if src.is_source and src.path.startswith("external/"):
include_npm_package_info = True
mistic marked this conversation as resolved.
Show resolved Hide resolved
break

declarations = depset([
f
for f in ctx.files.srcs
if (
f.path.endswith(".d.ts") or
# package.json may be required to resolve "typings" key
f.path.endswith("/package.json")
mistic marked this conversation as resolved.
Show resolved Hide resolved
) 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(f.path.split("/node_modules/")) < 3 and f.path.find("/node_modules/typescript/lib/lib.") == -1
])

transitive_declarations_depsets = [declarations]

for dep in ctx.attr.deps:
mistic marked this conversation as resolved.
Show resolved Hide resolved
if DeclarationInfo in dep:
transitive_declarations_depsets.append(dep[DeclarationInfo].transitive_declarations)
if NpmPackageInfo in dep:
sources_depsets.append(dep[NpmPackageInfo].sources)
if JSModuleInfo in dep:
sources_depsets.append(dep[JSModuleInfo].sources)
if JSNamedModuleInfo in dep:
named_module_sources_depsets.append(dep[JSNamedModuleInfo].sources)

transitive_declarations = depset(transitive = transitive_declarations_depsets)
transitive_sources = depset(transitive = sources_depsets + named_module_sources_depsets)

providers = [
DefaultInfo(
files = files_depset,
runfiles = ctx.runfiles(files = ctx.files.srcs),
files = direct_sources,
),
DeclarationInfo(
mistic marked this conversation as resolved.
Show resolved Hide resolved
declarations = declarations,
transitive_declarations = transitive_declarations,
type_blacklisted_declarations = depset([]),
),
js_module_info(
sources = direct_sources,
deps = ctx.attr.deps,
),
js_named_module_info(
sources = direct_named_module_sources,
deps = ctx.attr.deps,
),
AmdNamesInfo(names = ctx.attr.amd_names),
js_module_info(depset(js_files)),
]

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_sources,
transitive_declarations,
]),
))

# 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)))
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 = direct_sources,
sources = transitive_sources,
workspace = workspace_name,
))

return providers
return struct(
typescript = struct(
mistic marked this conversation as resolved.
Show resolved Hide resolved
declarations = declarations,
devmode_manifest = None,
es5_sources = depset(),
es6_sources = depset(),
replay_params = None,
transitive_declarations = transitive_declarations,
transitive_es5_sources = depset(),
transitive_es6_sources = depset(),
tsickle_externs = [],
type_blacklisted_declarations = depset(),
),
providers = providers,
)

_js_library = rule(
implementation = _impl,
js_library = rule(
implementation = _js_library_impl,
attrs = {
"amd_names": attr.string_dict(doc = _AMD_NAMES_DOC),
"is_windows": attr.bool(mandatory = True, doc = "Automatically set by macro"),
# module_name for legacy ts_library module_mapping support
# TODO: remove once legacy module_mapping is removed
"module_name": attr.string(),
"package_name": attr.string(),
"amd_names": attr.string_dict(
doc = _AMD_NAMES_DOC,
default = {},
),
"deps": attr.label_list(
doc = "Transitive dependencies of the package",
mistic marked this conversation as resolved.
Show resolved Hide resolved
),
"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",
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",
allow_files = True,
mandatory = True,
),
},
doc = "Defines a js library package",
)

def js_library(
name,
srcs,
amd_names = {},
package_name = None,
**kwargs):
"""Internal use only. May be published to the 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,
package_name = package_name,
# module_name for legacy ts_library module_mapping support
# TODO: remove once legacy module_mapping is removed
module_name = package_name,
is_windows = select({
"@bazel_tools//src/conditions:host_windows": True,
"//conditions:default": False,
}),
**kwargs
)
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin")
load("//internal/js_library:js_library.bzl", "js_library")

package(default_visibility = ["//internal/linker/test:__subpackages__"])

copy_to_bin(
name = "srcs",
srcs = ["index.js"],
)

js_library(
name = "dynamic_linked_pkg",
package_name = "dynamic_linked",
srcs = ["index.js"],
srcs = [":srcs"],
)
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin")
load("//internal/js_library:js_library.bzl", "js_library")

package(default_visibility = ["//internal/linker/test:__subpackages__"])

copy_to_bin(
name = "srcs",
srcs = ["index.js"],
)

js_library(
name = "dynamic_linked_scoped_pkg",
package_name = "@linker_scoped/dynamic_linked",
srcs = ["index.js"],
srcs = [":srcs"],
)
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin")
load("//internal/js_library:js_library.bzl", "js_library")

package(default_visibility = ["//internal/linker/test:__subpackages__"])

copy_to_bin(
name = "srcs",
srcs = ["index.js"],
)

js_library(
name = "static_linked_scoped_pkg",
package_name = "@linker_scoped/static_linked",
srcs = ["index.js"],
srcs = [":srcs"],
)
Original file line number Diff line number Diff line change
@@ -1,9 +1,15 @@
load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin")
load("//internal/js_library:js_library.bzl", "js_library")

package(default_visibility = ["//internal/linker/test:__subpackages__"])

copy_to_bin(
name = "srcs",
srcs = ["index.js"],
)

js_library(
name = "transitive_static_linked_pkg",
package_name = "transitive_static_linked",
srcs = ["index.js"],
srcs = [":srcs"],
)
9 changes: 7 additions & 2 deletions internal/node/test/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
load("@build_bazel_rules_nodejs//:index.bzl", "generated_file_test", "nodejs_binary", "nodejs_test", "npm_package_bin")
load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin", "generated_file_test", "nodejs_binary", "nodejs_test", "npm_package_bin")
load("@npm//typescript:index.bzl", "tsc")
load("//internal/js_library:js_library.bzl", "js_library")
load("//internal/node:node_repositories.bzl", "BUILT_IN_NODE_PLATFORMS")
Expand Down Expand Up @@ -57,10 +57,15 @@ nodejs_binary(
entry_point = ":entry_file",
)

copy_to_bin(
name = "module-name",
srcs = ["module-name.js"],
)

js_library(
name = "module_name_lib",
package_name = "build_bazel_rules_nodejs/internal/node/test",
srcs = ["module-name.js"],
srcs = [":module-name"],
)

nodejs_binary(
Expand Down
12 changes: 9 additions & 3 deletions internal/node/test/lib1/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,13 +1,19 @@
load("@build_bazel_rules_nodejs//:index.bzl", "copy_to_bin")
load("//internal/js_library:js_library.bzl", "js_library")

package(default_visibility = ["//internal:__subpackages__"])

js_library(
name = "lib1",
package_name = "lib1",
copy_to_bin(
name = "srcs",
srcs = [
"package.json",
"src/index.js",
"src/some.js",
],
)

js_library(
name = "lib1",
package_name = "lib1",
srcs = [":srcs"],
)
Loading