RDP's in TS OSDK #1035

nihalbhatnagar · 2024-12-05T14:40:15Z

This PR adds the base for runtime derived properties in the OSDK.

The following are features to be added before the completion of the project but are not covered by this PR

Calculated properties. The spec for this feature has not been merged in gateway, but we can start exploring the syntax.
Finalizing names for selectProperty, collectList, collectSet
Allowing for derived properties whose types are not defined at compile time.
- We need to explore how we can let them narrow their types as well as seeing if we can get any type guarantees when the actual object is returned.
- Our typing breaks if the user has a conditional return with different property types at runtime. For example, a === b ? integerProperty : stringProperty. For this PR, enforcing that the user does a simple definition here is fine, but needs to be fixed in the future. Support any typed or narrowed properties will have the result of fixing this issue.

nihalbhatnagar · 2024-12-09T22:07:58Z

packages/api/src/derivedProperties/DeriveObjectSet.ts

+  ) => this;
+}
+
+type CollectAggregations = "collectSet" | "collectList";


Parroting gateway names for now. This probably needs a review into valueSet or valueList, but the gateway names should be fine for right now

nihalbhatnagar · 2024-12-10T18:52:02Z

packages/client/src/objectSet/ObjectSet.test.ts

+      });
+    });
+
+    it("propagates derived property type to future object set operations with correct types", () => {


This test and others with no clause are just for us to ensure that we're typing things correctly.

nihalbhatnagar · 2024-12-10T18:58:07Z

packages/api/src/derivedProperties/WithPropertyObjectSet.ts

+} from "../ontology/ObjectTypeDefinition.js";
+import type { LinkedType, LinkNames } from "../util/LinkUtils.js";
+import type { WithPropertyDefinition } from "./WithPropertiesClause.js";
+


I use inheritance here to make sure we enforce what methods are available and to reduce duplication

nihalbhatnagar · 2024-12-10T18:59:00Z

packages/api/src/derivedProperties/WithPropertyObjectSet.ts

+    SingleLinkWithPropertyObjectSet<Q>
+{}
+
+export interface BaseWithPropertyObjectSet<


This object set only has filter and pivot as aggregations and selectProperty is not available on the base object set.

nihalbhatnagar · 2024-12-10T19:02:34Z

packages/api/src/derivedProperties/WithPropertyObjectSet.ts

+  >;
+}
+
+interface SingleLinkWithPropertyObjectSet<


BaseObjectSet extends SingleLinkWithPropertyObjectSet. This makes the selectProperty method available only for chains of single links, which correlates with the backend. The pivotTo operation in BaseObjectSet enforces that if we pivot to a many link, we from then on use ManyLinkWithPropertyObjectSet which does not have selectProperty available and only returns further ManyLink object sets.

packages/api/src/objectSet/ObjectSet.ts

ssanjay1

Overall looking good! Left some minor comments

ssanjay1 · 2024-12-12T14:53:32Z

packages/api/src/derivedProperties/WithPropertyObjectSet.ts

+interface ManyLinkWithPropertyObjectSet<
+  Q extends ObjectOrInterfaceDefinition,
+> extends AggregatableWithPropertyObjectSet<Q> {
+  readonly pivotTo: <L extends LinkNames<Q>>(


So just clarifying, the reason we need to duplicate pivotTo here is because if there's a many link anywhere in the clause, you can't select anymore? Even if you go from a many to a single link?

Yeah that's correct, which makes sense because for this to work you have to basically be navigating to exactly one object

We should probably document that here too.

ssanjay1 · 2024-12-12T14:56:31Z

packages/api/src/derivedProperties/WithPropertyObjectSet.ts

+
+interface AggregatableWithPropertyObjectSet<
+  Q extends ObjectOrInterfaceDefinition,
+> extends FilterableDeriveObjectSet<Q> {


Does this have to extend FilterableDeriveObjectSet<Q> purely to make ManyLinkWithPropertyObjectSet work? If so, could we just have that extend FilterableDeriveObjectSet<Q> as well? Feels a bit confusing the way its setup now.

Not a huge deal though

Fixed! Agreed this makes it more readable

ssanjay1 · 2024-12-12T14:58:27Z

packages/api/src/derivedProperties/WithPropertyObjectSet.ts

+  ) => this;
+}
+
+type CollectAggregations = "collectSet" | "collectList";


nit: for this and the type below can we call them something like CollectDeriveAggregations, really want to make sure we don't confuse these with existing normal agg options

Debating between the above Derive, ow WithPropertyBaseAggregations, or BaseWithPropertyAggregations. We aren't using "derive" as a term, but it is much cleaner than the other 2. Number 3 is kind of confusing, but number 2 would mean they al start the same which I don't want. Stuck with number 1 for conciseness

ssanjay1 · 2024-12-12T15:01:52Z

packages/api/src/derivedProperties/WithPropertiesClause.ts

+export type WithPropertyDefinition<
+  T extends ObjectMetadata.Property,
+> = {
+  definitionId: unknown;


Can we not narrow this type down at all?

We can type this down to a string, I was wondering if we should type this as unknown to tell the user -> dont touch this

Typed it as a string even though we could use a number in case we decide we want to use a UUD or something else in the future

ssanjay1 · 2024-12-12T15:15:09Z

packages/api/src/aggregate/AggregatableKeys.ts

@@ -36,14 +36,19 @@ type AGG_FOR_TYPE<T> = number extends T ? NumericAggregateOption

 export type ValidAggregationKeys<
  Q extends ObjectOrInterfaceDefinition,
+  StringOptions extends string = StringAggregateOption,


If we want to keep all the agg options a bit simpler to parse though, another way to format this type could be keeping the Agg_For_Type and modifying it to take an extra boolean (that controls whether this is for derive or not, default to false) and have it branch to the right agg options that way, rather than having the responsibility on the user to pass in custom options if they want. That being said, if you feel like in the future we'll need to customize agg options even more, this does make it more flexible so up to you

If you do keep it like this, lets remove AGG_FOR_TYPE if its not being used anywhere

Hmm yeah this does make sense to me, will refactor

ssanjay1 · 2024-12-13T15:17:53Z

packages/client/src/derivedProperties/createWithPropertiesObjectSet.ts

+        type: "searchAround",
+        objectSet,
+        link,
+      }, definitionMap) as any;


is as any the best we can do here?

Looks like we don't need it, this was left over from dev

ssanjay1 · 2024-12-13T15:25:35Z

packages/client/src/objectSet/ObjectSet.test.ts

+    // Executed code fails since we're providing bad strings to the function
+    it.fails("correctly narrows types of aggregate function", () => {
+      client(Employee).withProperties({
+        "derivedPropertyName": (base) => {


me being annoying nit: but for completeness, lets add an empty string arg as well and make sure that errors

ssanjay1 · 2024-12-13T15:28:13Z

packages/client/src/objectSet/ObjectSet.test.ts

+    it("allows fetching derived properties with correctly typed Osdk.Instance types", async () => {
+      const objectWithRdp = await client(Employee).withProperties({
+        "derivedPropertyName": (base) =>
+          base.pivotTo("lead").selectProperty("employeeId"),


Can we add a test to verify things don't break if we select a property that has all undefined values?

What do you mean?

Basically if a prop returns an undefined value

ssanjay1 · 2024-12-13T15:33:35Z

packages/client/src/objectSet/ObjectSet.test.ts

+  describe("Derived Properties Object Set", () => {
+    it("does not allow aggregate or selectProperty before a link type is selected", () => {
+      client(Employee).withProperties({
+        "derivedPropertyName": (base) =>


Can we also add a test to make sure that within a WithPropertiesClause you can't just return something like base.pivotTo("lead") without doing some sort of selection or aggregation?

Apologies in advance if I missed this somewhere, but don't seem to see that being tested anywhere yet

This is guarded by the type of the lambda, but I will add an explicit test for this behavior

nihalbhatnagar · 2024-12-16T15:21:49Z

packages/api/src/aggregate/AggregatableKeys.ts

  : never;

 export type ValidAggregationKeys<
  Q extends ObjectOrInterfaceDefinition,
-  StringOptions extends string = StringAggregateOption,
-  NumericOptions extends string = NumericAggregateOption,
+  R extends "aggregate" | "withPropertiesAggregate" = "aggregate",


@ssanjay1 Is this what you had in mind?

Yea that works

Though maybe keeping parity with the other names it should be aggregate | derivedPropertiesAggregate or something along those lines, basically including the word derive

ericanderson · 2025-01-09T18:12:12Z

packages/api/src/derivedProperties/WithPropertyObjectSet.ts

+  | "avg"
+  | BaseWithPropAggregations
+  | CollectWithPropAggregations;
+


If I am thinking about whats ideal we need to add some type tests. I'm okay if its in a follow up

I will follow this up, this PR is a little bloated with tests

ericanderson

Left a few notes we should fix up. If we don't do the IR I think we should fix up the types as I mention to not carry so much complexity around.

I am still undecided on the IR or not. I get not wanting to create a complex language (or reproduce what the backend has) but I also feel like there are no surprises there either. IDK, this is a tough one.

ericanderson · 2025-01-09T18:15:38Z

packages/api/src/objectSet/ObjectSet.ts

+> = {
+  __DefinitionMetadata: {
+    properties: {
+      [T in Extract<keyof D, string>]: D[T] extends


I'd have to check but my gut is keyof D & string is better for the compiler than the conditional type that Extract causes.

ericanderson · 2025-01-09T18:26:11Z

packages/api/src/objectSet/ObjectSet.ts

+  ) => ObjectSet<
+    WithPropertiesObjectDefinition<
+      Q,
+      D


I'm okay with it for now but this feels like a place where having the simpler types that are not based on functions produces more readable types? We have to do the extraction work anyway to create things like properties and props below but we are required to keep the function hierarchy in the type generics as this object is passed around instead of resolving them at the time withProperties is called and thus carrying a lot less. Like we could just be carrying { foo: number, bar: string } lets say. Or even { foo: PropertyDefinition, bar: PropertyDefinition }.

ericanderson · 2025-01-09T18:26:55Z

packages/api/src/index.ts

+export type {
+  WithPropertyObjectSet,
+} from "./derivedProperties/WithPropertyObjectSet.js";
+


Extra new line causes grouping when organizing imports instead of perfect sorting.

ericanderson · 2025-01-09T18:28:42Z

packages/client/src/objectSet/ObjectSet.test.ts

@@ -485,6 +488,285 @@ describe("ObjectSet", () => {
    });
  });

+  describe("Derived Properties Object Set", () => {


I would want to see some expectTypeOf entries for all of this. It both makes it easier to review and gives protections against the future that we didn't break things. Right now we just know that the incoming syntax is correct but not the output.

I added some expectTypeOf entries for tests I had already written and my newer tests that use the new ObjectSetWithProperties type should also cover a large part of the output testing

ericanderson · 2025-01-09T18:32:54Z

packages/client/src/objectSet/ObjectSet.test.ts

+          type: "methodInput",
+        }, map);
+
+        const clause: WithPropertiesClause<Employee> = {


For something like this, especially so you can add type tests you want this to be more like const clause = { ... } satisfies WithPropertiesClause<Employee> so that it will capture the raw values

ericanderson · 2025-01-09T18:35:23Z

packages/client/src/objectSet/ObjectSet.test.ts

+      expect(objectWithUndefinedRdp.derivedPropertyName).toBeUndefined();
+    });
+
+    describe("withPropertiesObjectSet", () => {


Nit: don't pass this string. This test seems to be directly testing createWithPropertiesObjectSet and so you can literally pass a reference to said function.

I think this also means that this entire describe should really be in createWithPropertiesObjectSet.test.ts so its clearer whats under test.

ericanderson · 2025-01-09T18:42:05Z

packages/client/src/objectSet/ObjectSet.test.ts

+        };
+
+        const result = clause["derivedPropertyName"](deriveObjectSet);
+        const definition = map.get(result.definitionId);


I have some mixed feelings about the definitionId still, especially considering it ends up on the public api.

ericanderson · 2025-01-09T18:54:13Z

packages/client/src/derivedProperties/createWithPropertiesObjectSet.ts

+            "Invalid aggregation operation " + aggregationOperation,
+          );
+      }
+      definitionMap.set(definitionId.toString(), {


Suppose for a moment definitionMap is actually Map<object, DerivedPropertyDefinition>.

instead of definitionId we could do this:

const ret = {} as WithPropertyDefinition<any>; definitionMap.set(ret, {... }); return ret;

No need to do accounting of ids and we don't have to leak them/make them part of our public API.

Its still not perfect because people might return their own objects thinking thats okay when its not and they still might do something weird like reuse an object from before (which won't work because its a different map) but its closer.

Ultimately I think we are probably better just designing an IR that is based on raw objects.

I like this idea for how we'd do the definitionMap, I'll add it to this PR. I'd try and also explore a global cache here that would allow the user to store off the definition in another variable but we'd probably run into typing issues there when we introduce calculated properties that can reference other derived properties. I'll also explore the IR solution here, where I think the biggest benefit is reusability of the type. However, I do think it's quite complicated and I'd like to also see what the demand for this would be. I'll do the IR exploration and other issues in a follow up.

…onstruct the object set definition very easily

nihalbhatnagar · 2025-01-15T23:22:15Z

I updated the types in the API to improve the hover readability. This actually unlocked the ability to super easily have users define a type for their object set with added properties, so now they can write functions that expect certain property types to be on object sets.

This is what the hover over an object set looks like now:

Hovering over the .withProperties method still looks wonky, but we need to capture the generic in this clause.

Here's what the extracted type looks like on hover:

There's some more type fiddling to be done here to try and maintain consistency across all three of these scenarios.

nihalbhatnagar · 2025-01-15T23:25:25Z

packages/client/src/objectSet/ObjectSet.test.ts

+        let objectSet: ObjectSetWithProperties<
+          Employee,
+          { "hello": PropertyDef<"string", "nullable", "single"> }
+        >;


I'm using the already exported PropertyDef type to have the users define their types. This is an example of how a user would type their function or anything they want

nihalbhatnagar · 2025-01-15T23:28:31Z

packages/api/src/objectSet/ObjectSet.ts

 }
+
+export type ObjectSetWithProperties<


This name needs to be discussed. It is very similar to the WithPropertiesObjectSet type that we already export. I like this name for this type, so I think we should likely rename the WithPropertiesObjectSet to PropertyDefinitionObjectSet. Maybe this name needs to describe that you can define the type of your object set more? Thoughts?

nihalbhatnagar · 2025-01-15T23:40:58Z

packages/client/src/objectSet/ObjectSet.test.ts

+          "hello": (base) => base.pivotTo("lead").selectProperty("fullName"),
+        });
+
+        type extractedType = typeof objectSet;


Heres how you extract the type from an object set with derived properties on it

nihalbhatnagar · 2025-01-16T15:07:16Z

packages/api/src/objectSet/ObjectSet.ts

+    {
+      [K in keyof D]: D[K] extends
+        (baseObjectSet: any) => WithPropertyDefinition<infer P> ? PropertyDef<
+          P["type"],


The user facing type expects a PropertyDef instead of an ObjectMetadata.Property. I chose to localize it here instead of 2 places when extracting the type from the object type definition

* Rework types * fix test * Cleanup RDP names

nihalbhatnagar · 2025-01-29T02:19:30Z

Updated Osdk.Instance types look like this:

nihalbhatnagar · 2025-01-29T02:20:54Z

Updated Builder name looks like this:

ssanjay1 · 2025-01-30T15:37:19Z

packages/api/src/objectSet/ObjectSet.ts

+        ExtractOptions<R, S>,
+        PropertyKeys<B> extends L ? PropertyKeys<B>
+          : Extract<L, PropertyKeys<B>>,
+        Y extends Record<string, SimplePropertyDef>


nit: should we possible pull any of this into helper types considering its used throughout this file?

ssanjay1 · 2025-01-30T15:38:04Z

packages/client/src/object/convertWireToOsdkObjects.test.ts

@@ -205,12 +205,12 @@ describe("convertWireToOsdkObjects", () => {
      "userAgent",
    );

-    let object = {
+    const object = {


This is just ESlint stuff right?

I think so, I don't remember changing this

ssanjay1 · 2025-01-30T15:42:04Z

packages/api/src/OsdkObjectFrom.ts

  > =
    & OsdkBase<Q>
    & Pick<
      CompileTimeMetadata<Q>["props"],
-      GetPropsKeys<Q, P>
+      // If there aren't any additional properties, then we want GetPropsKeys to default to PropertyKeys<Q>
+      GetPropsKeys<Q, P, [R] extends [never] ? false : true>


Mmm, I'm a bit confused and probably missing something here, what if they had downselected the original object props?

Yeah I think I am confused too. Do we have a test that shows what this? NM. I saw a test down below.

Mmm ok yea so looking closer, you only default if P is also never and they never downselected

Yeah thats correct, this is to make sure passing in never still defaults to PropertyKeys<Q> but also allows you to pass in a never if you choose to add an RDP.

ericanderson

Alright I left a couple of nits. I'm mostly on board. Lets iterate on the type names/hierarchy for the builders.

I'm also a little bit curious about whether we can reuse the 2nd parameter of Osdk.Instance instead of adding a 4th one but I definitely need to think about back compat scenarios for that.

I also think we need to clean up the types for ObjectSet now as we are carrying a lot of information thats hard to follow.

ericanderson · 2025-01-30T16:17:42Z

packages/api/src/OsdkObjectFrom.ts

  > =
    & OsdkBase<Q>
    & Pick<
      CompileTimeMetadata<Q>["props"],
-      GetPropsKeys<Q, P>
+      // If there aren't any additional properties, then we want GetPropsKeys to default to PropertyKeys<Q>
+      GetPropsKeys<Q, P, [R] extends [never] ? false : true>


Yeah I think I am confused too. Do we have a test that shows what this? NM. I saw a test down below.

ericanderson · 2025-01-30T16:24:50Z

packages/api/src/derivedProperties/DerivedProperty.ts

@@ -114,18 +125,15 @@ interface SingleLinkWithPropertyObjectSet<
 }

 /*
- * The ManyLinkWithPropertyObjectSet overrides the pivotTo operation because once we traverse a single link,
+ * The DefaultBuilder overrides the pivotTo operation because once we traverse a single link,


I want to bike shed this name because when I first saw it I was a bit confused about what was default vs not.

Hmm yeah I wrote the name more with a "coming off the tounge nicely" rather than being super informative because I don't imagine any of our users will care about the type of the RDP object set. If the goal is for more readable code, I can add more comments?

ericanderson · 2025-01-30T16:28:01Z

packages/api/src/derivedProperties/DerivedProperty.ts

-  BaseWithPropertyObjectSet<Q>
+  AggregatableBuilder<Q>,
+  FilterableBuilder<Q>,
+  DerivedProperty.Builder<Q>


I'm confused a bit on the hierarchy. Why are some things in the namespace and others not?

ericanderson · 2025-01-30T16:34:51Z

packages/api/src/objectSet/ObjectSet.test.ts

+              Employee,
+              never,
+              "class",
+              { mom: "integer" | undefined }


I see. So the third param still means exactly original properties and the forth param gets reduced based on what was selected.

This makes sense, ignore my other comment about a test for this.

ericanderson · 2025-01-30T16:36:42Z

packages/api/src/objectSet/ObjectSet.ts

+        ExtractOptions<R, S>,
+        PropertyKeys<B> extends L ? PropertyKeys<B>
+          : Extract<L, PropertyKeys<B>>,
+        Y extends Record<string, SimplePropertyDef>


ericanderson · 2025-01-30T16:37:31Z

packages/api/src/objectSet/ObjectSet.ts

@@ -79,8 +84,15 @@ export interface MinimalObjectSet<Q extends ObjectOrInterfaceDefinition>
    args?: FetchPageArgs<Q, L, R, A, S>,
  ) => Promise<
    PageResult<
-      PropertyKeys<Q> extends L ? Osdk.Instance<Q, ExtractOptions<R, S>>


Looks like this is removing the simplified version of the output.

Hmm I may be wrong here but I wrote this so that it defaults to PropertyKeys<employee> if it's the full property set, so this should be matching the behavior of the top right?

ericanderson · 2025-01-30T16:40:57Z

packages/api/src/objectSet/ObjectSet.ts

+        Q,
+        L,
+        R,
+        S


nit: unnecessary formatting change expanding SelectArg<>

ericanderson · 2025-01-30T16:45:16Z

packages/api/src/objectSet/ObjectSet.ts

+        Q,
+        L,
+        R,
+        S


nit: unnecessary formatting change expanding SelectArg<>

ericanderson · 2025-01-30T16:49:17Z

packages/api/src/objectSet/ObjectSet.ts

+    B,
+    any
+  >,
+> extends MinimalObjectSet<Q, B, Y> {


So I did this ObjectSetPrime thing to avoid needing to rewrite a ton of the types because everything south of where I was could just work on the modified ObjectSet that we got by MergeObjectSet. I'm not sure that is still worth it, especially since we went up passing both the merge and the non-merged around.

Hmm so maybe we should consider merging ObjectSet.fns back into here? Also think we need to discuss MinimalObjectSet because thats really only used for interfaces but they should have feature parity at this point? Maybe we need to just extract out intersect/union and get rid of Minimal(merge everything but intersect/union into regular ObjectSet)

Now that Im in here playing with types, one thing that ObjectSetPrime got me was knowing for sure what the second param was (removing the chance that its the value from the definition) which I think complicated your types a bit.

I think splitting each function out might be the best move here because the different between Minimal vs not vs interface etc is really confusing. Experimenting

ericanderson · 2025-01-30T16:50:35Z

packages/client/src/objectSet/ObjectSet.test.ts

-        expectTypeOf(order2).toEqualTypeOf<typeof order1>();
-        expectTypeOf(order1).toEqualTypeOf<typeof order2>();
-      });
-    });


Why delete all of these tests?

I felt that these tests were a repeat of the type tests we had present in the tests located in the api package now, so I wanted to cut down on the clutter

Looking back the last test (testing the order) may not be explicitly tested for anymore so I will add that to the API tests. With regards to the others, I felt like the extra "expect ts error" checks weren't providing much value as we're already checking the type

ssanjay1

Naming stuff seems fine to me now, approving to unblock

nihalbhatnagar added 8 commits December 4, 2024 09:47

Runtime Derived Property Syntax

246f893

Working derive clause

50ca838

Update to use map

4e17a4e

Update aggregation keys

01f5935

Update

ebe19e4

Update PR

4ea27dc

Initial test suite

d2d0a0d

Working version

efbed3d

nihalbhatnagar marked this pull request as ready for review December 9, 2024 21:05

Cleanup

40ee625

nihalbhatnagar added the do not merge label Dec 9, 2024

nihalbhatnagar added 3 commits December 9, 2024 17:00

Add test case

3e535da

remove use of crypto

20835ea

Changeset

76a3ee1

nihalbhatnagar commented Dec 9, 2024

View reviewed changes

nihalbhatnagar added 3 commits December 10, 2024 09:43

E2E testing

73c23fe

Osdk.Instance typing update

d605787

Rename and cleanup

214dad7

nihalbhatnagar removed the do not merge label Dec 10, 2024

nihalbhatnagar commented Dec 10, 2024

View reviewed changes

Fix UUID

73fbffa

nihalbhatnagar commented Dec 10, 2024

View reviewed changes

packages/api/src/objectSet/ObjectSet.ts Outdated Show resolved Hide resolved

ssanjay1 reviewed Dec 13, 2024

View reviewed changes

nihalbhatnagar added 3 commits December 13, 2024 17:30

Quick fixes

9070c29

Update aggregations syntax

ac7f03f

Merge branch 'main' into nihalb/rdp-v1

2777334

nihalbhatnagar commented Dec 16, 2024

View reviewed changes

ericanderson reviewed Jan 9, 2025

View reviewed changes

nihalbhatnagar added 3 commits January 15, 2025 17:01

Updated types to produce a shorter definition and also allows us to c…

a2f331d

…onstruct the object set definition very easily

Updated tests

bec53ca

Update tests

d8fa066

nihalbhatnagar commented Jan 15, 2025

View reviewed changes

Merge branch 'main' into nihalb/rdp-v1

ecb0522

nihalbhatnagar commented Jan 15, 2025

View reviewed changes

Update to no longer use definitionId

abc0b2d

nihalbhatnagar commented Jan 16, 2025

View reviewed changes

nihalbhatnagar and others added 5 commits January 16, 2025 14:36

Additional type test

831f890

Rework RDP types (#1113)

a270994

* Rework types * fix test * Cleanup RDP names

Reorganize files and rename Rdp to DerivedProperty

7a8847f

Osdk.Instance<> for RDP's

2659b3b

Refactor tests

1167fae

nihalbhatnagar force-pushed the nihalb/rdp-v1 branch from 30788b1 to 1167fae Compare January 29, 2025 01:34

nihalbhatnagar added 2 commits January 28, 2025 20:54

Merge branch 'main' into nihalb/rdp-v1

330dac1

Add tests back

eb99bab

Remove comment

c5686e0

ssanjay1 reviewed Jan 30, 2025

View reviewed changes

ericanderson reviewed Jan 30, 2025

View reviewed changes

ericanderson and others added 4 commits January 30, 2025 13:58

Simplify types

624b662

slim the delta more

2812538

Update names of builders

c2c675f

Use types instead of interfaces to support better hover

95829f8

ssanjay1 approved these changes Feb 7, 2025

View reviewed changes

RDP's in TS OSDK #1035

Are you sure you want to change the base?

RDP's in TS OSDK #1035

Conversation

nihalbhatnagar commented Dec 5, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nihalbhatnagar Dec 10, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ssanjay1 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nihalbhatnagar Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nihalbhatnagar Dec 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericanderson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nihalbhatnagar commented Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

nihalbhatnagar Jan 15, 2025 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nihalbhatnagar Jan 16, 2025 • edited Loading

Choose a reason for hiding this comment

nihalbhatnagar commented Jan 29, 2025

nihalbhatnagar commented Jan 29, 2025

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nihalbhatnagar Jan 30, 2025 • edited Loading

Choose a reason for hiding this comment

ericanderson left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

nihalbhatnagar commented Dec 5, 2024 •

edited

Loading

nihalbhatnagar Dec 10, 2024 •

edited

Loading

nihalbhatnagar Dec 13, 2024 •

edited

Loading

nihalbhatnagar Dec 13, 2024 •

edited

Loading

nihalbhatnagar commented Jan 15, 2025 •

edited

Loading

nihalbhatnagar Jan 15, 2025 •

edited

Loading

nihalbhatnagar Jan 16, 2025 •

edited

Loading

nihalbhatnagar Jan 30, 2025 •

edited

Loading