Repository navigation
Conversation
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
2 times, most recently
from
October 27, 2026 09:08
6a26022 to
713012f
Compare
1 of 3 tasks
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
2 times, most recently
from
October 27, 2026 18:35
1600a86 to
d865755
Compare
This was referenced Oct 27, 2026
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
2 times, most recently
from
October 28, 2026 16:37
db00f04 to
9e1d318
Compare
This was referenced Oct 28, 2026
moetemp
changed the title
This was referenced Oct 28, 2026
1 of 3 tasks
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
from
October 29, 2026 04:00
9e1d318 to
566af30
Compare
moetemp
changed the base branch from
moe/up/proto-unsigned-ints
to
moe/up/python-proto-modules
October 29, 2026 04:00
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
from
October 29, 2026 06:28
566af30 to
8c475f3
Compare
moetemp
force-pushed
the
moe/up/python-proto-modules
branch
from
October 29, 2026 06:28
81fd8c7 to
36a8328
Compare
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
2 times, most recently
from
October 29, 2026 07:35
a649a98 to
cbdf5c3
Compare
moetemp
force-pushed
the
moe/up/python-proto-modules
branch
from
October 29, 2026 07:35
2f28526 to
2497fc5
Compare
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
from
October 29, 2026 09:29
cbdf5c3 to
770e53a
Compare
moetemp
force-pushed
the
moe/up/python-proto-modules
branch
from
October 29, 2026 09:29
2497fc5 to
52b7104
Compare
moetemp
changed the title
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
from
October 29, 2026 17:08
770e53a to
e1d283e
Compare
moetemp
force-pushed
the
moe/up/python-proto-modules
branch
from
October 29, 2026 17:08
52b7104 to
e00ebe2
Compare
moetemp
added a commit
that referenced
this pull request
Oct 29, 2026
…#310) ### What changed? This PR plans a protobuf `oneof` case that names a proto-bound WIT record as that record, as nexgen already does for a record field. - Only oneof-bound variants get the new plan, and an `option` around the payload stays. - A .NET case record names its payload's model in full. Else a case named like its record hides the model, and `record Superseded(Superseded Value)` doesn't compile. Brook's next WIT names a case like that. - The first commit is the fix, and the stream sample's TypeScript output appears with it. The second commit generates the stream service for Python, with basedpyright off for it until #311. `tsc` skips the service until #296. In the stream sample's .NET output, the read item now holds the records, not the proto classes. It's based on #308. ### Why? Brook's read item, `variant stream-item`, binds a `oneof` of three proto-bound records. Before the fix, Python and TypeScript generation panicked on it, and .NET typed each case as the proto class. ### How did you test it? Link to a test plan if any - - [x] Unit Tests - [ ] Staging - [ ] End to End Tests New TypeScript and .NET tests round-trip the read item's cases, and the .NET test leaves out the data case. A unit test covers the full name of a case payload. Co-authored-by: Tim Conley <timothy.conley@temporal.io>
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
from
October 29, 2026 17:18
e1d283e to
eeab36c
Compare
moetemp
force-pushed
the
moe/up/python-proto-modules
branch
2 times, most recently
from
October 29, 2026 18:27
25efbe1 to
eb8425d
Compare
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
from
October 29, 2026 18:27
eeab36c to
2986e8f
Compare
tconley1428
reviewed
Oct 29, 2026
|
|
||
| public async read( | ||
| fromOffset?: number, | ||
| fromOffset?: Long, |
Collaborator
There was a problem hiding this comment.
I think all the confusion between number and Long is likely a mistake, and not one we should compound with additional caveats and edge cases. If the overall goal is make these sepecific fields show up as longs, we could potentially just explicitly alias them in the wit.
moetemp
force-pushed
the
moe/up/python-proto-modules
branch
from
October 30, 2026 00:00
eb8425d to
79f16e2
Compare
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
from
October 30, 2026 00:00
2986e8f to
c7b747b
Compare
A proto-backed request holds a 64-bit integer as a `Long`, but a resource method took a number, which can't hold a Redis Store offset. A resource's 64-bit value is now always a `Long`. nexgen refuses one that would go into a record without `@nexus.proto`, which holds it as a number, and a 64-bit map key in any resource. The stream service type-checks.
moetemp
force-pushed
the
moe/up/python-proto-modules
branch
from
October 30, 2026 01:00
79f16e2 to
58790da
Compare
moetemp
force-pushed
the
moe/up/ts-long-resource-params
branch
from
October 30, 2026 01:00
c7b747b to
08816c8
Compare
Contributor
Author
|
Closed in favour of #345 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What changed?
This PR types each 64-bit integer of a TypeScript resource as
Long, in its fields, method parameters and results, as a proto-backed model holds it.A TypeScript resource's 64-bit value is now refused where it would cross a JSON model. A record without
@nexus.protoholds a 64-bit integer as anumber, but the resource holds it as aLong. The two types don't match, and a conversion tonumberloses the digits above 2^53. So nexgen refuses these cases, and names the record and the fix (@nexus.proto, or another type).@nexus.proto, also through a proto message inside that record.nexgen also refuses a 64-bit map key in any TypeScript resource, since a
Longcan't key aRecord.No sample has such a service. In the stream sample's TypeScript output,
Stream.readtakesfromOffsetandmaxBytesasLong, and the service type-checks.Why?
The proto-backed request has a
Longfield, soresources.tsdidn't type-check. ALong.fromNumberconversion fixes the types but not the values. A Brook offset is about1.9e18, so anumberloses its low bits.How did you test it?
Link to a test plan if any -
A new TypeScript test reads from an offset above 2^53, then from the page's
nextOffset. Rejection fixtures cover a parameter and a field that go into a plain request record, a parameter that goes into a proto message inside one, a resource result with no proto wire message, and the map key.