Skip to content

Commit

Permalink
feat(builtin): add DeclarationInfo to js_library
Browse files Browse the repository at this point in the history
Adds a declaration_info provider factory which simplifies ts_project. Also add a ts_project test to validate that js_library be a dep to ts_project.

BREAKING CHANGE:

Removed provide_declarations() factory function for DeclarationInfo. Use declaration_info() factory function instead.
  • Loading branch information
gregmagolan authored and alexeagle committed Jun 9, 2020
1 parent b120627 commit 2b89f32
Show file tree
Hide file tree
Showing 9 changed files with 76 additions and 30 deletions.
21 changes: 16 additions & 5 deletions internal/js_library/js_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
DO NOT USE - this is not fully designed, and exists only to enable testing within this repo.
"""

load("//:providers.bzl", "LinkablePackageInfo")
load("//:providers.bzl", "LinkablePackageInfo", "declaration_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.
Expand Down Expand Up @@ -51,6 +51,7 @@ def write_amd_names_shim(actions, amd_names_shim, targets):

def _impl(ctx):
files = []
typings = []

for src in ctx.files.srcs:
if src.is_source and not src.path.startswith("external/"):
Expand All @@ -59,13 +60,18 @@ def _impl(ctx):
copy_cmd(ctx, src, dst)
else:
copy_bash(ctx, src, dst)
files.append(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)

files_depset = depset(files)

result = [
providers = [
DefaultInfo(
files = files_depset,
runfiles = ctx.runfiles(files = ctx.files.srcs),
Expand All @@ -75,13 +81,18 @@ def _impl(ctx):

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

return result
# 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)))

return providers

_js_library = rule(
implementation = _impl,
Expand Down
29 changes: 19 additions & 10 deletions internal/providers/declaration_info.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -14,20 +14,11 @@

"""This module contains a provider for TypeScript typings files (.d.ts)"""

def provide_declarations(**kwargs):
"""Factory function for creating checked declarations with externs.
Do not directly construct DeclarationInfo()
"""

# TODO: add some checking actions to ensure the declarations are well-formed
return DeclarationInfo(**kwargs)

DeclarationInfo = provider(
doc = """The DeclarationInfo provider allows JS rules to communicate typing information.
TypeScript's .d.ts files are used as the interop format for describing types.
Do not create DeclarationInfo instances directly, instead use the provide_declarations factory function.
Do not create DeclarationInfo instances directly, instead use the declaration_info factory function.
TODO(alexeagle): The ts_library#deps attribute should require that this provider is attached.
Expand All @@ -41,3 +32,21 @@ This prevents needing an aspect in rules that consume the typings, which improve
"type_blacklisted_declarations": """A depset of .d.ts files that we should not use to infer JSCompiler types (via tsickle)""",
},
)

def declaration_info(declarations, deps = []):
"""Constructs a DeclarationInfo including all transitive declarations from DeclarationInfo providers in a list of deps.
Returns a single DeclarationInfo.
"""
transitive_depsets = [declarations]
for dep in deps:
if DeclarationInfo in dep:
transitive_depsets.append(dep[DeclarationInfo].transitive_declarations)

return DeclarationInfo(
declarations = declarations,
transitive_declarations = depset(transitive = transitive_depsets),
# Downstream ts_library rules will fail if they don't find this field
# Even though it is only for Google Closure Compiler externs generation
type_blacklisted_declarations = depset(),
)
15 changes: 2 additions & 13 deletions packages/typescript/internal/ts_project.bzl
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
"ts_project rule"

load("@build_bazel_rules_nodejs//:providers.bzl", "DeclarationInfo", "NpmPackageInfo", "run_node")
load("@build_bazel_rules_nodejs//:providers.bzl", "DeclarationInfo", "NpmPackageInfo", "declaration_info", "run_node")

_DEFAULT_TSC = (
# BEGIN-INTERNAL
Expand Down Expand Up @@ -138,18 +138,7 @@ def _ts_project_impl(ctx):
# Don't provide DeclarationInfo if there are no typings to provide.
# Improves error messaging if a ts_project needs declaration = True
if len(typings_outputs) or len(ctx.attr.deps):
providers.append(
DeclarationInfo(
declarations = depset(typings_outputs),
transitive_declarations = depset(typings_outputs, transitive = [
dep[DeclarationInfo].transitive_declarations
for dep in ctx.attr.deps
]),
# Downstream ts_library rules will fail if they don't find this field
# Even though it is only for Google Closure Compiler externs generation
type_blacklisted_declarations = depset(),
),
)
providers.append(declaration_info(depset(typings_outputs), ctx.attr.deps))

return providers

Expand Down
16 changes: 16 additions & 0 deletions packages/typescript/test/ts_project/js_library/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
load("@npm_bazel_typescript//:index.bzl", "ts_project")
load("//internal/js_library:js_library.bzl", "js_library")

js_library(
name = "lib_a",
srcs = [
"a.d.ts",
"a.js",
],
)

ts_project(
name = "tsconfig",
srcs = ["b.ts"],
deps = ["lib_a"],
)
1 change: 1 addition & 0 deletions packages/typescript/test/ts_project/js_library/a.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export declare const a: string;
3 changes: 3 additions & 0 deletions packages/typescript/test/ts_project/js_library/a.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
'use strict';
exports.__esModule = true;
exports.a = 'a';
2 changes: 2 additions & 0 deletions packages/typescript/test/ts_project/js_library/b.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
import {a} from './a';
console.log(a.toLowerCase());
15 changes: 15 additions & 0 deletions packages/typescript/test/ts_project/js_library/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,15 @@
{
"files": ["b.ts"],
"compilerOptions": {
// Help TypeScript locate the a.d.ts file from dep. Needed when running in a sandbox or remote.
"rootDirs": [
".",
"../../../../../bazel-out/darwin-fastbuild/bin/packages/typescript/test/ts_project/js_library",
"../../../../../bazel-out/k8-fastbuild/bin/packages/typescript/test/ts_project/js_library",
"../../../../../bazel-out/x64_windows-fastbuild/bin/packages/typescript/test/ts_project/js_library",
"../../../../../bazel-out/darwin-dbg/bin/packages/typescript/test/ts_project/js_library",
"../../../../../bazel-out/k8-dbg/bin/packages/typescript/test/ts_project/js_library",
"../../../../../bazel-out/x64_windows-dbg/bin/packages/typescript/test/ts_project/js_library",
],
}
}
4 changes: 2 additions & 2 deletions providers.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ Users should not load files under "/internal"
load(
"//internal/providers:declaration_info.bzl",
_DeclarationInfo = "DeclarationInfo",
_provide_declarations = "provide_declarations",
_declaration_info = "declaration_info",
)
load(
"//internal/providers:js_providers.bzl",
Expand All @@ -44,8 +44,8 @@ load(
_node_modules_aspect = "node_modules_aspect",
)

provide_declarations = _provide_declarations
DeclarationInfo = _DeclarationInfo
declaration_info = _declaration_info
JSNamedModuleInfo = _JSNamedModuleInfo
js_named_module_info = _js_named_module_info
JSEcmaScriptModuleInfo = _JSEcmaScriptModuleInfo
Expand Down

0 comments on commit 2b89f32

Please sign in to comment.