feat: introduce bufbuild/connect-es

[alexeagle]

See https://github.com/bufbuild/connect-es

This is just a first step to get a live protoc invocation, but doesn't provide any rules for users (maybe they don't need any, and existing proto_nodejs_library is fine).
It also doesn't include any gazelle extension affordance for generating BUILD files that use buf plugins.

Tested:

$ bazel build //example/thing:thing_node_es_compile
Target //example/thing:thing_node_es_compile up-to-date:
  bazel-bin/example/thing/thing_pb.js
  bazel-bin/example/thing/thing_connect.js
INFO: Elapsed time: 0.229s, Critical Path: 0.00s

$ head bazel-bin/example/thing/thing*.js
==> bazel-bin/example/thing/thing_connect.js <==
// @generated by protoc-gen-connect-es v0.12.0 with parameter "target=js,keep_empty_files=true"
// @generated from file example/thing/thing.proto (package example.thing, syntax proto3)
/* eslint-disable */
// @ts-nocheck

==> bazel-bin/example/thing/thing.js <==
// source: example/thing/thing.proto
/**
 * @fileoverview
 * @enhanceable
 * @suppress {missingRequire} reports error on implicit type usages.
 * @suppress {messageConventions} JS Compiler reports an error if a variable or
 *     field starts with 'MSG_' and isn't a translatable message.
 * @public
 */
// GENERATED CODE -- DO NOT EDIT!

==> bazel-bin/example/thing/thing_pb.js <==
// @generated by protoc-gen-es v1.3.0 with parameter "target=js,keep_empty_files=true"
// @generated from file example/thing/thing.proto (package example.thing, syntax proto3)
/* eslint-disable */
// @ts-nocheck

import { proto3, Timestamp } from "@bufbuild/protobuf";

/**
 * @generated from enum example.thing.Status
 */

[pcj]

Recommend rebase.

[alexeagle]

Hey @pcj it's rebased now.

[alexeagle]

I removed the changes in the example/ folder for now, because:

• the repo isn't "gazelle-clean", see chore: run //:gazelle #343
• If the example wants to show a mix of stephenh/ts-proto and bufbuild/es-connect, then we have the problem of options being inherited that aren't desired. For example, # gazelle:proto_plugin ts_proto option esModuleInterop=true appears in example/BUILD.bazel, but shouldn't be an option that's passed when a subpackage uses gazelle:proto_plugin ts_proto implementation bufbuild:es
• it makes the PR bigger without adding any test assertions

I see there's also a test fixture in example/golden. But neither WORKSPACE file there uses rules_ts, because the stephenh/ts-proto plugin isn't tested here. I'm trying to follow your existing conventions and not invent anything new. So if you'd like to test it in this way, perhaps we should first do a "pre-factor" to add that missing coverage for the existing plugin, then I can follow along.

[voxeljorge]

Some notes on getting this to work:

• I had to add an npm_translate_lock to my workspace since this dep appears to currently be missing from the dep macros
• I had to use # gazelle:map_kind to map the proto_ts_library to my own macro to be able to correctly specify the tsconfig for proto_ts_library

Both of these issues might be out of scope for this PR but would probably have to be worked around by anyone wanting to use these new plugins. Once these two were resolved the codegen seems to work complete with tsc checks.

[voxeljorge]

One other issue I encountered with this, when attempting to use proto_compiled_sources I get an action conflict between the ts_project declared in proto_ts_library and the compiled output from the proto_compile target that is part of proto_compiled_sources.

The issue here seems to be that as part of the nature of how ts_project works, it copies source files to the output tree before running tsc/etc. If the source files already exist in the output tree it will skip this copy step but to make that work the ts_project target generated by proto_ts_library needs to depend on the output of proto_compile. I don't see a way to configure the sources list in the current proto_ts_library rule implementation, so setting this up may require some kind of custom implementation or a new wrapper. I'm not sure if that should be part of this PR either since the standard proto_compile route still seems to be working just fine.

[voxeljorge]

@pcj any chance you may be able to look at this one again soon? We're likely to start using it and would like to avoid running on a fork for too long.

[pcj]

@voxeljorge Thanks for the ping, I'll start reviewing this today.

[voxeljorge]

hi @pcj just a friendly reminder about this one, if there's anything I can do to help move it along please let me know :)

[ewhauser]

One other issue I encountered with this, when attempting to use proto_compiled_sources I get an action conflict between the ts_project declared in proto_ts_library and the compiled output from the proto_compile target that is part of proto_compiled_sources.

The issue here seems to be that as part of the nature of how ts_project works, it copies source files to the output tree before running tsc/etc. If the source files already exist in the output tree it will skip this copy step but to make that work the ts_project target generated by proto_ts_library needs to depend on the output of proto_compile. I don't see a way to configure the sources list in the current proto_ts_library rule implementation, so setting this up may require some kind of custom implementation or a new wrapper. I'm not sure if that should be part of this PR either since the standard proto_compile route still seems to be working just fine.

@voxeljorge Did you come up with a workaround for this? I'm running into the same issue separate from this PR

[voxeljorge]

We got around this by replacing the built-in rule using a # gazelle:map_kind directive to replace proto_ts_library with a macro of our own which depends on the output of proto_compile rather than the checked in sources. This lets us both have the in-tree generated sources as well as the ones from proto_compile which already exist in the output tree, which allows the ts_project rule to skip copying and avoids the action conflict.

ts_project seems smart enough to skip files which already exist in the output tree, so there may be a cleaner way to introduce the output of proto_compile into the set of dependencies of proto_ts_library but I never figured out another way. The map_kind approach seems to be working fine though.

[ewhauser]

We got around this by replacing the built-in rule using a # gazelle:map_kind directive to replace proto_ts_library with a macro of our own which depends on the output of proto_compile rather than the checked in sources. This lets us both have the in-tree generated sources as well as the ones from proto_compile which already exist in the output tree, which allows the ts_project rule to skip copying and avoids the action conflict.

ts_project seems smart enough to skip files which already exist in the output tree, so there may be a cleaner way to introduce the output of proto_compile into the set of dependencies of proto_ts_library but I never figured out another way. The map_kind approach seems to be working fine though.

Thanks. That makes sense. Can you share your macro and perhaps the relevant sections of your tsconfig if you messed with the paths? I tried something like this:

def proto_ts_library(
        name,
        **kwargs):
    kwargs.pop("srcs")
    _proto_ts_library(
        name = name,
        tsconfig = "@//go/rules_proto/protobuf-ts:tsconfig",
        transpiler = partial.make(swc, swcrc = "@//go/rules_proto/protobuf-ts:.swcrc"),
        srcs = [":pb_ts_compiled_sources"],
        **kwargs
    )

But tsc couldn't find the sources:

Use --sandbox_debug to see verbose messages from the sandbox and retain the sandbox build root for debugging
error TS18003: No inputs were found in config file '/private/var/tmp/_bazel_ewhauser/c1fee43d9f70be2c9377b78115996411/sandbox/darwin-sandbox/757/execroot/__main__/bazel-out/darwin_arm64-f
astbuild/bin/go/rules_proto/protobuf-ts/tsconfig.json'. Specified 'include' paths were '["**/*"]' and 'exclude' paths were '[]'.

Are you using a single tsconfig.json for all the protos?

[voxeljorge]

This is the macro we're using. We keep all of our proto definition in a top level package called //protos so that we can keep all the generated code separate from the rest of our code easily. We use proto_compiled_sources for Go and TS currently which together end up generating a lot of files.

def proto_ts_library(name, srcs, data = [], declaration = True, tsconfig = "//:tsconfig", **kwargs):
    compiled_sources_target = ":" + name.removesuffix("ts_proto") + "es_compiled_sources"
    ts_project(
        name = name,
        srcs = [compiled_sources_target],
        declaration = declaration,
        tsconfig = tsconfig,
        data = depset(
            data + [
                # For each generated TS proto library, include a package.json file
                # in the data dependencies which specifies {"type": "module"} to
                # ensure that the generated .js files under the /protos directory
                # are interpreted as ES modules by Node at runtime. Without this,
                # Node treats them as CommonJS modules which causes problems in
                # certain environments, such as during Jest test execution.
                # https://nodejs.org/api/packages.html
                "//protos:package_json",
            ],
        ).to_list(),
        **kwargs
    )

    native.filegroup(
        name = name + "_srcs",
        srcs = [compiled_sources_target],
        visibility = ["//visibility:public"],
    )

[pcj]

Doing some overdue maintainance work on stackb/rules_proto today. @alexeagle are you happy with the current state of this PR? If so let's get it rebased such that it will trigger the github actions CI rather than the bazel-buildkite CI.

[voxeljorge]

@pcj We're currently using this PR as-is (with the macro from my comment) to successfully generate so I can confirm that the basic functionality seems in order.

[alexeagle]

@pcj I've rebased the PR.

[pcj]

LGTM