Fixes for npm_package: (#198)

* Fixes for npm_package:

- Allow source files to appear under deps, like .d.ts srcs of a ts_library
- Include ts_library transitive_declarations
- Include data attributes of ts_library

Fixes #196
Fixes #197

* Add tests for npm_package fixes

internal/common/typescript_mock_lib.bzl

@@ -14,19 +14,29 @@
 
 """A mock typescript_lib rule.
 
-Allows testing that node_jasmine_test will work with ts_library from
-rules_typescript without introducing a circular dependency.
+Allows testing that jasmine_node_test will work with ts_library from
+rules_typescript without introducing a circular dependency between
+rules_nodejs and rules_typescript repositories.
 """
 
 def _mock_typescript_lib(ctx):
   es5_sources = depset()
+  transitive_decls = depset()
   for s in ctx.attr.srcs:
-    es5_sources = depset(transitive=[es5_sources, s.files])
-  return struct(typescript = struct(es5_sources = es5_sources))
+    es5_sources = depset([f for f in s.files if f.path.endswith(".js")], transitive = [es5_sources])
+    transitive_decls = depset([f for f in s.files if f.path.endswith(".d.ts")], transitive = [transitive_decls])
+  return struct(
+      runfiles = ctx.runfiles(collect_default=True, collect_data = True),
+      typescript = struct(
+          es5_sources = es5_sources,
+          transitive_declarations = transitive_decls
+      ),
+  )
 
 mock_typescript_lib = rule(
   implementation = _mock_typescript_lib,
   attrs = {
     "srcs": attr.label_list(allow_files = True),
+    "data": attr.label_list(allow_files = True, cfg = "data"),
   }
 )

internal/npm_package/npm_package.bzl

@@ -58,7 +58,20 @@ def create_package(ctx, devmode_sources, nested_packages):
 def _npm_package(ctx):
   files = depset()
   for d in ctx.attr.deps:
-    files = depset(transitive = [files, d.files, d.node_sources])
+    transitive = [
+        files,
+        # Collect whatever is in the "data"
+        d.data_runfiles.files,
+        # For JavaScript-producing rules, gather up the devmode Node.js sources
+        d.node_sources,
+    ]
+
+    # ts_library doesn't include .d.ts outputs in the runfiles
+    # see comment in rules_typescript/internal/common/compilation.bzl
+    if hasattr(d, "typescript"):
+      transitive.append(d.typescript.transitive_declarations)
+
+    files = depset(transitive = transitive)
 
   package_dir = create_package(ctx, files.to_list(), ctx.files.packages)
 

internal/npm_package/packager.js

@@ -81,7 +81,13 @@ function main(args) {
     } else if (!path.relative(genDir, f).startsWith('..')) {
       rootDir = genDir;
     } else {
-      throw new Error(`dependency ${f} is not under bazel-bin or bazel-genfiles`);
+      // It might be nice to enforce here that deps don't contain sources
+      // since those belong in srcs above.
+      // The `deps` attribute should typically be outputs of other rules.
+      // However, things like .d.ts sources of a ts_library or data attributes
+      // of ts_library will result in source files that appear in the deps
+      // so we have to allow this.
+      rootDir = '.';
     }
     return path.join(outDir, path.relative(path.join(rootDir, baseDir), f));
   }

internal/npm_package/test/BUILD.bazel

@@ -1,11 +1,21 @@
 load("//:defs.bzl", "npm_package", "jasmine_node_test")
+load("//internal/common:typescript_mock_lib.bzl", "mock_typescript_lib")
 
 genrule(
     name = "produces_files",
     outs = ["a_dep"],
     cmd = "echo \"a_dep content\" > $@",
 )
 
+mock_typescript_lib(
+    name = "ts_library",
+    srcs = [
+        "foo.d.ts",
+        "foo.js",
+    ],
+    data = ["data.json"],
+)
+
 npm_package(
     name = "dependent_pkg",
     srcs = ["dependent_file"],
@@ -16,7 +26,10 @@ npm_package(
     srcs = ["some_file"],
     packages = [":dependent_pkg"],
     replacements = {"replace_me": "replaced"},
-    deps = [":produces_files"],
+    deps = [
+        ":produces_files",
+        ":ts_library",
+    ],
 )
 
 jasmine_node_test(

internal/npm_package/test/data.json

@@ -0,0 +1 @@
+[]

internal/npm_package/test/foo.d.ts

@@ -0,0 +1 @@
+export const a: string;

internal/npm_package/test/foo.js

@@ -0,0 +1 @@
+export const a = '';

internal/npm_package/test/npm_package.spec.js

@@ -21,4 +21,10 @@ describe('npm_package srcs', () => {
   it('copies files from other packages', () => {
     expect(read('dependent_file')).toEqual('dependent_file content');
   });
+  it('copies declaration files from ts_library', () => {
+    expect(read('foo.d.ts')).toEqual('export const a: string;');
+  });
+  it('copies data dependencies', () => {
+    expect(read('data.json')).toEqual('[]');
+  });
 });