Latest versions don't import google proto files automatically

[atassis]

Trying to generate the messages with google.protobuf types used- there are links on them on code, but they are no .ts files generated for them anymore. Saw no breaking changes in latest 5 versions

[gribbet]

Is this caused by #283?

[atassis]

Yes, it is. 1.79.7 generates google dir, while 1.79.8 and above dont

[stephenh]

Right. In #283, @emil-e quoted the google protoc docs that, AFAIU, the new behavior is more correct...

However, it does seem like a large breaking change, and I think has also silently broken ts-proto's integration tests, i.e. for tests like use-date-false/metadata.proto, it is importing a google.protobuf type, and is no longer emitting the google/protobuf/timestmap.ts file, and is only working b/c that file is checked in and so happens to still be there.

To fix use-date-false, I'd have to put timestamp.proto on the protoc param list, but naively write now the update-bins.sh script doesn't even know where those are at, and just assumes protoc knows how to find them from its machine-local installation.

@emil-e I'm thinking we need to either revert your change, or at least make exceptions for any google.* proto files?

What would be safer is putting your change behind a flag, like emitImportedFiles=false which defaults to true, which is the previous behavior, and you could opt in to the more-correct-but-breaking behavior.

What do you think?

[altro3]

I can confirm the appearance of a bug in version 1.79.8. I use the standard protobuff wrappers.proto file in my project. Prior to version 1.79.8, the assembly went smoothly.

[emil-e]

@emil-e I'm thinking we need to either revert your change, or at least make exceptions for any google.* proto files?

What would be safer is putting your change behind a flag, like emitImportedFiles=false which defaults to true, which is the previous behavior, and you could opt in to the more-correct-but-breaking behavior.

What do you think?

I don't think one should have to manually specify any of the Google protobuf files since as far as I know, you don't have to do that for any of the officially supported languages. I believe the reason that those implementations don't have this problem is that the protobuf runtime libraries have those types included in the library itself. I don't know if this is practical for ts-proto in which case an alternative solution could be to make exceptions for the Google files.

Putting the new behavior behind a flag would work fine for our use cases but on pure principle, it's best to have the goal that correct behavior is the default, at least in the long run. But it's up to you, of course.

[atassis]

I think it is better to leave an option to choose between different behaviors for the developer

[atassis]

Guys, I can try implementing an option by myself if you show me an example PR where an option is implemented

[stephenh]

🎉 This issue has been resolved in version 1.81.0 🎉

The release is available on:

• npm package (@latest dist-tag)
• GitHub release

Your semantic-release bot 📦🚀

[stephenh]

Thanks for the offer @atassis ! Sorry, didn't get a chance to reply to your comment before @doochik went ahead and got a fix in. :-)

The 1.81.0 release restores the previous behavior, and @emil-e you can opt into the "only filesToGenerate" behavior.

Thanks all!