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
[FeatureRequest] Have option to generate completed object with nested object #924
Comments
What happens when the message is recursive? message Foo {
Bar bar = 1;
}
message Bar {
Foo foo = 1;
} Your proposal would cause a stack overflow. |
@jcready is right that schemas with two-way pointers would infinite loop. IIRC I've seen the Java protobuf output handles this by creating a static/shared "default foo" / "default bar" instances, but I think only works b/c they are immutable, and any mutations to the default foo/default bar actually create new instances. Also fwiw @tnhh00037 , personally/for my own codebases, I wouldn't want these defaults applied to the production code paths; imo production code should always fill in real values, and not get for-free defaults. That said, you mention unit tests, and I think that is more valuable/appropriate use case, basically creating test factories that allow tests to easily get already-filled-in messages. For test factories, personally I'd want them to be a different method, like Granted, @jcready is right that So, I dunno, happy to accept a PR for this, but I think my asks/guidance would be:
|
As a former ROS developer ( need to mention beforehand that ROS message doesn't support recursive message ), when defining a ROS message like this
builiding this above message with library like class Information {
constructor(initObject={}) {
if (initObj === null) {
this.name = null;
this.age = null;
this.detail = null
}
else {
// check for this message's fields by key name - otherwise assign default values
if (initObj.hasOwnProperty('name')) {
this.name = initObj.name;
}
else {
this.name = '';
}
if (initObj.hasOwnProperty('age')) {
this.age = initObj.age;
}
else {
this.age = 0;
}
if (initObj.hasOwnProperty('detail')) {
this.detail = initObj.detail;
}
else {
this.detail = new Detail();
}
}
}
} Because no recursive, so you can create the whole message without worrying about stack overflow. The reason why ROS support this behavior is because it helps ensure consistency and reliability in robotic applications. Without default values, software might encounter runtime errors or unexpected behavior when processing message. I understand the application between ROS and Protobuf are different. What I want is to extends protobuf's ability so that it can support more use cases, by defining one message interface but can re-use in case Protocol changing from GRPC to something else. Your point of view is a real thing
Anyway, let's come back to how we will resolve this. I've got some names: |
Hm. Fwiw I'm not familiar with ROS, but when you mention:
Doesn't that make this feature less necessary / maybe more hassle than it's worth? My rationale is that because protobuf messages can never have required children (only scalars can be required), an
And so client code that does like:
Already has a "a default", which is
So you've got three cases to handle now, instead of just two. Which seems like a step backwards. It seems like to really make this feature useful, you'd want to fundamentally change interface Information {
detail: Detail
} So you're code can go back to have "only two" (or even "only one") codepaths: handling Such a fundamental change would go beyond "a new create method", and really be something fundamentally different like a new option like
So, that's my current/updated thoughts--instead of "a new create method", I think your code/ask is probably better of with this Lmk if that makes sense, and if you'd like to work on a PR that does that. As a disclaimer it will probably be a pretty involved PR. |
Current behavior
With message like this
Using
create
orfromPartial
method will not assign value fordetail
field (undefined
)To fix that, user needs to do like below. If object has deeper nested field, it's going to be requiring a lot of manual work.
Suggestion!!
Have option to let user create completed object with nested level.
With this ability user can create object for unit test + main logic without needing to create separated object for each nested field.
I recommend adding flag
--ts_proto_opt=defaultNestedField=true
Then in
src/main.ts
Test result
Modified code result in generated constant
Information
like belowThe text was updated successfully, but these errors were encountered: