add support generating TS bindings #16
Conversation
Signed-off-by: karthik2804 <[email protected]>
| wit-bindgen-core = "0.34.0" | ||
| wasm-pkg-common = "0.5.1" | ||
| wasm-pkg-client = "0.5.1" | ||
| js-component-bindgen-component = { git = "https://github.com/bytecodealliance/jco" } |
There was a problem hiding this comment.
These two Git references should be pinned - otherwise you're at the mercy of main randomly changing out from underneath you.
| wit-bindgen-core = "0.34.0" | ||
| wasm-pkg-common = "0.5.1" | ||
| wasm-pkg-client = "0.5.1" | ||
| js-component-bindgen-component = { git = "https://github.com/bytecodealliance/jco" } |
There was a problem hiding this comment.
Sanity check if this is still needed? I can see where -bindgen is used but not -bindgen-component
| } | ||
| BindingsLanguage::Ts => { | ||
| todo!("generate ts") | ||
| let imported_interfaces = get_imported_interfaces(&resolve, world_id); |
There was a problem hiding this comment.
This would seem a good opportunity to pull both this and the Rust arm out into helper functions, so that readers can follow the top-to-bottom flow of run without needing to track where they are entering and existing lengthy case blocks.
| for (name, contents) in files.iter() { | ||
| let output_path = self.output.join(name); | ||
| if !output_path.to_str().unwrap().contains("/interfaces/") { | ||
| // // Skip non-interface files |
|
|
||
| for (name, contents) in files.iter() { | ||
| let output_path = self.output.join(name); | ||
| if !output_path.to_str().unwrap().contains("/interfaces/") { |
There was a problem hiding this comment.
This seems like it include all files if self.output contains /interfaces/. Is that intentional? Or would it be better to filter on name before constructing output_path?
Are there any package / world / interface names that could make a naive contains check go awry?
| "Bindings generated for TypeScript in {0}.", | ||
| self.output.to_str().expect("Failed to parse output path") | ||
| ); | ||
| println!("\nMake sure to add the following interfaces to webpack as externals"); |
There was a problem hiding this comment.
How do I do this? Could the tool print the actual text I need to add and where to add it?
|
|
||
| println!("\nupdate `knitwit.json` for \"{}\" to include dependency components. You would need to add the following fields:", self.component_id); | ||
| println!(" - `project.worlds` array needs to contain `deps` as one of the worlds"); | ||
| println!(" - `project.witPaths` array needs to contain the relative path to `<root of spin app>/.wit/components/{}`:", self.component_id); |
There was a problem hiding this comment.
Do we not know the root of the app?
| println!(" - {0}/{1}", pkg_name, interface); | ||
| } | ||
|
|
||
| println!("\nupdate `knitwit.json` for \"{}\" to include dependency components. You would need to add the following fields:", self.component_id); |
There was a problem hiding this comment.
Should update be capitalised here? Maybe this is intended to be another bullet in the "Make sure to..." list (which would be fine) but that's not how things seem to be presented at the moment...?
| .filter_map(|(_k, v)| match v { | ||
| wit_parser::WorldItem::Interface { id, .. } => { | ||
| let i = &resolve.interfaces[*id]; | ||
| let pkg_id = i.package.unwrap(); |
There was a problem hiding this comment.
This is infallible because...? (I assume .package is fallible for a reason, just want to check that we know it will never fail here.)
| let i = &resolve.interfaces[*id]; | ||
| let pkg_id = i.package.unwrap(); | ||
| let pkg = &resolve.packages[pkg_id]; | ||
| Some((pkg.name.clone(), i.name.clone().unwrap_or_default())) |
There was a problem hiding this comment.
An empty interface name will cause surprising output in the "add this to webpack" output.
|
closing in favour of newer approach |
This PR uses
jconow to generate bindings for TS and add instructions on how to setup the toolchain to consume the components in TS/JS