Skip to content

Commit

Permalink
OSDK learns __EXPERIMENTAL_strictNonNull to throw, drop objects, or r…
Browse files Browse the repository at this point in the history
…eturn `| undefined` for properties, allowing for correct typesafety. (#387)
  • Loading branch information
ericanderson authored Jun 12, 2024
1 parent 413e511 commit b3563e0
Show file tree
Hide file tree
Showing 22 changed files with 912 additions and 112 deletions.
5 changes: 5 additions & 0 deletions .changeset/purple-cameras-invite.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@osdk/client": minor
---

OSDK learns \_\_EXPERIMENTAL_strictNonNull to throw, drop objects, or return `| undefined` for properties, allowing for correct typesafety.
61 changes: 61 additions & 0 deletions examples-extra/basic/cli/src/demoStrictness.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
/*
* Copyright 2024 Palantir Technologies, Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import type { Osdk } from "@osdk/client";
import {
BoundariesUsState,
Employee,
FooInterface,
} from "@osdk/examples.basic.sdk";
import { expectType } from "ts-expect";
import { client } from "./client.js";

export async function demoStrictnessObject() {
const { data: defaultResults } = await client(BoundariesUsState)
.fetchPage();
expectType<string>(defaultResults[0].usState);

const { data: dropResults } = await client(BoundariesUsState)
.fetchPage({ $__EXPERIMENTAL_strictNonNull: "drop" });
expectType<string>(dropResults[0].usState);

const { data: notStrictResults } = await client(BoundariesUsState)
.fetchPage({ $__EXPERIMENTAL_strictNonNull: false });
expectType<string | undefined>(notStrictResults[0].usState);

const { data: throwResults } = await client(BoundariesUsState)
.fetchPage({ $__EXPERIMENTAL_strictNonNull: "throw" });
expectType<string>(throwResults[0].usState);

const { data: fooDataNotStrict } = await client(FooInterface)
.fetchPage({ $__EXPERIMENTAL_strictNonNull: false });

const employeeNotStrict = fooDataNotStrict[0].$as(Employee);
}

export async function demoStrictnessInterface() {
const { data: fooDataNotStrict } = await client(FooInterface)
.fetchPage({ $__EXPERIMENTAL_strictNonNull: false });

const employeeNotStrict = fooDataNotStrict[0].$as(Employee);
expectType<
Osdk<
Employee,
"$notStrict" | "firstName" | "email",
"$notStrict" | "firstName" | "email"
>
>(employeeNotStrict);
}
1 change: 1 addition & 0 deletions packages/api/src/ontology/ObjectOrInterface.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ export type ObjectOrInterfaceDefinition<
| ObjectTypeDefinition<K>
| InterfaceDefinition<K, L>;

/** @deprecated */
export type ObjectOrInterfacePropertyKeysFrom<
O extends OntologyDefinition<any, any>,
K extends ObjectOrInterfaceKeysFrom<O>,
Expand Down
55 changes: 55 additions & 0 deletions packages/client/src/Definitions.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
/*
* Copyright 2024 Palantir Technologies, Inc. All rights reserved.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

import type { ObjectTypePropertyDefinition } from "@osdk/api";
import { describe, expectTypeOf, it } from "vitest";
import type { OsdkObjectPropertyType } from "./Definitions.js";

describe("OsdkObjectPropertyType", () => {
describe("{ nullable: false } property", () => {
const nonNullDef = {
type: "string",
nullable: false,
} satisfies ObjectTypePropertyDefinition;

it("is `| undefined` for `false`", () => {
expectTypeOf<OsdkObjectPropertyType<typeof nonNullDef, false>>()
.toEqualTypeOf<string | undefined>();
});

it("is not `| undefined` for `true`", () => {
expectTypeOf<OsdkObjectPropertyType<typeof nonNullDef, true>>()
.toEqualTypeOf<string>();
});
});

describe("{ nullable: true } property", () => {
const nullableDef = {
type: "string",
nullable: true,
} satisfies ObjectTypePropertyDefinition;

it("is | undefined for `false`", () => {
expectTypeOf<OsdkObjectPropertyType<typeof nullableDef, false>>()
.toEqualTypeOf<string | undefined>();
});

it("is `| undefined` for `true`", () => {
expectTypeOf<OsdkObjectPropertyType<typeof nullableDef, true>>()
.toEqualTypeOf<string | undefined>();
});
});
});
12 changes: 10 additions & 2 deletions packages/client/src/Definitions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,16 @@ type MaybeNullable<T extends ObjectTypePropertyDefinition, U> =
type Raw<T> = T extends Array<any> ? T[0] : T;
type Converted<T> = T extends Array<any> ? T[1] : T;

export type OsdkObjectPropertyType<T extends ObjectTypePropertyDefinition> =
MaybeNullable<
/**
* @param {T} ObjectTypePropertyDefinition in literal form
* @param {STRICTLY_ENFORCE_NULLABLE} S for strict. If false, always `|undefined`
*/
export type OsdkObjectPropertyType<
T extends ObjectTypePropertyDefinition,
STRICTLY_ENFORCE_NULLABLE extends boolean = true,
> = STRICTLY_ENFORCE_NULLABLE extends false
? MaybeArray<T, Converted<PropertyValueWireToClient[T["type"]]>> | undefined
: MaybeNullable<
T,
MaybeArray<T, Converted<PropertyValueWireToClient[T["type"]]>>
>;
Expand Down
88 changes: 81 additions & 7 deletions packages/client/src/OsdkObjectFrom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,24 @@ describe("ConvertProps", () => {
expectTypeOf<ConvertProps<FooInterface, Employee, "$all">>()
.toEqualTypeOf<"fullName">();
});

it("converts non-strict nullness for handles single prop", () => {
expectTypeOf<
ConvertProps<
FooInterface,
Employee,
| "fooSpt"
| "$notStrict"
>
>()
.toEqualTypeOf<"fullName" | "$notStrict">();
});
it("converts non-strict nullness for $all", () => {
expectTypeOf<
ConvertProps<FooInterface, Employee, "$all" | "$notStrict">
>()
.toEqualTypeOf<"fullName" | "$notStrict">();
});
});

describe("converts from a concrete to an interface", () => {
Expand All @@ -43,6 +61,27 @@ describe("ConvertProps", () => {
expectTypeOf<ConvertProps<Employee, FooInterface, "peeps">>()
.toEqualTypeOf<never>();
});

it("resolves to never for unused when not strict", () => {
expectTypeOf<
ConvertProps<Employee, FooInterface, "peeps" | "$notStrict">
>()
.toEqualTypeOf<never>();
});

it("converts non-strict nullness for single prop", () => {
expectTypeOf<
ConvertProps<Employee, FooInterface, "fullName" | "$notStrict">
>()
.toEqualTypeOf<"fooSpt" | "$notStrict">();
});

it("converts non-strict nullness for $all", () => {
expectTypeOf<
ConvertProps<Employee, FooInterface, "$all" | "$notStrict">
>()
.toEqualTypeOf<"$all" | "$notStrict">();
});
});

describe("multiprop", () => {
Expand All @@ -52,6 +91,17 @@ describe("ConvertProps", () => {
>()
.toEqualTypeOf<"fooSpt">();
});

it("resolves to known only when not strict", () => {
expectTypeOf<
ConvertProps<
Employee,
FooInterface,
"peeps" | "fullName" | "$notStrict"
>
>()
.toEqualTypeOf<"fooSpt" | "$notStrict">();
});
});

it("handles $all", () => {
Expand Down Expand Up @@ -88,7 +138,7 @@ describe("Osdk", () => {
it("can't properly compare the retained types without it", () => {
type InvalidPropertyName = "not a real prop";

// This should fail but it doesnt. We dont actively capture Z
// This should fail but it doesn't. We don't actively capture Z
// (and we cant)
expectTypeOf<
OsdkAsHelper<Employee, "fullName", "fullName", FooInterface>
Expand Down Expand Up @@ -117,18 +167,42 @@ describe("Osdk", () => {
});
});

describe("$notStrict", () => {
describe("is present", () => {
it("makes { nullable: false } as `|undefined`", () => {
expectTypeOf<
Osdk<Employee, "employeeId" | "$notStrict">["employeeId"]
>()
.toEqualTypeOf<number | undefined>();
expectTypeOf<
GetUnderlyingProps<
OsdkAsHelper<Employee, "fullName", "fullName", FooInterface>
>
>().toEqualTypeOf<"fullName">();
});
});

describe("is absent", () => {
it("makes { nullable: false } as NOT `|undefined`", () => {
expectTypeOf<
Osdk<Employee, "employeeId">["employeeId"]
>()
.toEqualTypeOf<number>();
expectTypeOf<
GetUnderlyingProps<
OsdkAsHelper<Employee, "fullName", "fullName", FooInterface>
>
>().toEqualTypeOf<"fullName">();
});
});
});

it("Converts into self properly", () => {
expectTypeOf<
GetUnderlyingProps<
OsdkAsHelper<FooInterface, "fooSpt", "fullName", FooInterface>
>
>().toEqualTypeOf<"fullName">();

type z = Osdk<FooInterface, "fooSpt">;
type FM<T extends any[]> = T extends (infer P)[] ? P : never;
type zz = Awaited<
Promise<FetchPageResult<FooInterface, "fooSpt", false>>
>["data"];
});

it("retains original props if set", () => {
Expand Down
66 changes: 48 additions & 18 deletions packages/client/src/OsdkObjectFrom.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,17 @@ import type {
import type { OsdkBase, OsdkObjectPrimaryKeyType } from "@osdk/client.api";
import type { OsdkObjectPropertyType } from "./Definitions.js";
import type { OsdkObjectLinksObject } from "./definitions/LinkDefinitions.js";
import type { UnionIfTrue } from "./object/FetchPageResult.js";

type DropRidAndAll<T extends string> = Exclude<T, "$rid" | "$all">;
type DropDollarOptions<T extends string> = Exclude<
T,
"$rid" | "$all" | "$notStrict"
>;

type DropDollarAll<T extends string> = Exclude<
T,
"$all"
>;

type ApiNameAsString<T extends ObjectOrInterfaceDefinition> = NonNullable<
T["apiName"]["__Unbranded"]
Expand All @@ -42,31 +51,46 @@ export type ConvertProps<
> = TO extends FROM ? P
: TO extends ObjectTypeDefinition<any> ? (
(
TO["interfaceMap"][ApiNameAsString<FROM>][
P extends "$all"
? keyof FROM["properties"] extends
keyof TO["interfaceMap"][ApiNameAsString<FROM>]
? keyof FROM["properties"]
: never
: DropRidAndAll<P>
]
UnionIfTrue<
TO["interfaceMap"][ApiNameAsString<FROM>][
P extends "$all" ? (
keyof FROM["properties"] extends
keyof TO["interfaceMap"][ApiNameAsString<FROM>]
? keyof FROM["properties"]
: never
)
: DropDollarOptions<P>
],
P extends "$notStrict" ? true : false,
"$notStrict"
>
)
)
: TO extends InterfaceDefinition<any> ? P extends "$all" ? "$all"
: FROM extends ObjectTypeDefinition<any>
? DropRidAndAll<P> extends keyof FROM["inverseInterfaceMap"][
ApiNameAsString<TO>
] ? FROM["inverseInterfaceMap"][ApiNameAsString<TO>][DropRidAndAll<P>]
: UnionIfTrue<
TO extends InterfaceDefinition<any> ? P extends "$all" ? "$all"
: FROM extends ObjectTypeDefinition<any>
? DropDollarOptions<P> extends keyof FROM["inverseInterfaceMap"][
ApiNameAsString<TO>
] ? FROM["inverseInterfaceMap"][ApiNameAsString<TO>][
DropDollarOptions<P>
]
: never
: never
: never
: never;
: never,
P extends "$notStrict" ? true : false,
"$notStrict"
>;

/** DO NOT EXPORT FROM PACKAGE */
export type ValidToFrom<FROM extends ObjectOrInterfaceDefinition> = FROM extends
InterfaceDefinition<any, any>
? ObjectTypeDefinition<any> | InterfaceDefinition<any, any>
: InterfaceDefinition<any, any>;

/**
* @param P The properties to add from Q
* @param Z The existing underlying properties
*/
type UnderlyingProps<
Q extends ObjectOrInterfaceDefinition,
P extends string,
Expand Down Expand Up @@ -98,8 +122,14 @@ export type Osdk<
)
]: IsNever<P> extends true
// when we don't know what properties, we will show all but ensure they are `| undefined`
? OsdkObjectPropertyType<Q["properties"][PP]> | undefined
: OsdkObjectPropertyType<Q["properties"][PP]>;
? OsdkObjectPropertyType<
Q["properties"][PP],
false // P is never so we do not have to check for "$notStrict"
>
: OsdkObjectPropertyType<
Q["properties"][PP],
P extends "$notStrict" ? false : true
>;
}
& {
/** @deprecated use $apiName */
Expand Down
Loading

0 comments on commit b3563e0

Please sign in to comment.