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

proto-loader not populating file descriptor dependency list #2595

Open
jtimmons opened this issue Oct 10, 2023 · 0 comments
Open

proto-loader not populating file descriptor dependency list #2595

jtimmons opened this issue Oct 10, 2023 · 0 comments

Comments

@jtimmons
Copy link
Contributor

jtimmons commented Oct 10, 2023

Problem description

The *Definition structures that proto-loader returns each have a fileDescriptorProtos field which include the files needed to resolve each type. The file descriptor includes a "dependency list" of files that they require but this is currently always returning nothing

This was discussed in #79

Reproduction steps

This can be reproduced using the following two proto file definitions:
sample.proto

syntax = "proto3";
package sample;
import 'vendor.proto';

service SampleService {
  rpc Hello (HelloRequest) returns (HelloResponse) {}
}

message HelloRequest {
  string hello = 1;
}

message HelloResponse {
  vendor.HelloStatus status = 2;
}

vendor.proto

syntax = "proto3";
package vendor;

enum HelloStatus {
  HELLO = 1;
  WORLD = 2;
}

this then inspected using the following code:

import { FileDescriptorProto } from 'google-protobuf/google/protobuf/descriptor_pb';
import * as protoLoader from '@grpc/proto-loader';

const root = protoLoader.loadSync('sample.proto', { includeDirs: [__dirname] });

Object.values(root).forEach(({ fileDescriptorProtos }) => {
  if (Array.isArray(fileDescriptorProtos)) {
    fileDescriptorProtos.forEach((bin) => {
      const proto = FileDescriptorProto.deserializeBinary(bin);
      console.log(`File: ${proto.getName()}\tDependencies: ${proto.getDependencyList()}`);
      /* output:
File: sample.proto      Dependencies: 
File: vendor.proto      Dependencies: 
File: sample.proto      Dependencies: 
File: vendor.proto      Dependencies: 
File: sample.proto      Dependencies: 
File: vendor.proto      Dependencies: 
      */
    });
  }
});

we would expect that sample.proto would include vendor.proto in its dependency list due to the import and usage of it in the HelloResponse field

Environment

  • OS name, version and architecture: [e.g. Linux Ubuntu 18.04 amd64]: Mac OS 13.4.1
  • Node version [e.g. 8.10.0]: v20.8.0
  • Node installation method [e.g. nvm]: nvm
  • If applicable, compiler version [e.g. clang 3.8.0-2ubuntu4]: N/A
  • Package name and version [e.g. gRPC@1.12.0]: proto-loader@0.6.9

Additional context

We are working around this in the @grpc/reflection package by manually adding those dependencies based on the proto definitions with this visitor-pattern-based logic

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

2 participants