-
Notifications
You must be signed in to change notification settings - Fork 349
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
feat: Reduce code size by using nullish coalescing operator in fromPartial #376
feat: Reduce code size by using nullish coalescing operator in fromPartial #376
Conversation
} else { | ||
message.numberField = 0; | ||
{ | ||
message.numberField = object.numberField ?? 0; | ||
} |
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.
This block is not needed in the output code. However, I can't get it to work without it.
src/main.ts
Outdated
isValueType(ctx, field) | ||
) { | ||
// An optimized case of the else below that works when `readSnippet` returns the plain input | ||
chunks.push(code`{`); // Without this extra scope the code generation breaks 🤷. We don't really need it. |
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.
Any clue why thi line is needed? Without it the code generation breaks compeltely.
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.
@webmaster128 wow, that was weird. I pushed a commit to this PR that re-fiddled with the braces in fromPartial
. I'm not really sure how it was working before :-) but I think it's straightened out now.
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.
Ah, very nice, thanks. I thought there is some magic in the code generator such that braces are not supposed to be closed manually. Now it's clearer.
2b28dc5
to
ab60a28
Compare
6f01de5
to
ea653c0
Compare
Alright, I'm happy with this now. In https://github.com/confio/cosmjs-types/pull/11/files you see what this change means for the code generated larger projects. Rebased onto current main branch. How do you want this to be released? Is this a feature or a fix? |
ea653c0
to
3af7913
Compare
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.
@webmaster128 looks great! Much cleaner! Yeah, releasing as a feature sgtm.
Thanks :) |
# [1.84.0](v1.83.3...v1.84.0) (2021-11-02) ### Features * Reduce code size by using nullish coalescing operator in fromPartial ([#376](#376)) ([19d2ded](19d2ded))
🎉 This PR is included in version 1.84.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
Closes #371
Builds on #375 (merge that first, then rebase)