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

Adds cloneAndUpdate to Osdk Objects #1146

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

nihalbhatnagar
Copy link
Contributor

This method will be helpful for updating objects received back from subscriptions.

const rawObj = this[UnderlyingOsdkObject];
const def = this[ObjectDefRef];

if (
Copy link
Contributor Author

@nihalbhatnagar nihalbhatnagar Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I opted to throw here because I didn't want a mistake like this to go unnoticed, but I realize this may not be what we want to happen in production. I could instead throw only in develop and console.warn in prod? Also, understandably we can expose a more powerful cloneAndUpdate that will replace your primary key for you and hope you know what you're doing, but I'm slightly against that since I'd like to know upfront in develop if I do something wrong

update.$title = update[def.titleProperty];
}

const newObject = { ...this[UnderlyingOsdkObject], ...update };
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't filter undefineds, subscribe isn't going to give undefineds for properties it didn't recieve updates for but rather omit the property entirely, and if you get a new object with an undefined via a load I think it should update with that.

Q
>["props"][K];
},
) => Osdk.Instance<Q, OPTIONS, P>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We maintain the scope of the object we call cloneAndUpdate on always in the type, though the raw object may have more properties as well

}

const newObject = { ...this[UnderlyingOsdkObject], ...update };
return createOsdkObject(this[ClientRef], this[ObjectDefRef], newObject);
Copy link
Contributor Author

@nihalbhatnagar nihalbhatnagar Jan 30, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This also allows random properties to end up as attached to the object(if youas any type the input). I don't have strong feelings here but opted to expose this functionality if it was really needed by anyone. It would be easy to filter by known properties only here though

ericanderson
ericanderson previously approved these changes Feb 3, 2025
@policy-bot policy-bot bot dismissed ericanderson’s stale review February 3, 2025 21:16

Invalidated by push of f2c7026

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants