feat: streaming update code generation for typescript #8988
Conversation
✅ Deploy Preview for determined-ui canceled.
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #8988 +/- ##
==========================================
- Coverage 47.74% 47.72% -0.02%
==========================================
Files 1161 1161
Lines 143093 143253 +160
Branches 2370 2371 +1
==========================================
+ Hits 68322 68371 +49
- Misses 74618 74729 +111
Partials 153 153
Flags with carried forward coverage won't be shown. Click here to find out more.
|
master/cmd/stream-gen/main.go
Outdated
| "model.TaskID": {"string", ""}, | ||
| "model.RequestID": {"number", "0"}, | ||
| "*model.RequestID": {"number | undefined", "undefined"}, | ||
| "model.State": {"string", ""}, |
There was a problem hiding this comment.
shouldn't this be an enum?
There was a problem hiding this comment.
I modified this part to use enum
There was a problem hiding this comment.
sorry i wasn't clear -- shouldn't the generated typescript type be an enum?
There was a problem hiding this comment.
Do you mean we should use a enum like CommandState for model.State?
There was a problem hiding this comment.
Yes -- if the field has constraints, we can communicate that through the type and make sure that we're handling all cases.
| @@ -27,7 +27,8 @@ export class ExperimentSpec extends StreamSpec { | |||
| return this.#id; | |||
| }; | |||
|
|
|||
| public toWire = (): Record<string, Array<number>> => { | |||
| // eslint-disable-next-line @typescript-eslint/no-explicit-any | |||
| public toWire = (): Record<string, any> => { | |||
There was a problem hiding this comment.
can this be Record<string, unknown>?
|
|
||
| source := s.Args["source"] | ||
| entity := strings.ToLower(strings.TrimSuffix(s.Name, "SubscriptionSpec")) | ||
| caser := cases.Title(language.English) |
There was a problem hiding this comment.
If s.Name has more than one words, like s.Name = "StorageBackendSubscriptionSpec", I'm wondering if we should generate export class StorageBackendSpec extends StreamSpec instead of export class StoragebackendSpec extends StreamSpec.
There was a problem hiding this comment.
I think it's safer to just generate StorageBackendSpec because it's possible to have StorageBackendSubscriptionSpec and StorageSubscriptionSpec from backend and we don't want to risk duplicating class name when generating
There was a problem hiding this comment.
Not a blocker: when s.name = "StorageBackendSubscriptionSpec", case.String(entity) = "Storagebackend".
There was a problem hiding this comment.
I think I misunderstood your first comment, I get it now. More than one word can be tricky, thank you for pointing it out
| @@ -0,0 +1,100 @@ | |||
| // Code generated by stream-gen. DO NOT EDIT. | |||
There was a problem hiding this comment.
remember to mark this as ignored for eslint and prettier, and mark it as generated in .gitattributes
Description
Edit stream-gen to auto generate
webui/react/src/services/stream/wire.tsTest Plan
All streaming updates associated with projects should work as expected.
Commentary (optional)
Checklist
docs/release-notes/.See Release Note for details.
Ticket