Add [implements=<I>]L plainname for multiple imports#613
Add [implements=<I>]L plainname for multiple imports#613
Conversation
lukewagner
left a comment
lukewagner
left a comment
There was a problem hiding this comment.
Awesome, thanks for writing this up! The PR looks great; just minor nits. I think what'll be important for this change is seeing how it "plays out" in the producer and consumer tooling, so I think we'll want to keep this PR open until we get some experience from tool implementations, but so far so good from my perspective.
design/mvp/Binary.md
Outdated
| * Validation requires that `[implements=<I>]` annotated `plainname`s only | ||
| occur on `instance` imports or exports and that `I` is a valid | ||
| `interfacename`. |
There was a problem hiding this comment.
I think the grammatical rules (the EBNF in Explainer.md) force I to be a valid interfacename without validation having to say so here.
Requiring [implements=<I>]L to only annotate instance-typed imports/exports makes sense, but if we do it here, then I think we should also require it for plain interfacename-named imports/exports too. This restriction actually makes sense and shouldn't break anyone b/c it matches what you can do in WIT today (thereby removing an expressivity gap from #614), so I'm in favor.
design/mvp/Explainer.md
Outdated
| Here, both imports implement `wasi:keyvalue/store` but have distinct plain | ||
| names `primary` and `secondary`. Bindings generators can use the | ||
| `[implements=<I>]` annotation to know which interface the instance implements, | ||
| enabling them to generate the same typed bindings for both imports. |
There was a problem hiding this comment.
The resource types (e.g. bucket) defined directly in wasi:keyvalue/store will be treated as different, so "generate the same typed bindings for both imports" isn't quite right. (And this makes sense: if we want to allow two totally different implementations of these two keyvalue stores, each implementation should be able to have its own implementation of bucket.) However I suppose bindings generators could/should share types for value types (like the key-response record), since they're not tied to the implementation; they just need predictable names. So maybe "enabling them to share value type bindings"?
Second, it might also be good to give some examples of how the interfacename helps hosts/clients of a component.
design/mvp/Explainer.md
Outdated
| * An `[implements=<I>]L` name and a bare `I` `interfacename` are always | ||
| strongly-unique (they are different name kinds: `plainname` vs | ||
| `interfacename`). |
There was a problem hiding this comment.
I might totally be forgetting a case, but I think we don't need this bullet at all; the "Otherwise" clause gives us the behavior we want.
design/mvp/WIT.md
Outdated
| import primary: store; | ||
| import secondary: store; |
There was a problem hiding this comment.
Since you're using wasi:http/incoming-handler for the export, maybe use fully-qualified wasi:keyvalue/store for the imports (to avoid making a distinction that is beside the main point)?
design/mvp/WIT.md
Outdated
| instance type of the `store` interface. The plain name of the import is the | ||
| `id` before the colon (e.g., `primary`), not the interface name. This | ||
| contrasts with `import store;` (without the `id :` prefix), which would create | ||
| a single import using the full interface name `local:demo/store`. |
There was a problem hiding this comment.
Maybe also speak to the export which has the name my-handler and how this is different than export wasi:http/incoming-handler (and perhaps we could switch to wasi:http/handler in anticipation of p3 :).
Add parsing, validation, and uniqueness rules for the new `[implements=<interface>]label` extern name form from the component model `implements` proposal. An implements name labels an instance import/export that implements a named interface. Uniqueness: `[implements=<I>]L` conflicts with bare label `L` and with `[method]L.L` / `[static]L.L` (the existing l.l edge case), but is strongly unique from interface names, constructors, and normal method/static names. See implements feature: WebAssembly/component-model#613
Extend the WIT `extern-type` grammar to allow `use-path` as a third case, enabling `import id: use-path` and `export id: use-path` to create plain-named imports/exports whose instance type matches a named interface. This allows importing the same interface multiple times under different plain names (e.g., `import primary: store; import secondary: store;`), encoded using the `[implements=<interfacename>]label` annotation pattern. Fixes #287 Co-authored-by: Luke Wagner <mail@lukewagner.name>
2 participants
Add
[implements=<I>]L plainnamefor multiple imports of the same interfaceExtend the WIT
extern-typegrammar to allowuse-pathas a third case,enabling
import id: use-pathandexport id: use-pathto createplain-named imports/exports whose instance type matches a named interface.
This allows importing the same interface multiple times under different
plain names (e.g.,
import primary: store; import secondary: store;),encoded using the
[implements=<interfacename>]labelannotation pattern.Fixes #287