Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add example that uses google.protobuf well-known types #1820

Closed
gonzojive opened this issue Apr 11, 2020 · 17 comments
Closed

Add example that uses google.protobuf well-known types #1820

gonzojive opened this issue Apr 11, 2020 · 17 comments

Comments

@gonzojive
Copy link

gonzojive commented Apr 11, 2020

🚀 feature request

Relevant Rules

  1. ts_proto_library: There are no examples of protos that depend on @com_google_protobuf//:timestamp_proto or similar in the examples directory

  2. ts_devserver: When I tried including a ts_proto_library that depends on "@com_google_protobuf//:timestamp_proto", I get errors. See next section.

Description

the following error in my devserver:

ts_scripts.js?v=1585115769616:1966 GET http://red-main:5432/google-protobuf/google/protobuf/timestamp_pb.js net::ERR_ABORTED 404 (Not Found)
...
zone.min.js?v=1585115769616:19 Uncaught Error: Script error for "google-protobuf/google/protobuf/timestamp_pb", needed by: examples_angular/httpserver/frontendpb/frontend_pb, examples_angular/httpserver/frontendpb/frontend_grpc_web_pb
http://requirejs.org/docs/errors.html#scripterror
    at makeError (ts_scripts.js?v=1585115769616:175)
    at HTMLScriptElement.onScriptError (ts_scripts.js?v=1585115769616:1745)
    at e.invokeTask (zone.min.js?v=1585115769616:19)
    at t.runTask (zone.min.js?v=1585115769616:19)
    at t.invokeTask [as invoke] (zone.min.js?v=1585115769616:19)
    at _ (zone.min.js?v=1585115769616:50)

Where my proto file looks like

syntax = "proto3";

package frontend;

option go_package = "github.com/...../web/viz/httpserver/frontendpb";

import "google/protobuf/timestamp.proto";


service FrontendService {
    rpc GetGraph (GetGraphRequest) returns (GetGraphResponse) {}
}

message GetGraphRequest {
    // Factor by which to scale the result data.
    double scale = 1;
}

message GetGraphResponse {
    repeated Trace traces = 1;
}

message Trace {
    google.protobuf.Timestamp start = 5;
    message Point {
        double x = 1;
        string x_string = 3;
        google.protobuf.Timestamp x_timestamp = 4;
        double y = 2;
    }
    repeated Point points = 1;

    string plotly_type = 2;
    string plotly_mode = 3;
    string marker_color = 4;
}

The build rule looks like

proto_library(
    name = "frontendpb_proto",
    srcs = ["frontend.proto"],
    visibility = ["//visibility:public"],
    deps = [
        "@com_google_protobuf//:timestamp_proto",
    ],
)

go_proto_library(
    name = "frontendpb_go_proto",
    compilers = ["@io_bazel_rules_go//proto:go_grpc"],
    importpath = "..../web/viz/httpserver/frontendpb",
    proto = ":frontendpb_proto",
    visibility = ["//visibility:public"],
)

Describe the solution you'd like

A tiny example of a working gRPC app (Typescript without Angular, please, as small of a WORKSPACE and BUILD file as possible).

Describe alternatives you've considered

I am currently working around my issues with well-known types by using worse versions of those types.

@alexeagle
Copy link
Collaborator

@mrmeku maybe?

@gonzojive
Copy link
Author

Any pointers to repos that use the well-known protobuf types?

@gonzojive
Copy link
Author

gonzojive commented Jun 5, 2020

I tried again recently and am getting a new error in protos the depend on google/type/latlng.proto: Uncaught ReferenceError: proto is not defined

The protobuf code raising the error is

// GENERATED CODE -- DO NOT EDIT!

var jspb = require('google-protobuf');
var goog = jspb;
var global = Function('return this')();

var google_type_latlng_pb = require('proj/google/type/latlng_pb');
goog.object.extend(proto, google_type_latlng_pb);

Looking at the relevant codegen code in the protobuffers project:

  // Generate "require" statements.
  if ((options.import_style == GeneratorOptions::kImportCommonJs ||
       options.import_style == GeneratorOptions::kImportCommonJsStrict)) {
    printer->Print("var jspb = require('google-protobuf');\n");
    printer->Print("var goog = jspb;\n");

    // Do not use global scope in strict mode
    if (options.import_style == GeneratorOptions::kImportCommonJsStrict) {
      printer->Print("var proto = {};\n\n");
    } else {
      printer->Print("var global = Function('return this')();\n\n");
    }

    for (int i = 0; i < file->dependency_count(); i++) {
      const std::string& name = file->dependency(i)->name();
      printer->Print(
          "var $alias$ = require('$file$');\n"
          "goog.object.extend(proto, $alias$);\n",
          "alias", ModuleAlias(name), "file",
          GetRootPath(file->name(), name) + GetJSFilename(options, name));
    }
  }

The ts_devserver I'm using has these scripts:

    scripts = [
        "@npm//:node_modules/tslib/tslib.js",
        ":rxjs_umd_modules",
        # We are manaully adding the bazel generated named-UMD date-fns bundle here as
        # named-UMD bundles for non-APF npm packages are not yet automatically added.
        # This file is generated by the npm_umd_bundle @npm//date-fns:date-fns__umd
        # rule that is setup by yarn_install.
        "@npm//date-fns:date-fns.umd.js",

        # Needed for gRPC support
        "@npm_bazel_labs//grpc_web:bootstrap_scripts",
        "@npm//google-protobuf:google-protobuf.umd.js",
        "@npm_bazel_labs//protobufjs:bootstrap_scripts",
    ],

@gonzojive
Copy link
Author

gonzojive commented Jun 5, 2020

More relevant code:

load("@rules_proto//proto:defs.bzl", "proto_library")
load("@npm_bazel_labs//:index.bzl", "ts_proto_library")

proto_library(
    name = "app_proto",
    srcs = ["app.proto"],
    visibility = ["//visibility:public"],
    deps = ["@go_googleapis//google/type:latlng_proto"],
)

ts_proto_library(
    name = "app_ts_proto",
    proto = ":app_proto",
    visibility = ["//visibility:public"],
)
syntax = "proto3";

package dashboard;

import "google/type/latlng.proto";

message Place {
    string name = 1;
    google.type.LatLng lat_lng = 2;
}

ts_proto_library source code seems to call pbjs with either --wrap=default or --wrap=es6. edit: This isn't the ts_proto_library rule being used actually.. the one on grpc_web is.

Unfortunately I haven't managed to get a development version of rules_nodejs
working reliably, so I haven't tracked down the issue further.

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 60 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Sep 15, 2020
@github-actions
Copy link

This issue was automatically closed because it went two weeks without a reply since it was labeled "Can Close?"

@gonzojive
Copy link
Author

Please reopen.

@ztl8702
Copy link

ztl8702 commented Sep 29, 2020

I have an example using googleapi annotations, kind related.

https://github.com/MindongLab/yngdieng/blob/871e37ed4f6fb8947e09df067e96b28255d05355/shared/services.proto#L8

Added @go_googleapis//google/api:annotations_proto as a dep of ts_proto_library:

https://github.com/MindongLab/yngdieng/blob/871e37ed4f6fb8947e09df067e96b28255d05355/shared/BUILD#L50

However I had to strip out import * from ../google/api/annotations_pb statements in the generated .ts files, in rollup config: https://github.com/MindongLab/yngdieng/blob/871e37ed4f6fb8947e09df067e96b28255d05355/web/src/rollup.config.js#L6

@bshimc
Copy link

bshimc commented Oct 31, 2020

I believe ReferenceError: proto is not defined is grpc/grpc-web#447 (comment)

@alexeagle alexeagle reopened this Nov 16, 2020
@alexeagle alexeagle removed the Can Close? We will close this in 30 days if there is no further activity label Nov 16, 2020
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 90 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Feb 15, 2021
@gonzojive
Copy link
Author

gonzojive commented Feb 15, 2021 via email

@github-actions github-actions bot removed the Can Close? We will close this in 30 days if there is no further activity label Feb 16, 2021
@normmcgarry
Copy link

I'm experiencing this issue as well, but wanted to report a workaround that I'm using.

Example:

syntax = "proto3";

import "google/protobuf/empty.proto";
import "google/protobuf/timestamp.proto";

service ExampleService {
  rpc GetExample(google.protobuf.Empty) returns (Examples) {}
}
...
proto_library(
    name = "example_proto",
    srcs = [
        "example.proto",
    ],
    deps = [
        "@com_google_protobuf//:empty_proto",
        "@com_google_protobuf//:timestamp_proto",
    ],
)

ts_proto_library(
    name = "example_ts_grpc",
    proto = ":example_proto",
)

Generated:

var jspb = require('google-protobuf');
var goog = jspb;
var global = Function('return this')();

var google_protobuf_empty_pb = require('google-protobuf/google/protobuf/empty_pb');
goog.object.extend(proto, google_protobuf_empty_pb);
var google_protobuf_timestamp_pb = require('google-protobuf/google/protobuf/timestamp_pb');
goog.object.extend(proto, google_protobuf_timestamp_pb);
goog.exportSymbol('proto.GetExample', null, global);

Proto doesn't seem to exist until after the first:

goog.exportSymbol('proto.GetExample', null, global);

I made a workaround that seems to work:

Create a local empty proto:

syntax = "proto3";

message MissingProtoFix {}

Then import it above your other imports.

syntax = "proto3";

import "protos/fix.proto";
import "google/protobuf/empty.proto";
import "google/protobuf/timestamp.proto";

@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 90 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Jul 14, 2021
@gonzojive
Copy link
Author

gonzojive commented Jul 14, 2021 via email

@github-actions github-actions bot removed the Can Close? We will close this in 30 days if there is no further activity label Jul 16, 2021
@github-actions
Copy link

This issue has been automatically marked as stale because it has not had any activity for 90 days. It will be closed if no further activity occurs in two weeks. Collaborators can add a "cleanup" or "need: discussion" label to keep it open indefinitely. Thanks for your contributions to rules_nodejs!

@github-actions github-actions bot added the Can Close? We will close this in 30 days if there is no further activity label Oct 15, 2021
@tsawada
Copy link
Contributor

tsawada commented Oct 15, 2021

@github-actions github-actions bot removed the Can Close? We will close this in 30 days if there is no further activity label Oct 16, 2021
@alexeagle
Copy link
Collaborator

okay thanks @tsawada

FWIW I think rules_nodejs is going to drop all protobuf/grpc as part of our scope reduction for 5.0, others are doing a more capable job with that and it's orthogonal to providing a node toolchain and fetching npm dependencies.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants