fix(ts_proto_library): require explicit srcs to copy (#617)

* Example on bazel failing when 2 ts_proto_library are in a single BUILD file error: `Error in directory_path: directory_path rule '_logger_pb.d.ts_dirpath' in package 'examples/proto_grpc' conflicts with existing directory_path rule` * fix: explicit files_to_copy for multiple ts_proto_lib * fix(ts_proto_library): require explicit srcs to copy The hack of using a glob over the srcs causes a collision when there is more than one ts_proto_library in a bazel package. The resulting error doesn't give a clue what fix is required. * chore: docgen * fix: docs --------- Co-authored-by: egorm <[email protected]>
aspect-build · Jun 3, 2024 · 50ff189 · 50ff189
1 parent 21b41be
commit 50ff189
Show file tree

Hide file tree

Showing 7 changed files with 114 additions and 9 deletions.
diff --git a/docs/proto.md b/docs/proto.md
@@ -48,7 +48,7 @@ Future work
 
 <pre>
 ts_proto_library(<a href="#ts_proto_library-name">name</a>, <a href="#ts_proto_library-node_modules">node_modules</a>, <a href="#ts_proto_library-gen_connect_es">gen_connect_es</a>, <a href="#ts_proto_library-gen_connect_query">gen_connect_query</a>,
-                 <a href="#ts_proto_library-gen_connect_query_service_mapping">gen_connect_query_service_mapping</a>, <a href="#ts_proto_library-copy_files">copy_files</a>, <a href="#ts_proto_library-files_to_copy">files_to_copy</a>, <a href="#ts_proto_library-kwargs">kwargs</a>)
+                 <a href="#ts_proto_library-gen_connect_query_service_mapping">gen_connect_query_service_mapping</a>, <a href="#ts_proto_library-copy_files">copy_files</a>, <a href="#ts_proto_library-proto_srcs">proto_srcs</a>, <a href="#ts_proto_library-files_to_copy">files_to_copy</a>, <a href="#ts_proto_library-kwargs">kwargs</a>)
 </pre>
 
 A macro to generate JavaScript code and TypeScript typings from .proto files.
@@ -64,7 +64,8 @@ A macro to generate JavaScript code and TypeScript typings from .proto files.
 | <a id="ts_proto_library-gen_connect_query"></a>gen_connect_query |  whether protoc_gen_connect_query should generate [TanStack Query](https://tanstack.com/query) clients, and therefore `*_connectquery.{js,d.ts}` should be written.   |  `False` |
 | <a id="ts_proto_library-gen_connect_query_service_mapping"></a>gen_connect_query_service_mapping |  mapping from source proto file to the named RPC services that file contains. Needed to predict which files will be generated by gen_connect_query. See https://github.com/connectrpc/connect-query-es/tree/main/examples/react/basic/src/gen<br><br>For example, given `a.proto` which contains a service `Foo` and `b.proto` that contains a service `Bar`, the mapping would be `{"a.proto": ["Foo"], "b.proto": ["Bar"]}`   |  `{}` |
 | <a id="ts_proto_library-copy_files"></a>copy_files |  whether to copy the resulting `.d.ts` files back to the source tree, for the editor to locate them.   |  `True` |
-| <a id="ts_proto_library-files_to_copy"></a>files_to_copy |  which files from the protoc output to copy. By default, scans for *.proto in the current package and replaces with the typical output filenames.   |  `None` |
+| <a id="ts_proto_library-proto_srcs"></a>proto_srcs |  the .proto files that are being generated. Repeats the `srcs` of the `proto_library` target. This is used only to determine a default for `files_to_copy`.   |  `None` |
+| <a id="ts_proto_library-files_to_copy"></a>files_to_copy |  which files from the protoc output to copy. By default, performs a replacement on `proto_srcs` with the typical output filenames.   |  `None` |
 | <a id="ts_proto_library-kwargs"></a>kwargs |  additional named arguments to the ts_proto_library rule   |  none |
 
 
diff --git a/examples/connect_node/proto/BUILD.bazel b/examples/connect_node/proto/BUILD.bazel
@@ -4,9 +4,11 @@ load("@rules_proto//proto:defs.bzl", "proto_library")
 
 package(default_visibility = ["//visibility:public"])
 
+proto_srcs = ["eliza.proto"]
+
 proto_library(
     name = "eliza_proto",
-    srcs = ["eliza.proto"],
+    srcs = proto_srcs,
 )
 
 ts_proto_library(
@@ -18,6 +20,7 @@ ts_proto_library(
     },
     node_modules = "//examples/connect_node:node_modules",
     proto = ":eliza_proto",
+    proto_srcs = proto_srcs,
 )
 
 js_library(

diff --git a/examples/proto_grpc/BUILD.bazel b/examples/proto_grpc/BUILD.bazel
@@ -6,9 +6,13 @@ load("@rules_proto//proto:defs.bzl", "proto_library")
 
 npm_link_all_packages(name = "node_modules")
 
+logger_srcs = ["logger.proto"]
+
+status_srcs = ["status.proto"]
+
 proto_library(
     name = "logger_proto",
-    srcs = ["logger.proto"],
+    srcs = logger_srcs,
     visibility = ["//visibility:public"],
     deps = [
         "//examples/connect_node/proto:eliza_proto",
@@ -17,18 +21,39 @@ proto_library(
     ],
 )
 
+proto_library(
+    name = "status_proto",
+    srcs = status_srcs,
+    visibility = ["//visibility:public"],
+    deps = [
+        "@com_google_protobuf//:any_proto",
+    ],
+)
+
 ts_proto_library(
     name = "logger_ts_proto",
     node_modules = ":node_modules",
     proto = ":logger_proto",
+    proto_srcs = logger_srcs,
     visibility = ["//visibility:public"],
     deps = ["//examples/connect_node/proto"],
 )
 
+ts_proto_library(
+    name = "status_ts_proto",
+    node_modules = ":node_modules",
+    proto = ":status_proto",
+    proto_srcs = status_srcs,
+    visibility = ["//visibility:public"],
+)
+
 ts_project(
     name = "proto_grpc",
     srcs = ["main.ts"],
-    deps = [":logger_ts_proto"],
+    deps = [
+        ":logger_ts_proto",
+        ":status_ts_proto",
+    ],
 )
 
 js_test(

diff --git a/examples/proto_grpc/status.proto b/examples/proto_grpc/status.proto
@@ -0,0 +1,27 @@
+// https://github.com/grpc/grpc/blob/master/src/proto/grpc/status/status.proto
+
+syntax = "proto3";
+
+package rpc;
+
+import "google/protobuf/any.proto";
+
+option go_package = "google.golang.org/genproto/googleapis/rpc/status;status";
+option java_multiple_files = true;
+option java_outer_classname = "StatusProto";
+option java_package = "com.google.rpc";
+option objc_class_prefix = "RPC";
+
+message Status {
+  // The status code, which should be an enum value of [google.rpc.Code][google.rpc.Code].
+  int32 code = 1;
+
+  // A developer-facing error message, which should be in English. Any
+  // user-facing error message should be localized and sent in the
+  // [google.rpc.Status.details][google.rpc.Status.details] field, or localized by the client.
+  string message = 2;
+
+  // A list of messages that carry the error details.  There will be a
+  // common set of message types for APIs to use.
+  repeated google.protobuf.Any details = 3;
+}
diff --git a/examples/proto_grpc/status_connect.d.ts b/examples/proto_grpc/status_connect.d.ts
@@ -0,0 +1,5 @@
+// @generated by protoc-gen-connect-es v1.4.0 with parameter "keep_empty_files=true,target=js+dts"
+// @generated from file examples/proto_grpc/status.proto (package rpc, syntax proto3)
+/* eslint-disable */
+// @ts-nocheck
+
diff --git a/examples/proto_grpc/status_pb.d.ts b/examples/proto_grpc/status_pb.d.ts
@@ -0,0 +1,42 @@
+// @generated by protoc-gen-es v1.8.0 with parameter "keep_empty_files=true,target=js+dts"
+// @generated from file examples/proto_grpc/status.proto (package rpc, syntax proto3)
+/* eslint-disable */
+// @ts-nocheck
+
+import type { Any, BinaryReadOptions, FieldList, JsonReadOptions, JsonValue, PartialMessage, PlainMessage } from "@bufbuild/protobuf";
+import { Message, proto3 } from "@bufbuild/protobuf";
+
+/**
+ * @generated from message rpc.Status
+ */
+export declare class Status extends Message<Status> {
+  /**
+   * @generated from field: int32 code = 1;
+   */
+  code: number;
+
+  /**
+   * @generated from field: string message = 2;
+   */
+  message: string;
+
+  /**
+   * @generated from field: repeated google.protobuf.Any details = 3;
+   */
+  details: Any[];
+
+  constructor(data?: PartialMessage<Status>);
+
+  static readonly runtime: typeof proto3;
+  static readonly typeName = "rpc.Status";
+  static readonly fields: FieldList;
+
+  static fromBinary(bytes: Uint8Array, options?: Partial<BinaryReadOptions>): Status;
+
+  static fromJson(jsonValue: JsonValue, options?: Partial<JsonReadOptions>): Status;
+
+  static fromJsonString(jsonString: string, options?: Partial<JsonReadOptions>): Status;
+
+  static equals(a: Status | PlainMessage<Status> | undefined, b: Status | PlainMessage<Status> | undefined): boolean;
+}
+
diff --git a/ts/proto.bzl b/ts/proto.bzl
@@ -47,7 +47,7 @@ load("@aspect_bazel_lib//lib:write_source_files.bzl", "write_source_files")
 load("@aspect_rules_js//js:defs.bzl", "js_binary")
 load("//ts/private:ts_proto_library.bzl", ts_proto_library_rule = "ts_proto_library")
 
-def ts_proto_library(name, node_modules, gen_connect_es = True, gen_connect_query = False, gen_connect_query_service_mapping = {}, copy_files = True, files_to_copy = None, **kwargs):
+def ts_proto_library(name, node_modules, gen_connect_es = True, gen_connect_query = False, gen_connect_query_service_mapping = {}, copy_files = True, proto_srcs = None, files_to_copy = None, **kwargs):
     """
     A macro to generate JavaScript code and TypeScript typings from .proto files.
 
@@ -66,8 +66,9 @@ def ts_proto_library(name, node_modules, gen_connect_es = True, gen_connect_quer
           For example, given `a.proto` which contains a service `Foo` and `b.proto` that contains a service `Bar`,
           the mapping would be `{"a.proto": ["Foo"], "b.proto": ["Bar"]}`
         copy_files: whether to copy the resulting `.d.ts` files back to the source tree, for the editor to locate them.
-        files_to_copy: which files from the protoc output to copy. By default, scans for *.proto in the current package
-            and replaces with the typical output filenames.
+        proto_srcs: the .proto files that are being generated. Repeats the `srcs` of the `proto_library` target.
+            This is used only to determine a default for `files_to_copy`.
+        files_to_copy: which files from the protoc output to copy. By default, performs a replacement on `proto_srcs` with the typical output filenames.
         **kwargs: additional named arguments to the ts_proto_library rule
     """
     if type(node_modules) != "string":
@@ -136,7 +137,8 @@ def ts_proto_library(name, node_modules, gen_connect_es = True, gen_connect_quer
     if not copy_files:
         return
     if not files_to_copy:
-        proto_srcs = native.glob(["**/*.proto"])
+        if not proto_srcs:
+            fail("Either proto_srcs should be set, or copy_files should be False")
         files_to_copy = [s.replace(".proto", "_pb.d.ts") for s in proto_srcs]
         if gen_connect_es:
             files_to_copy.extend([s.replace(".proto", "_connect.d.ts") for s in proto_srcs])