Adds an example for subscriptions #990
Conversation
| import type { ObjectOrInterfaceDefinition, Osdk } from "@osdk/api"; | ||
|
|
||
| export function consolidateOsdkObject< | ||
| T extends Osdk.Instance<V, any, any>, |
There was a problem hiding this comment.
Why are you capturing T and U separately if they're the same type?
There was a problem hiding this comment.
I capture them separately since I want to specify the return type to be one or the other. I want the ability for someon to pass in Osdk.Instance<A> and Osdk.Instance<A, never "propA"> and return the type in the original. I tested this locally and this is the case that the type is narrowed properly. Note: I need to swap the return type to be T instead of U
There was a problem hiding this comment.
The bigger question I have here is which one should we return. I think we should always scope the object down to the oldObject. So if oldObject has a more narrow definition, we want the oldDefinition to be returned and also delete extraneous keys off of the new object.
There was a problem hiding this comment.
The edge case that's left out here is if someone wants an object scoped down to Object B. They can't just reverse what they put in since we explicitly use the properties on the latter object.
There was a problem hiding this comment.
I think this exclusion is ok. In my mind, this can be used for loadObject and subscriptions, where they already have a defined store. For either, they will likely want an object in the shape of that store instead of what they receive necessarily on the load or subscribe. I can't really imagine why they would want the edge case described above
There was a problem hiding this comment.
If so, maybe we rename as such then. I was confused because earlier it seemed like you were just expanding to whatever the new return type was
There was a problem hiding this comment.
We just need this name to be clear that you're updating existing properties, without expanding the property scope of the old object (even if the new object has more props)
| if (!oldObject) { | ||
| return upToDateObject; | ||
| } | ||
| const combinedObject = oldObject as any; |
There was a problem hiding this comment.
Do we need this any cast? If we don't capture them separately you can probably use the object as is
There was a problem hiding this comment.
So when I update the return type, I'm going to remove this any cast and just iterate on object B based on the keys of object A and update the old object.
|
|
||
| for (const key in upToDateObject) { | ||
| if (upToDateObject.hasOwnProperty(key)) { | ||
| combinedObject[key] = upToDateObject[key]; |
There was a problem hiding this comment.
Hmmm, also I think we may need to update the internal values here right? Not totally sure so double check me, but if someone starts like casting things $as and back, the underlying data may not actually reflect?
|
|
||
| import type { ObjectOrInterfaceDefinition, Osdk } from "@osdk/api"; | ||
|
|
||
| export function consolidateOsdkObject< |
There was a problem hiding this comment.
Not used anymore, will delete
| @@ -203,6 +203,16 @@ export namespace Osdk { | |||
| OPTIONS, | |||
| ConvertProps<Q, NEW_Q, P> | |||
| >; | |||
| readonly $cloneAndUpdate: < | |||
| T extends Osdk.Instance<Q, any, L>, | |||
| L extends PropertyKeys<Q>, | |||
There was a problem hiding this comment.
This is what I'm still figuring out, not sure why but I can't pass in an object that is not Osdk.Instance, eg Instance<Employee, never, "id"> doesn't work with capturing the generic or any
There was a problem hiding this comment.
I'd have to check it out to play with it.
You probably also want to just support an object that is the raw values?
| @@ -180,37 +173,7 @@ describe("convertWireToOsdkObjects", () => { | |||
| expect(prototypeBefore).not.toBe(prototypeAfter); | |||
| }); | |||
|
|
|||
| it("works even with unknown apiNames - new", async () => { | |||
There was a problem hiding this comment.
Not sure why these are deleted, merge issue probably will fix
No description provided.