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.

Sign up for GitHub

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

URI generics default to Uri; ConfigBuilder accepts string | Uri where possible #81

Open
wants to merge 5 commits into
base: origin-dev
Choose a base branch
from
Open

URI generics default to Uri; ConfigBuilder accepts string | Uri where possible #81

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
47f660b
made Uri default type of TUri generic in client; config builder now a…
krisbitney Sep 14, 2023
869670f
Merge remote-tracking branch 'origin/origin-dev' into kris/uri-type-i…
krisbitney Sep 14, 2023
4aa7c5a
Merge remote-tracking branch 'origin/origin-dev' into kris/uri-type-i…
krisbitney Oct 20, 2023
16d6ec4
using union type instead of generics for Uri
krisbitney Oct 31, 2023
148364f
Merge remote-tracking branch 'origin/origin-dev' into kris/uri-type-i…
krisbitney Oct 31, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions packages/client/src/PolywrapClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,40 +75,37 @@ export class PolywrapClient extends PolywrapCoreClient {
}

@Tracer.traceMethod("PolywrapClient: getEnvByUri")
public getEnvByUri<TUri extends Uri | string = string>(
public getEnvByUri<TUri extends Uri | string = Uri>(
uri: TUri
): WrapperEnv | undefined {
return super.getEnvByUri(Uri.from(uri));
}

@Tracer.traceMethod("PolywrapClient: getManifest")
public async getManifest<TUri extends Uri | string = string>(
public async getManifest<TUri extends Uri | string = Uri>(
uri: TUri
): Promise<Result<WrapManifest, WrapError>> {
return super.getManifest(Uri.from(uri));
}

@Tracer.traceMethod("PolywrapClient: getFile")
public async getFile<TUri extends Uri | string = string>(
public async getFile<TUri extends Uri | string = Uri>(
uri: TUri,
options: GetFileOptions
): Promise<Result<string | Uint8Array, WrapError>> {
return super.getFile(Uri.from(uri), options);
}

@Tracer.traceMethod("PolywrapClient: getImplementations")
public async getImplementations<TUri extends Uri | string = string>(
public async getImplementations<TUri extends Uri | string = Uri>(
uri: TUri,
options?: GetImplementationsOptions
): Promise<Result<Uri[], WrapError>> {
return super.getImplementations(Uri.from(uri), options);
}

@Tracer.traceMethod("PolywrapClient: invokeWrapper")
public async invokeWrapper<
TData = unknown,
TUri extends Uri | string = string
>(
public async invokeWrapper<TData = unknown, TUri extends Uri | string = Uri>(
options: InvokerOptions<TUri> & { wrapper: Wrapper }
): Promise<InvokeResult<TData>> {
return super.invokeWrapper({
Expand All @@ -118,7 +115,7 @@ export class PolywrapClient extends PolywrapCoreClient {
}

@Tracer.traceMethod("PolywrapClient: invoke")
public async invoke<TData = unknown, TUri extends Uri | string = string>(
public async invoke<TData = unknown, TUri extends Uri | string = Uri>(
Copy link
Contributor

Choose a reason for hiding this comment

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

Changing the default to Uri doesn't fix #79 , it just flips the coin and requires you to have a Uri instance if you specify a return type.

The problem is that if you specialize the generic with the first template argument, the second one appears to be required. Instead of using a template argument for TUri maybe we should just make the actual argument type options: InvokerOptions<Uri | string> and we can determine the type within the function's body. This way if you specialize the first template argument, there isn't a second one that needs to be specified as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Okay, my mistake. I misunderstood. I read "...instead of String we should support that the default is Uri" and thought we just wanted to modify the generic type. I changed everything to use union types instead, so we should be good to go.

options: InvokerOptions<TUri>
): Promise<InvokeResult<TData>> {
return super.invoke({
Expand All @@ -128,7 +125,7 @@ export class PolywrapClient extends PolywrapCoreClient {
}

@Tracer.traceMethod("PolywrapClient: tryResolveUri")
public async tryResolveUri<TUri extends Uri | string = string>(
public async tryResolveUri<TUri extends Uri | string = Uri>(
options: TryResolveUriOptions<TUri>
): Promise<Result<UriPackageOrWrapper, unknown>> {
return super.tryResolveUri({
Expand Down
4 changes: 2 additions & 2 deletions packages/client/src/__tests__/core/embedded-package.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ import fs from "fs";
import path from "path";
import { GetPathToTestWrappers } from "@polywrap/test-cases";
import { InMemoryFileReader, WasmPackage } from "@polywrap/wasm-js";
import { IWrapPackage } from "@polywrap/core-js";
import {IWrapPackage, Uri} from "@polywrap/core-js";
import { Result, ResultErr, ResultOk } from "@polywrap/result";
import { PolywrapClient } from "../../PolywrapClient";
import { PolywrapClientConfigBuilder } from "@polywrap/client-config-builder-js";

jest.setTimeout(200000);

const wrapperPath = `${GetPathToTestWrappers()}/subinvoke/00-subinvoke/implementations/as`;
const wrapperUri = `fs/${wrapperPath}`;
const wrapperUri = Uri.from(`fs/${wrapperPath}`);

describe("Embedded package", () => {
it("can invoke an embedded package", async () => {
Expand Down
4 changes: 2 additions & 2 deletions packages/client/src/__tests__/core/embedded-wrapper.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,15 +2,15 @@ import fs from "fs";
import path from "path";
import { GetPathToTestWrappers } from "@polywrap/test-cases";
import { WasmWrapper, InMemoryFileReader } from "@polywrap/wasm-js";
import { Wrapper } from "@polywrap/core-js";
import {Uri, Wrapper} from "@polywrap/core-js";
import { Result, ResultErr, ResultOk } from "@polywrap/result";
import { PolywrapClient } from "../../PolywrapClient";
import { PolywrapClientConfigBuilder } from "@polywrap/client-config-builder-js";

jest.setTimeout(200000);

const wrapperPath = `${GetPathToTestWrappers()}/subinvoke/00-subinvoke/implementations/as`;
const simpleWrapperUri = `fs/${wrapperPath}`;
const simpleWrapperUri = Uri.from(`fs/${wrapperPath}`);

describe("Embedded wrapper", () => {
it("can invoke an embedded wrapper", async () => {
Expand Down
34 changes: 17 additions & 17 deletions packages/client/src/__tests__/core/error-structure.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ describe("Error structure", () => {
describe("URI resolution", () => {
test("Invoke a wrapper that is not found", async () => {
const client = new PolywrapClient();
const result = await client.invoke<string>({
const result = await client.invoke<string, string>({
uri: asSubinvokeWrapperUri.uri + "-not-found",
method: "simpleMethod",
args: {
Expand All @@ -61,7 +61,7 @@ describe("Error structure", () => {
test("Subinvoke a wrapper that is not found", async () => {
const client = new PolywrapClient();
const result = await client.invoke<number>({
uri: asConsumerWrapperUri.uri,
uri: asConsumerWrapperUri,
method: "throwError",
args: {
a: "Hey",
Expand Down Expand Up @@ -99,7 +99,7 @@ describe("Error structure", () => {
test("Invoke a wrapper with malformed arguments", async () => {
const client = new PolywrapClient();
const result = await client.invoke<string>({
uri: asSubinvokeWrapperUri.uri,
uri: asSubinvokeWrapperUri,
method: "add",
args: {
a: "1",
Expand Down Expand Up @@ -130,7 +130,7 @@ describe("Error structure", () => {
test("Invoke a wrapper method that doesn't exist", async () => {
const client = new PolywrapClient();
const result = await client.invoke<string>({
uri: asSubinvokeWrapperUri.uri,
uri: asSubinvokeWrapperUri,
method: "notExistingMethod",
args: {
arg: "test",
Expand Down Expand Up @@ -165,13 +165,13 @@ describe("Error structure", () => {
const config = new PolywrapClientConfigBuilder()
.addDefaults()
.setRedirects({
"authority/imported-invoke": asInvokeWrapperUri.uri,
"authority/imported-subinvoke": asSubinvokeWrapperUri.uri,
"authority/imported-invoke": asInvokeWrapperUri,
"authority/imported-subinvoke": asSubinvokeWrapperUri,
})
.build();
const client = new PolywrapClient(config);
const result = await client.invoke<boolean>({
uri: asConsumerWrapperUri.uri,
uri: asConsumerWrapperUri,
method: "throwError",
args: {
a: "Hey",
Expand Down Expand Up @@ -249,7 +249,7 @@ describe("Error structure", () => {
});
test("Invoke a wrapper with incompatible version", async () => {
const client = new PolywrapClient();
const result = await client.invoke<string>({
const result = await client.invoke<string, string>({
uri: "wrap://fs/tmp",
method: "simpleMethod",
});
Expand All @@ -275,7 +275,7 @@ describe("Error structure", () => {
test("Invoke a wrapper with malformed arguments", async () => {
const client = new PolywrapClient();
const result = await client.invoke<string>({
uri: rsSubinvokeWrapperUri.uri,
uri: rsSubinvokeWrapperUri,
method: "add",
args: {
a: "1",
Expand Down Expand Up @@ -306,7 +306,7 @@ describe("Error structure", () => {
test("Invoke a wrapper method that doesn't exist", async () => {
const client = new PolywrapClient();
const result = await client.invoke<string>({
uri: rsSubinvokeWrapperUri.uri,
uri: rsSubinvokeWrapperUri,
method: "notExistingMethod",
args: {
arg: "test",
Expand Down Expand Up @@ -341,14 +341,14 @@ describe("Error structure", () => {
const config = new PolywrapClientConfigBuilder()
.addDefaults()
.setRedirects({
"authority/imported-invoke": rsInvokeWrapperUri.uri,
"authority/imported-subinvoke": rsSubinvokeWrapperUri.uri,
"authority/imported-invoke": rsInvokeWrapperUri,
"authority/imported-subinvoke": rsSubinvokeWrapperUri,
})
.build();

const client = new PolywrapClient(config);
const result = await client.invoke<number>({
uri: rsConsumerWrapperUri.uri,
uri: rsConsumerWrapperUri,
method: "throwError",
args: {
a: "Hey",
Expand Down Expand Up @@ -414,7 +414,7 @@ describe("Error structure", () => {

test("Invoke a plugin wrapper with malformed args", async () => {
const client = await createClient();
const result = await client.invoke<Uint8Array>({
const result = await client.invoke<Uint8Array, string>({
uri: SysBundle.bundle.fileSystem.uri,
method: "readFile",
args: {
Expand Down Expand Up @@ -444,7 +444,7 @@ describe("Error structure", () => {

test("Invoke a plugin wrapper with a method that doesn't exist", async () => {
const client = await createClient();
const result = await client.invoke<Uint8Array>({
const result = await client.invoke<Uint8Array, string>({
uri: SysBundle.bundle.fileSystem.uri,
method: "readFileNotFound",
args: {
Expand All @@ -468,7 +468,7 @@ describe("Error structure", () => {

test("Invoke a plugin wrapper that throws explicitly", async () => {
const client = await createClient();
const result = await client.invoke<string>({
const result = await client.invoke<string, string>({
uri: "wrap://plugin/mock",
method: "methodThatThrows",
});
Expand All @@ -493,7 +493,7 @@ describe("Error structure", () => {

test("Invoke a plugin wrapper that throws unexpectedly", async () => {
const client = await createClient();
const result = await client.invoke<Uint8Array>({
const result = await client.invoke<Uint8Array, string>({
uri: SysBundle.bundle.fileSystem.uri,
method: "readFile",
args: {
Expand Down
7 changes: 4 additions & 3 deletions packages/client/src/__tests__/core/type-test-cases.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import BigNumber from "bignumber.js";
import { PolywrapClientConfigBuilder } from "@polywrap/client-config-builder-js";
import { GetPathToTestWrappers } from "@polywrap/test-cases";
import { ResultOk } from "@polywrap/result";
import {Uri} from "@polywrap/core-js";

export const typeTestCases = (implementation: string) => {
describe("types test cases", () => {
Expand All @@ -18,7 +19,7 @@ export const typeTestCases = (implementation: string) => {
.build();
const client = new PolywrapClient(config);

const uri = `fs/${GetPathToTestWrappers()}/asyncify/implementations/${implementation}`;
const uri = Uri.from(`fs/${GetPathToTestWrappers()}/asyncify/implementations/${implementation}`);
const subsequentInvokesResult = await client.invoke({
uri: uri,
method: "subsequentInvokes",
Expand Down Expand Up @@ -343,7 +344,7 @@ export const typeTestCases = (implementation: string) => {
});

test(`json-type ${implementation}`, async () => {
const uri = `fs/${GetPathToTestWrappers()}/json-type/implementations/${implementation}`;
const uri = Uri.from(`fs/${GetPathToTestWrappers()}/json-type/implementations/${implementation}`);
const client = new PolywrapClient();
type Json = string;
const value = JSON.stringify({ foo: "bar", bar: "bar" });
Expand Down Expand Up @@ -675,7 +676,7 @@ export const typeTestCases = (implementation: string) => {
});

test(`map-type ${implementation}`, async () => {
const uri = `fs/${GetPathToTestWrappers()}/map-type/implementations/${implementation}`;
const uri = Uri.from(`fs/${GetPathToTestWrappers()}/map-type/implementations/${implementation}`);
const client = new PolywrapClient();
const mapClass = new Map<string, number>()
.set("Hello", 1)
Expand Down
4 changes: 2 additions & 2 deletions packages/client/src/__tests__/core/wasm-wrapper.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ describe("wasm-wrapper", () => {
test("can invoke with string URI", async () => {
const client = new PolywrapClient();
const result = await client.invoke<number>({
uri: wrapperUri.uri,
uri: wrapperUri,
method: "add",
args: {
a: 1,
Expand All @@ -55,7 +55,7 @@ describe("wasm-wrapper", () => {
test("invoke with decode defaulted to true works as expected", async () => {
const client = new PolywrapClient();
const result = await client.invoke<number>({
uri: wrapperUri.uri,
uri: wrapperUri,
method: "add",
args: {
a: 1,
Expand Down
2 changes: 1 addition & 1 deletion packages/client/src/types/InvokerOptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { IUriResolutionContext, Uri } from "@polywrap/core-js";

export interface InvokerOptions<TUri extends Uri | string = string> {
export interface InvokerOptions<TUri extends Uri | string = Uri> {
/** The Wrapper's URI */
uri: TUri;

Expand Down
2 changes: 1 addition & 1 deletion packages/client/src/types/TryResolveUriOptions.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { IUriResolutionContext, Uri } from "@polywrap/core-js";

export interface TryResolveUriOptions<TUri extends Uri | string = string> {
export interface TryResolveUriOptions<TUri extends Uri | string = Uri> {
/** The Wrapper's URI */
uri: TUri;
resolutionContext?: IUriResolutionContext;
Expand Down
Loading
Loading