fix: Add check for lower bound with forceLong=number #1057
Conversation
|
|
||
| if (_m0.util.Long !== Long) { | ||
| _m0.util.Long = Long as any; | ||
| _m0.configure(); |
There was a problem hiding this comment.
@timostamm might be a moot point in the long run, but I think this "hook up Long to protobufjs and call configure" is required, and had been using a ts-poet-ism to get like auto-inserted into the output on the conditional use of Long in the output...
So the otherwise-lgtm change of using the { toString(): string } meant ts-poet doesn't know to implicitly add this anymore (which is admittedly kind of cute/too-cute on how its gets added).
Am I missing something, where this isn't actually required? The intent was just to have it "just work" for users, and not need to do this themselves. If we should keep it, we can probably figure out another way to get it conditionally into the output...
There was a problem hiding this comment.
Let's add it back 👍 I'll take a look tomorrow.
There was a problem hiding this comment.
Uh, I remember what's going on with protobuf.js and Long.js, and I wish I didn't.
protobufjs automagically loads long from node_modules, and uses it in Reader. If the magic fails (easily happens with bundlers), it falls back to an alternative and returns Number instead of Long.
We could hook the ConditionalOutput to Reader.int64 instead. Then we're free to refactor longToNumber. Doesn't seem like a good idea because the magic behavior is difficult to test.
This reverts commit f40a1b9.
Only keep the new check for MIN_SAFE_INTEGER
|
I don't think it's worth going through the the long conversion refactor, see #1057 (comment). Updated the PR to just add the check for lower bounds. |
## [1.176.3](v1.176.2...v1.176.3) (2024-06-07) ### Bug Fixes * Add check for lower bound with forceLong=number ([#1057](#1057)) ([01ef3c3](01ef3c3))
|
🎉 This PR is included in version 1.176.3 🎉 The release is available on: Your semantic-release bot 📦🚀 |
With default plugin options, or
useJsTypeOverridefor a field with optionjstype = JS_NUMBER, 64-bit integral values are converted to a JavaScript Number when decoding from binary. When a value is too large for Number, an error is thrown.But signed integers can also be too small for Number. This PR adds a check for the lower bound.