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

Putting into a common namespace two .proto files that both import the same third file throws an error #38

Closed
seishun opened this issue Jul 5, 2013 · 2 comments

Comments

@seishun
Copy link
Contributor

seishun commented Jul 5, 2013

Consider the following three files:

  • example1.proto
import "example3.proto";

message Test1 {
    required int32 a = 1;
}
  • example2.proto
import "example3.proto";

message Test2 {
  required string b = 2;
}
  • example3.proto
message Test3 {
  required string b = 2;
}

Trying to put example1.proto and example2.proto into a common namespace throws the following:

> var builder = ProtoBuf.protoFromFile("./example1.proto");
undefined
> ProtoBuf.protoFromFile("./example2.proto", builder)
Error: Duplicate name in namespace Namespace : Test3
    at ProtoBuf.Reflect.Namespace.addChild (E:\Downloads\node_modules\protobufjs\ProtoBuf.js:1192:27)
    at ProtoBuf.Builder.Builder.create (E:\Downloads\node_modules\protobufjs\ProtoBuf.js:2530:42)
    at ProtoBuf.Builder.Builder.import (E:\Downloads\node_modules\protobufjs\ProtoBuf.js:2590:26)
    at ProtoBuf.Builder.Builder.import (E:\Downloads\node_modules\protobufjs\ProtoBuf.js:2622:43)
    at Object.ProtoBuf.protoFromString (E:\Downloads\node_modules\protobufjs\ProtoBuf.js:2757:34)
    at Object.ProtoBuf.protoFromFile (E:\Downloads\node_modules\protobufjs\ProtoBuf.js:2790:53)
    at repl:1:11
    at REPLServer.self.eval (repl.js:110:21)
    at Interface.<anonymous> (repl.js:239:12)
    at Interface.EventEmitter.emit (events.js:95:17)
@dcodeIO
Copy link
Member

dcodeIO commented Jul 6, 2013

The recommended way of doing this is to use two builders through ProtoBuf.protoFromFile(...) or to create a compound .proto file for your application.

Of course file names could be remembered and excluded from the next include. However, ProtoBuf.js does not know of possible file changes, working directory changes and so on, which might lead to unexpected behaviour. Hmm..

@ghost ghost assigned dcodeIO Jul 6, 2013
@seishun
Copy link
Contributor Author

seishun commented Jul 10, 2013

Instead of remembering file names, you could remember the absolute paths - in node.js you can use path.resolve. It's not an issue in the browser, correct?

As for file changes, I think it would be best to follow the approach of node.js's require. If the file has been loaded before, it returns the cached module and doesn't care if the file has been modified.

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