Publish/import proto dependencies as npm packages

[andrewparmet]

I'm experimenting with ts-proto and it looks really good! I had a question about the generated files. It looks like ts-proto generates all files supplied by the CodeGeneratorRequest's assembled list of topologically sorted dependencies (proto_file).

This works great for standalone projects, but if I want to generate and publish artifacts, ideally they don't include all dependencies. That way I can publish two artifacts but only have one copy of generated dependencies from one used in the other. For example StringValue is generated everywhere, but it should only be generated once - in the project that defines it.

Would it make sense to include an option that generates files only from files_to_generate? Ideally I think this would be the default behavior, but it would break backwards compatibility.

[stephenh]

Yeah, I've generally felt the same way, i.e. it doesn't seem great for every artifact to re-output codegen for every referenced proto.

However, I'm not really sure what the alternative is though, b/c there is a real lack of standardized / recommended mapping between how a .proto import should be translated into its language-/platform-specific import.

I.e. if my service.proto pulls in google/proto/timestamp.proto, right now the import is local, i.e.:

import { Timestamp } from "./google/protobuf/timestamp";

If the import was from an npm package, which I think is what you're suggesting, what should that import be? ... from "google-protobuf-timestamp"? I.e. assume every proto is in its own artifact? That doesn't seem right...

Maybe this could only be done by config, i.e. telling ts-proto via a command line arg/something that "for our company, we've published google/proto/timestamp.proto to a google-common npm package". That seems pretty gross to handle at scale, like if you had ~10s/100s of proto microservices at some point.

I dunno. Really open to ideas here. Basically how to map a proto import to, as you said, "the project that defines it" (and also its npm package).

[andrewparmet]

We've sort of dealt with this problem for our own protobuf plugin for Kotlin. Our generator actually publishes the well-known types itself. Downstream projects pull in the generated source along with the proto files needed for further local codegen (using the protobuf-gradle-plugin). Both are packaged into the JAR file. We include this dependency automatically when you use the code generator.

What that could look like here is a separate package on npm ("ts-proto-core"?) with just the well-known types in it, and further downstream projects would depend on that package if they reference the well-known types. I'm relatively new to the JS ecosystem and package management is complex, so I don't know if that pattern makes sense here. JS also tends to have a copy-everything approach, so the compromise already taken might make the most sense.

You're right that there's no great convention for where these things go. Kotlin/Java have packages, but JS doesn't quite have that. I suppose ts-proto could map files into directories by protobuf package since JS doesn't care about relative file location so long as imports are pointing to the right places?

Right now I'm using Gradle to run ts-proto, like this:

plugins {
    java // needed for protobuf plugin
    id("com.google.protobuf") version "0.8.11"
}

protobuf {
    plugins {
        id("ts") {
            path = "node_modules/.bin/protoc-gen-ts_proto"
        }
    }

    generateProtoTasks {
        all().forEach { task ->
            task.plugins {
                id("ts")
            }
        }
    }
}

dependencies {
    protobuf("jar-that-cotains-our-proto-files")
}

which throws generated code into e.g. build/generated/source/proto/main/ts/google/protobuf/timestamp.ts, so referencing a canonical npm package would break the relative import. We can configure the output directory though.

We do get these nice imports in generated files:

import { Timestamp } from '../../../../../../google/protobuf/timestamp';

and I'm not sure how that would mess with things.

Presumably if we changed our output directory to match whatever the canonical package declared, these would resolve correctly?

[andrewparmet]

Yeah, looks like Google's JS generator also simply generates everything, so I'm not sure this is really addressable. Thanks!

[stephenh]

separate package on npm ("ts-proto-core"?) with just the well-known types in it

Yeah, I think this is a good idea. Maybe something like @ts-proto/google-protobuf-descriptor, @ts-proto/google-protobuf-wrappers etc.

And maybe anything that wasn't well-known, the package name was something like @your-org-set-via-a-config-param + the "path" / proto package + proto file name.

I think that would work okay.

build/generated/source/proto/main/ts/google/protobuf/timestamp.ts, so
referencing a canonical npm package would break the relative import.

I think this would end up being:

import { Timestamp } from '@ts-proto/google-protobuf-timestamp';

Which would work without any wrinkles around relative/etc. paths. Granted, if ts-proto users opt'd into this "use packages mode", they'd have to make sure any proto file they referenced also had an entry in their package.json file.

I'm not going to work on this right now, but I think this is a good idea and something we could have eventually, so I'm going to go ahead and re-open it.

[andrewparmet]

Fair enough. We chose to call that module "core" because the protobuf JAR that we used to generate it contains the definitions for the well-known types in wrappers.proto as well as any.proto, descriptor.proto, api.proto, and friends, so they were all generated together. To be honest we weren't thrilled about that name and it might change if we someday find a better one.

Looks like the "@" prefix has traction on NPM, so @ts-proto/google-protobuf looks like a good name, depending on what's packaged there.

[stephenh]

so they were all generated together

Right, that is a big wrinkle, i.e. how would ts-proto "know" (for either the well-known types or internal company types) "which types from separate files are generated 'together' and which are not".

I.e. one tempting rule-of-thumb would be that, since all of the well-known types have the same proto package, google.protobuf, ts-proto could assume that "well, anything with the same google.protobuf proto package must be compiled at the same time --> be in the same npm package". So then the npm package for all the well-known types would @ts-proto/google-protobuf.

But I'm not sure how that translates over to internal types, like if you have a proto package like package my_company, but have project-a.proto and project-b.proto in separate internal projects both use that same my_company package, that breaks the ^ rule-of-thumb about "all proto files that say they're in proto package $foo are in the same repo/dependency".

I.e. for internal types, each separate project/repository would have to use it's own proto package, i.e. package my_company.project_a and package my_company.project_b.

But maybe that'd be fine? Maybe that's what most protobuf users are already doing? I don't have a ton/really any data points about "proto package <--> project / repository proto file" naming conventions.

That said, the "proto package --> npm module" convention is so tempting, maybe that is what ts-proto should just go ahead and implement, and assume that users like yourself that want to use this feature would setup their proto package naming convention appropriately.

[andrewparmet]

Well, they almost have the same proto package. Other files included in that collection include the plugin definitions in google/protobuf/compiler/plugin.proto. So already at the bottom of the dependency tree there's a violation of "one proto package per project." I don't know what specified behavior would or should be if you tried to run the ts-proto plugin on the code from that JAR.

We do make that assumption about the google.protobuf package and all subpackages - since we can't simply use the specified package (JVM collisions for those classes which are included the protobuf runtime), we blanket-replace all com.google.protobuf with our own package. Any user-provided protobuf that uses that package will have its package renamed as well, even if they didn't want that behavior. We don't have enough users that someone has tried that, but presumably they'd use our package-override option to get what they wanted if they truly wanted their class to reside in the Java package com.google.protobuf.

We do have split packages where we define protos in different projects in the same protobuf package. We could, however, define them in different protobuf packages and use the package override I mentioned above to put them in the right Kotlin package, allowing them to use both of these features at once - live in the same Kotlin package, and be reference-able from multiple NPM packages.

The tough part remains projects that use more than one protobuf package and how to resolve the imports in downstream projects.

[paralin]

@stephenh I'm running into this still - it tries to import from ./google/protobuf/... but that doesn't exist.

I was thinking it might make sense to instead remap that import to @ts-proto/google/protobuf/...

[paralin]

@stephenh This is what I'm using in the meantime: https://github.com/aperturerobotics/ts-proto-common-types

if (modulePath.startsWith('./google/protobuf'))
    modulePath = '@aperturerobotics/ts-proto-common-types/' + path.normalize(modulePath);

and: aperturerobotics/protobuf-project@8b5c813#diff-1ede4818246d7c457badf7c6532da084ee62c58c8f541efeb49760c592b3d261R2

[stephenh]

@paralin cool, that makes sense... in theory @pcj 's #597 would allow setting up -Mgoogle.protobuf=@aperturerobotics/ts-proto-common-types to achieve what you want?

I was thinking it might make sense to instead remap that import to @ts-proto/google/protobuf/...

If by @ts-proto/google, you were thinking that'd be like a @ts-proto/google package available on npm, my assumption/hesitation has been that we'd quickly run into issues with users using slightly different options/settings between their project and the published @ts-proto/* project, resulting in incompatible types.

Like would the npm-published google.protobuf artifact have fromPartial, encode/decode methods, services, etc...

ts-proto is, for better or worse, such a swiss army knife of options, that I'm just assuming each user/company would have to setup their own repo of @whateverco/google.protobuf (exactly like you're doing by hand, which hopefully #597 will make more pleasant).

[paralin]

@stephenh That works, but I would expect the "built-in" types to "just work" without a special remap.

For example in protobuf-go it automatically references the Go package implementing those types. They have a assertion which ensures that the Go package imported matches the same major version of the generated code.

[stephenh]

@paralin sure, I understand the desire, but link me to the options that protobuf-go supports? Just scanning their docs, I see maybe three or four (-M, some path stuff)...

Afaict none seem to affect protobuf-go's output as materially as "this encode method just does or does exist" like ts-proto's does (or the fromPartial methods, or the fromJson methods, or the toJson methods).

(And even if we decided to ship the superset of "include all the methods", and rely on tree shaking to drop unused ones, there are still options like oneof, exact types, use optional, etc., that very materially affect the output in breaking changes ways.)

Granted, this is my fault for starting ts-proto down the "swiss army knife of options" approach, but it is what it is (albeit in response to users specifically asking/providing PRs for various styles).

So I get how/why protobuf-go can do this, but still assert its not realistic for ts-proto, short of only working with a very specific / blessed configuration of options that users are restricted from changing.

[paralin]

@stephenh The main requirement is when importing the Timestamp type from somewhere to another ts-proto message.

If the TImestamp type itself is generated with a "blessed set of default options" that should be fine.

Because the types that import the Timestamp don't really care about anything other than:

• toTimestamp
• fromTimestamp
• fromJsonTimestamp
• Timestamp.fromJSON
• Timestamp.encode
• Timestamp.decode

.. everything else doesn't matter. If there were a common package which implemented these 3 functions in one hand-written file and the Timestamp class with the "blessed defaults", and a "index.ts" exporting all of it, it would no longer be necessary to include these things in the generate code (just import them instead).