Feature/using component #678
Conversation
There was a problem hiding this comment.
hey chris, what an awesome work again! love the idea and really enjoy the thoughts it sparks (the debate on macros vs. code generation).
together with the upcoming type support in elixir this could work really really well for wasm components. most of my comments are not blocking, all can be discussed :)
|
|
||
| functions = | ||
| if wit_path = Keyword.get(opts, :wit) do | ||
| wit_contents = File.read!(wit_path) |
There was a problem hiding this comment.
thinking aloud: should we maybe not hand over a file path but rather the wit definition (file content) itself?
maybe the wit content doesn't come from a file but from - i don't know - a network request or was generated somehow.
There was a problem hiding this comment.
Hmm.. as it turns out rust code annoyingly needs a path passed to it, but it doesn't have to actually be a path that exists. I think it uses it to produce errors perhaps. In any event, if we change it to pass the wit contents as a string we have to make up a path somehow. Any thoughts?
There was a problem hiding this comment.
I'm thinking I will just have the wit option just mean a string containing the wit content, and add a wit_path or wit_file option to specify the file(s). In the future I'd also like to add component or component_bytes options to pass in the binary, which I think I can also parse with a little more code
| wit_contents = File.read!(wit_path) | ||
| exported_functions = Wasmex.Native.wit_exported_functions(wit_path, wit_contents) | ||
|
|
||
| for {function, arity} <- exported_functions do |
There was a problem hiding this comment.
can't wait for imported function support! :D
There was a problem hiding this comment.
yes, definitely high on my list of next things.
| let exported_functions = exports | ||
| .iter() | ||
| .filter_map(|(_key, value)| match value { | ||
| WorldItem::Function(function) => Some((&function.name, function.params.len())), |
There was a problem hiding this comment.
would be nifty if we could also return the types (params types and return types) for nicer introspection/debugging/tooling later
There was a problem hiding this comment.
also to prepare for elixir types (whenever they become available) - would be ace if wasmex could created typed interfaces/function definitions right away
There was a problem hiding this comment.
Yep, i did this way for expediency in getting something working we need a better API to get back data from wit parsing.
| use wit_parser::{Resolve, WorldItem}; | ||
|
|
||
| #[rustler::nif(name = "wit_exported_functions")] | ||
| pub fn exported_functions(env: rustler::Env, path: String, wit: String) -> NifResult<Term> { |
There was a problem hiding this comment.
I know you care only about functions for this PR, but is it an option to return all exports of the module instead?
similar to module_exports for regular wasm modules.
We would share a common/similar API (which I like for consistency) and having that additional info will surely help us later and we don't have to break any API for those changes.
There was a problem hiding this comment.
I'll take a look at module exports and see if we can consolidate, agree having a consistent api would be best.
| defmodule HelloWorld do | ||
| @moduledoc false | ||
|
|
||
| use Wasmex.Components.Component, wit: "test/component_fixtures/hello_world/hello-world.wit" | ||
| end |
There was a problem hiding this comment.
love it! would be great to see imported functions being support soon 🙌
oh and is it possible to move this module just above the test where we use it? (like an inlined module). this would make the test way easier to read IMO (since we don't have to jump files)
| defmodule HelloWorld do | ||
| @moduledoc false | ||
|
|
||
| use Wasmex.Components.Component, wit: "test/component_fixtures/hello_world/hello-world.wit" |
There was a problem hiding this comment.
I know we touched this topic in a private conversation before, but let's bring this discussion here :D
I love the idea! Calling component modules must be easy and automated and what you did here is much much better than anything we already have in wasmex. I'm not 100% certain how to do it though and would love your thoughts!
I was (and still am) internally debating whether we should use macros (as you did here) or rather have code generation. Let's try to collect a few pros/cons and see what's better. (please add your thoughts - I'm not decided, and we could even end up doing both)
Macro
- set-up: super smooth. just give it some
witand you're good - transparency: not great. you have to look at the wit definition (which is not elixir code) to see what this module even implements
- level of magic: high. it's not clear what exactly happens. will it only return functions, will it do imported function, will it do other fancy wasm things to my module? what if the wit file is some complicated mess?! how will my module look like? what if I only want to expose a few selected exported functions and not all?
- upgrade path: when we in wasmex want to add functionality (e.g. add imported functions) this would always be a breaking change to our users
- debug'ability: not sure. What if we implement more functions in this module that in turn call into the generated ones. It get hard to reason about this module rather fast, no?
- change tracking: if I am a user of wasmex and have such a wit module. Then changing my wit definition would result in this module changing too 🤔 Imagine being the reviewer of this PR in our users code. I could probably see what the wit-change is and could reason that this module changes too?! But it's very implicit and might lead to me missing a bug - e.g. when some other code calls into this generated module.
Code Generation
- not as easy to set-up: probably requires some mix task to generate this modules skeleton (maybe use igniter?!)
- any change in the wit definition results in two changes in the code base (the wit itself, and the generated module)
- what if the wit definition changes, but the module was forgotten to be re-generated? this might introduce an inconsistency -- or maybe is a feature to develop the module independent from the initial wit 🤔
- less magic, and by that also easier code reviews+debugging
- maybe tooling has it easier too? Like what if the LSP wants to find the function implementation. with generated code it's easy to see where the fn is implemented --- with a macro it's not as much
- it's more stuff to read and might look "scary". The macro hides away some "ugly" boilerplate whereas code generation create a larger diff that might be ugly to read
- code generation is what elixir/José seems to prefer (e.g.
mix phx.gen.auth, andigniterwhereas macros are were heavily used in ruby codebases and José consciously decided against their magic when designing Elixir in many places)
Conclusion
So. I don't know. I guess it's a trade off as all things in life. Coming from a corporate thinking (my day job ruined me) code generation looks sweet. it's more explicit and less magic. code reviews and manual extensibility seems to be favoured and it looks to vibe with what (I think) Elixir intends to be. But macros are so much easier to set-up.
What are your thoughts?
There was a problem hiding this comment.
If it was a macro, would we still have compiled-time checking of the name of functions and number of arguments passed? I think that’s the main thing I would want, apart from type-checking.
If I change my WIT file, and the Elixir compiler alerts me via the macros changing then that’s a good UX imo.
There was a problem hiding this comment.
I'm not sure I know or entirely understand your questions @RoyalIcing but in my experiments, things like completion in iex still work so there appear to be "normal" behaviour for the macro generated functions, since macros are expanded at compile time. I'm not sure this is a one size fits all answer here, so I kind of lean towards eventually supporting both options. That said, I think I'll continue on with the macro approach for the moment as I flesh things out as its an approach I'm pretty familiar with vs learning Igniter (which does sound fun but only so much time in day etc).
There was a problem hiding this comment.
@superchris My thinking is round what happens when the WIT changes. If the WIT is consumed by the macros then that would lead to a compile time error for most changes, which is what you’d want for a good developer experience.
If you went with the code generation I’d want some compile-time check that verifies the WIT and generated code still agree anyway. So for me it would really come down to whether being able to read the interface as Elixir code would be helpful or not. If you had code generation would you ever want to alter that code? I’m thinking probably not?
There was a problem hiding this comment.
huh! what a discussion here and on bluesky! considering the macro is a very thin wrapper right now, I'd say let's merge it!
we maybe want to expand this feature into more than that which might end up being code generation (e.g. to support an elixir module having only a sub-set of the exported functions implemented) but not now.
No description provided.