search

MobilityData / gtfs-realtime-bindings

Language bindings generated from the GTFS Realtime protocol buffer spec for popular languages.
Apache License 2.0
370 stars 127 forks source link

Node: Question regarding VehiclePosition class #133

Closed magnusburton closed 5 months ago

magnusburton commented 5 months ago

I had a question regarding VehiclePosition and it's definition. According to the specification, the variables referenced below are optional. This is not reflected in the class however. Why is that? I'm looking to convert a VehiclePosition class to and from a JSON string. I've done a similar thing with Alert and IAlert using gtfs.transit_realtime.Alert.toObject(alert).

https://github.com/MobilityData/gtfs-realtime-bindings/blob/ffe13e9b63436812bef91163a4c3698b07b2dfc7/nodejs/gtfs-realtime.d.ts#L1000-L1022

Thanks for your insights!

jameslinjl commented 5 months ago

@magnusburton I'm not familiar with all the ins-and-outs of source code generated from .proto files, but I believe that this discrepancy between the generated interface and the class is a long-standing issue (and is seen all throughout the generated TS types).

My understanding is that this isn't an issue with this project, but rather is a deeper issue with pbjs / pbts. This GH issue seems to detail folks running into similar issues. The discussion there centers more on the proto3 side of things, where all fields are now optional by default, but the same basic problem seems to exist for optional fields in proto2.

jameslinjl commented 5 months ago

At a previous job (years ago), I believe we actually moved off of pbts and over to ts-proto, which we found to generate superior TS types. When I was working on containerizing the node.js bindings here, I wanted to minimize change, so I decided to stick with pbts as the least amount of change required. I don't have bandwidth to contribute to this project these days, but it could be worth someone doing some research / experimentation to see if ts-proto would generate better types!

magnusburton commented 5 months ago

@jameslinjl Thanks for your comments. I'll give ts-proto a try and see if that produces better types.