From 92caefa7a72ed4a711f3c6291191d4fd455a07f3 Mon Sep 17 00:00:00 2001 From: egorm Date: Fri, 31 May 2024 15:13:03 -0700 Subject: [PATCH 1/5] 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` --- examples/proto_grpc/BUILD.bazel | 21 ++++++++++++++++++++- examples/proto_grpc/status.proto | 27 +++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 1 deletion(-) create mode 100644 examples/proto_grpc/status.proto diff --git a/examples/proto_grpc/BUILD.bazel b/examples/proto_grpc/BUILD.bazel index d597fd0f..870639e2 100644 --- a/examples/proto_grpc/BUILD.bazel +++ b/examples/proto_grpc/BUILD.bazel @@ -17,6 +17,15 @@ proto_library( ], ) +proto_library( + name = "status_proto", + srcs = ["status.proto"], + visibility = ["//visibility:public"], + deps = [ + "@com_google_protobuf//:any_proto", + ], +) + ts_proto_library( name = "logger_ts_proto", node_modules = ":node_modules", @@ -25,10 +34,20 @@ ts_proto_library( deps = ["//examples/connect_node/proto"], ) +ts_proto_library( + name = "status_ts_proto", + node_modules = ":node_modules", + proto = ":status_proto", + 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 new file mode 100644 index 00000000..269f1859 --- /dev/null +++ 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; +} From fbcc5650a356ef5be48ec99c2ad6668dc1121641 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Sun, 2 Jun 2024 10:29:33 -0700 Subject: [PATCH 2/5] fix: explicit files_to_copy for multiple ts_proto_lib --- examples/proto_grpc/BUILD.bazel | 13 +++++++- examples/proto_grpc/status_connect.d.ts | 5 +++ examples/proto_grpc/status_pb.d.ts | 42 +++++++++++++++++++++++++ 3 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 examples/proto_grpc/status_connect.d.ts create mode 100644 examples/proto_grpc/status_pb.d.ts diff --git a/examples/proto_grpc/BUILD.bazel b/examples/proto_grpc/BUILD.bazel index 870639e2..c8e090e8 100644 --- a/examples/proto_grpc/BUILD.bazel +++ b/examples/proto_grpc/BUILD.bazel @@ -28,6 +28,12 @@ proto_library( ts_proto_library( name = "logger_ts_proto", + # Must give explicit files_to_copy since the default behavior tries to glob(["*.proto"]) + # and that doesn't work when there are two ts_proto_library in the same package. + files_to_copy = [ + "logger_pb.d.ts", + "logger_connect.d.ts", + ], node_modules = ":node_modules", proto = ":logger_proto", visibility = ["//visibility:public"], @@ -36,6 +42,11 @@ ts_proto_library( ts_proto_library( name = "status_ts_proto", + # See comment above + files_to_copy = [ + "status_pb.d.ts", + "status_connect.d.ts", + ], node_modules = ":node_modules", proto = ":status_proto", visibility = ["//visibility:public"], @@ -46,7 +57,7 @@ ts_project( srcs = ["main.ts"], deps = [ ":logger_ts_proto", - ":status_ts_proto" + ":status_ts_proto", ], ) diff --git a/examples/proto_grpc/status_connect.d.ts b/examples/proto_grpc/status_connect.d.ts new file mode 100644 index 00000000..06c89d27 --- /dev/null +++ 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 new file mode 100644 index 00000000..eee6d6fb --- /dev/null +++ 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 { + /** + * @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); + + static readonly runtime: typeof proto3; + static readonly typeName = "rpc.Status"; + static readonly fields: FieldList; + + static fromBinary(bytes: Uint8Array, options?: Partial): Status; + + static fromJson(jsonValue: JsonValue, options?: Partial): Status; + + static fromJsonString(jsonString: string, options?: Partial): Status; + + static equals(a: Status | PlainMessage | undefined, b: Status | PlainMessage | undefined): boolean; +} + From d53b52f78b85f2b21c333aaaf4e48af6c56c6b3d Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Sun, 2 Jun 2024 10:38:49 -0700 Subject: [PATCH 3/5] 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. --- examples/connect_node/proto/BUILD.bazel | 5 ++++- examples/proto_grpc/BUILD.bazel | 21 ++++++++------------- ts/proto.bzl | 7 +++++-- 3 files changed, 17 insertions(+), 16 deletions(-) diff --git a/examples/connect_node/proto/BUILD.bazel b/examples/connect_node/proto/BUILD.bazel index bbe6717e..ae9e770d 100644 --- 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 index c8e090e8..65f740aa 100644 --- 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", @@ -19,7 +23,7 @@ proto_library( proto_library( name = "status_proto", - srcs = ["status.proto"], + srcs = status_srcs, visibility = ["//visibility:public"], deps = [ "@com_google_protobuf//:any_proto", @@ -28,27 +32,18 @@ proto_library( ts_proto_library( name = "logger_ts_proto", - # Must give explicit files_to_copy since the default behavior tries to glob(["*.proto"]) - # and that doesn't work when there are two ts_proto_library in the same package. - files_to_copy = [ - "logger_pb.d.ts", - "logger_connect.d.ts", - ], 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", - # See comment above - files_to_copy = [ - "status_pb.d.ts", - "status_connect.d.ts", - ], node_modules = ":node_modules", proto = ":status_proto", + proto_srcs = status_srcs, visibility = ["//visibility:public"], ) diff --git a/ts/proto.bzl b/ts/proto.bzl index c937bb6e..e9ff1c56 100644 --- 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,6 +66,8 @@ 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. + 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, scans for *.proto in the current package and replaces with the typical output filenames. **kwargs: additional named arguments to the ts_proto_library rule @@ -136,7 +138,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]) From ff926afad047ab3156e6a70f048654c156f7a07f Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Sun, 2 Jun 2024 10:40:14 -0700 Subject: [PATCH 4/5] chore: docgen --- docs/proto.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/proto.md b/docs/proto.md index 952c538e..40328c67 100644 --- a/docs/proto.md +++ b/docs/proto.md @@ -48,7 +48,7 @@ Future work 
 ts_proto_library(name, node_modules, gen_connect_es, gen_connect_query,
-                 gen_connect_query_service_mapping, copy_files, files_to_copy, kwargs)
+                 gen_connect_query_service_mapping, copy_files, proto_srcs, files_to_copy, kwargs)
A macro to generate JavaScript code and TypeScript typings from .proto files. @@ -64,6 +64,7 @@ A macro to generate JavaScript code and TypeScript typings from .proto files. | 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` | | 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

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. | `True` | +| 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` | | 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` | | kwargs | additional named arguments to the ts_proto_library rule | none | From 4b34a3a22c771ad3627bd063b9e54ab0e11fdeee Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Sun, 2 Jun 2024 10:43:48 -0700 Subject: [PATCH 5/5] fix: docs --- docs/proto.md | 2 +- ts/proto.bzl | 3 +-- 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/proto.md b/docs/proto.md index 40328c67..28189d17 100644 --- a/docs/proto.md +++ b/docs/proto.md @@ -65,7 +65,7 @@ A macro to generate JavaScript code and TypeScript typings from .proto files. | 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

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. | `True` | | 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` | -| 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` | +| 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` | | kwargs | additional named arguments to the ts_proto_library rule | none | diff --git a/ts/proto.bzl b/ts/proto.bzl index e9ff1c56..2aaa51e0 100644 --- a/ts/proto.bzl +++ b/ts/proto.bzl @@ -68,8 +68,7 @@ def ts_proto_library(name, node_modules, gen_connect_es = True, gen_connect_quer copy_files: whether to copy the resulting `.d.ts` files back to the source tree, for the editor to locate them. 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, scans for *.proto in the current package - and replaces with the typical output filenames. + 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":