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: ensure generated fromTimestamp works when useOptionals=all #832

Merged
merged 2 commits into from
May 16, 2023

Conversation

miselin
Copy link
Contributor

@miselin miselin commented May 11, 2023

The generated fromTimestamp raises a Typescript compilation error 't.seconds' is possibly 'undefined' when useOptionals=all as all the fields in the generated timestamp message are optional.

This PR changes the generated function to handle an undefined value by defaulting to zero.

(drafting while I get the test files updated)

@stephenh
Copy link
Owner

Hi @miselin ; ah yeah, this is a good fix! Updating the test files should work with yarn bin2ts, but lmk if that's not working. Thanks!

@miselin
Copy link
Contributor Author

miselin commented May 15, 2023

Hi @miselin ; ah yeah, this is a good fix! Updating the test files should work with yarn bin2ts, but lmk if that's not working. Thanks!

I think I'm fighting against something with Docker and mount permissions somewhere:

$ yarn build:test
./map-long-optional/test.proto
protoc-gen-dump: line 18: file.bin: Permission denied
mv: overwrite './map-long-optional/test.bin'? ^C

The scripts/tests don't have write access to the directory but it's a rw mount... very strange 🤔

@stephenh stephenh marked this pull request as ready for review May 16, 2023 01:00
@stephenh
Copy link
Owner

@miselin I went ahead and pushed up the test changes, and they look good; thanks!

@stephenh stephenh merged commit 1f82445 into stephenh:main May 16, 2023
6 checks passed
stephenh pushed a commit that referenced this pull request May 16, 2023
## [1.147.3](v1.147.2...v1.147.3) (2023-05-16)

### Bug Fixes

* ensure generated fromTimestamp works when useOptionals=all ([#832](#832)) ([1f82445](1f82445))
@stephenh
Copy link
Owner

🎉 This PR is included in version 1.147.3 🎉

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