Skip to content
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

fix: remove Record word conflict #638

Merged
merged 2 commits into from
Aug 8, 2022

Conversation

stezu
Copy link
Collaborator

@stezu stezu commented Aug 3, 2022

When a protobuf file contains a message named Record, it causes conflicts with the utility types at the bottom of the file which were using the built-in TS Record utility. This obviously causes the file to contain invalid Typescript since we'd expect Record to be the utility generic, not the non-generic type generated by this library.

To alleviate this issue, I added a test to treat Record as a reserved word and fixed the implementation to avoid using it and instead used the mapped types to emulate what Record does. The alternatives I considered seemed to be more trouble than they were worth given that there was only the single usage of Record in the generated output.

@stephenh
Copy link
Owner

stephenh commented Aug 6, 2022

@stezu this looks good!

Can you run yarn proto2bin to re-create the *.bin files? In theory none of those should have changed, and the proto2bin script will use docker to make sure any host-dependent / protoc-version-dependent changes are not included in the *.bin files.

@stezu
Copy link
Collaborator Author

stezu commented Aug 7, 2022

@stephenh I was wondering about that, thanks for the tip!

@stephenh
Copy link
Owner

stephenh commented Aug 8, 2022

Looks great, @stezu , and thanks for the regression test on Record!

@stephenh stephenh merged commit 5664d09 into stephenh:main Aug 8, 2022
stephenh pushed a commit that referenced this pull request Aug 8, 2022
## [1.121.5](v1.121.4...v1.121.5) (2022-08-08)

### Bug Fixes

* remove Record word conflict ([#638](#638)) ([5664d09](5664d09))
* resolve import collisions for services ([#651](#651)) ([ee0296f](ee0296f))
@stephenh
Copy link
Owner

stephenh commented Aug 8, 2022

🎉 This PR is included in version 1.121.5 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants