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
not creating creation methods when using structs #5919
Conversation
Looks like one build is broken. |
if (field.deprecated) continue; | ||
if (CanCreateFactoryMethod(struct_def)) { | ||
if (lang_.language == IDLOptions::kJs) { | ||
std::string paramDoc = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you make sure to use snake_case
and auto
where possible
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I only changed the indentation level here :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ok, fair enough..
CI error seems Rust related |
Would be nice to enable structs in |
Seems like a network error: error inflating zlib stream; class=Zlib |
@@ -1222,6 +1223,17 @@ class JsTsGenerator : public BaseGenerator { | |||
obj_api_unpack_func = unpack_func + "\n\n" + unpack_to_func; | |||
} | |||
|
|||
bool CanCreateFactoryMethod(const StructDef &struct_def) { | |||
// to preserve backwards compatibility, we allow the first field to be a struct |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@aardappel @krojew Is this valid? If we use the createX()
method when a struct is the first field, the buffer fails verification in other languages, but succeeds if we create the struct after the generated call to startX()
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hard to say what the reason was, since it was so long ago. But we're making backwards-incompatible changes anyway so this can be removed if it causes problems.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! I believe it should be removed since as far as I understand the buffer will always be invalid even if the struct is just the first field.
fixes #5914