polywrap / wrap-cli

Used to create, build, and integrate wraps.
https://polywrap.io
MIT License
170 stars 52 forks source link

Re-export imported types #1448

Open krisbitney opened 1 year ago

krisbitney commented 1 year ago

Is your feature request related to a problem? Please describe. Let's say I have wrapper that is supported by a plugin. The plugin implements an interface. The interface declares a type Connection. I want to use the Connection type in the wrapper as Interface_Connection and accept it as a method argument.

I can use Connection in the wrapper and accept it as a method argument without friction for app developers, but wrapper developers who want to integrate my wrapper will have problems. They cannot import the interface type Ethereum_Interface_Connection, nor can they import the interface type and cast it.

Alternatively, I can re-declare a type Connection in my wrapper. When I want to interact with the plugin, I create a new object Interface_Connection and copy the properties of Connection to it. This preserves the type's ergonomics for other developers.

    pub fn new(connection: &Option<Connection>) -> Self {
        let iprovider_connection = connection.as_ref().map(|conn| IProviderConnection {
            network_name_or_chain_id: conn.network_name_or_chain_id.clone(),
            node: conn.node.clone(),
        });
        Self {
            connection: iprovider_connection,
            iprovider: get_iprovider(),
        }
    }

Describe the solution you'd like While rebuilding a struct solves the problem, it is an inconvenience that seems unnecessary. In an extreme case, it could make the wasm module meaningfully larger.

The only meaningful difference between the types is the static URI property on the imported type.

I think imported types should be re-exported if they are used in the schema (this could possibly be limited to imported interfaces).

Describe alternatives you've considered

Additional context Add any other context or screenshots about the feature request here.

dOrgJelli commented 1 year ago

I think we can use theory to guide our implementation here, because I agree the current implementation is flawed, and we can use theory to prove it is flawed.

When a wrapper "implements an interface", I propose it should be a superset of said interface. This means that it should contain all elements of the interface within itself, and these elements should not differ from that of the interface. If you look at what we have today, the elements in the implementation do differ from that of the interface, because they are namespaced. For example: ens/interface.eth

type Module {
  interfaceMethod(arg: Object!): Bytes!
}

type Object {
  prop: String!
}

ens/impl.eth

#import { Module } into Interface from "ens/interface.eth"

type Module implements Interface_Module {
  anotherMethod(arg: String!): String!
}

When this schema is built, the result is as follows:

type Module implements Interface_Module {
  interfaceMethod(arg: Interface_Object!): Bytes!
  anotherMethod(arg: String!): String!
}

type Interface_Module @import(
  uri: "ens/interface.eth"
) {
  interfaceMethod(arg: Interface_Object!): Bytes!
}

type Interface_Object @import(
  uri: "ens/interface.eth"
) {
  prop: String!
}

As you can see above, the types within ens/interface.eth are not 1:1 to the types in ens/impl.eth. This is a problem and should be fixed (as you've stated above).

Proposed Solution

We should fundamentally differentiate between "importing a dependency" and "implementing an interface". This can be done by no longer using the #import { ... } into ... from ... syntax when you are implementing an interface. Instead we can simply do something similar to the following: ens/impl.eth

#implement * from "ens/interface.eth"

type Module {
  anotherMethod(arg: String!): String!
}

After the schema is built, the result would be:

type Module @implements(
  uri: "ens/interface.eth",
  type: "Module"
) {
  interfaceMethod(arg: Object!): Bytes!
  anotherMethod(arg: String!): String!
}

type Object @implements(
  uri: "ens/interface.eth",
  type: "Object"
) {
  prop: String!
}

Thoughts?

dOrgJelli commented 1 year ago

Related: https://github.com/polywrap/toolchain/issues/1376

krisbitney commented 1 year ago

This is a really fantastic idea! I think it would provide a better experience. I'm 100% for it.

This could also make it easy to validate that a wrapper implements an interface.

Niraj-Kamdar commented 1 year ago

@dOrgJelli This doesn't solve the problem with namespacing everything and creating long nested namespaced types. having the ability to import to the local namespace and re-export only needed types would cover more use-cases then the one for just the interface-implementations

dOrgJelli commented 1 year ago

@Niraj-Kamdar in order to better understand what you're describing, I think it'd be helpful if you:

dOrgJelli commented 1 year ago

Hey @krisbitney @Niraj-Kamdar, I talked with @namesty about this more today in the context of WRAP ABI 0.2. There's a lot to discuss, but in general I want to propose another solution to Kris' described problem above.

NOTE: Please disregard my prior proposal above depicting the #implement ... from "..." syntax.

So just as Kris and Niraj have both stated, the problem is that we need a better way of re-exporting types. Currently today, if you want to re-export a type, you must do the following (based on Kris' scenario above):

#import { Connection } into Interface from "ethereum-interface"

type Connection implements Interface_Connection {
}

which would result in a final ABI looking like this:

{
  objects: [{
    name: "Connection",
    properties: [...],
    ...
  }],
  importedObjects: [{
    uri: "ethereum-interface",
    namespace: "Interface",
    name: "Interface_Connection",
    properties: [...],
    ...
  }]
}

This however could be streamlined, and instead be something more like this:

#include { Connection } from "ethereum-interface"

which would result in a final ABI looking like this:

{
  objects: [{
    name: "Connection",
    properties: [...],
    ...
  }],
}

Pro: smaller ABI Con: lose URI information

NOTE: if we introduce the #include keyword, we'd also want to refactor the "local schema import" functionality to use the #include syntax as well. For example #include { CommonType } from "../common/schema.graphql"

Thoughts?

krisbitney commented 1 year ago

@dOrgJelli

I think it's good! I don't think losing the URI information really matters, but I also don't really remember how the URI information is used.

Niraj-Kamdar commented 1 year ago

@dOrgJelli How about?

#import { Connection } into Interface from "ethereum-interface"
#using namespace Interface::* <-- import whole Interface namespace to local namespace
#using namespace Interface::Connection <-- import Connection to local namespace
#using namespace Interface::{Connection, Module} 

or something rusty:

#import { Connection } into Interface from "ethereum-interface"
#use Interface::* <-- import whole Interface namespace to local namespace
#use Interface::Connection <-- import Connection to local namespace
#use Interface::{Connection, Module} 

We can also have concept of private/internal types with explicit re-export syntax PRO: Unexported type wouldn't end up in ABI, way to specify internal types in schema CON: lose some type info but doesn't really matter because only being used internally in the wasm module.

I haven't looked much into the ABI 0.2 but this may be incorporated inside ABI 0.2

dOrgJelli commented 1 year ago

Hey @Niraj-Kamdar, I agree that we should take an "ABI import minimization" approach, so that we are not exporting needless types in the ABI. If types are only used within the wasm code, and nowhere else, then we shouldn't be including them in the ABI.

If we break this down, I think we have 3 user-scenarios we need to support:

  1. Import type Foo, use Foo in the wrapper's implementation language via schema bindings.
  2. Import type Foo, use Foo within another schema type Bar (as a property or something like that).
  3. Import type Foo, re-export Foo from the schema as if it were defined inline.

Here is how I propose we solve these scenarios: 1

#import { Foo } into Namespace from "uri"
abi: { }

2

#import { Foo } into Namespace from "uri"

type Bar {
  prop: Foo!
}
abi = {
  imports: [ { ...foo... } ],
  objects: [ { ...bar with foo prop... } ]
}

3

#import { Foo } into Namespace from "uri"

type Foo implements Namespace_Foo {
  ...must re-add all of Foo's props...
}
abi = {
  objects: [ { ...foo... } ]
}

NOTE: if you #import { Module } ... (or other syntax for importing functions) it will always be included in the imports section of the ABI.