Improved string enum typing try 2

CHANGELOG.md

@@ -3,9 +3,14 @@
 
 ## Unreleased
 
+### Changed
+
+* String enums now generate private TypeScript types but only if used.
+  [#4174](https://github.com/rustwasm/wasm-bindgen/pull/4174)
+
 ### Fixed
 
-* Fixed methods with `self: &Self` consuming the object
+* Fixed methods with `self: &Self` consuming the object.
   [#4178](https://github.com/rustwasm/wasm-bindgen/pull/4178)
 
 --------------------------------------------------------------------------------

crates/backend/src/ast.rs

@@ -343,8 +343,12 @@ pub struct StringEnum {
     pub variants: Vec<Ident>,
     /// The JS string values of the variants
     pub variant_values: Vec<String>,
+    /// The doc comments on this enum, if any
+    pub comments: Vec<String>,
     /// Attributes to apply to the Rust enum
     pub rust_attrs: Vec<syn::Attribute>,
+    /// Whether to generate a typescript definition for this enum
+    pub generate_typescript: bool,
     /// Path to wasm_bindgen
     pub wasm_bindgen: Path,
 }

crates/backend/src/encode.rs

@@ -362,7 +362,9 @@ fn shared_import_type<'a>(i: &'a ast::ImportType, intern: &'a Interner) -> Impor
 fn shared_import_enum<'a>(i: &'a ast::StringEnum, _intern: &'a Interner) -> StringEnum<'a> {
     StringEnum {
         name: &i.js_name,
+        generate_typescript: i.generate_typescript,
         variant_values: i.variant_values.iter().map(|x| &**x).collect(),
+        comments: i.comments.iter().map(|s| &**s).collect(),
     }
 }
 

crates/cli-support/src/js/binding.rs

@@ -8,6 +8,7 @@ use crate::js::Context;
 use crate::wit::InstructionData;
 use crate::wit::{Adapter, AdapterId, AdapterKind, AdapterType, Instruction};
 use anyhow::{anyhow, bail, Error};
+use std::collections::HashSet;
 use std::fmt::Write;
 use walrus::{Module, ValType};
 
@@ -68,6 +69,7 @@ pub struct JsFunction {
     pub js_doc: String,
     pub ts_arg_tys: Vec<String>,
     pub ts_ret_ty: Option<String>,
+    pub ts_refs: HashSet<TsReference>,
     /// Whether this function has a single optional argument.
     ///
     /// If the function is a setter, that means that the field it sets is optional.
@@ -76,6 +78,14 @@ pub struct JsFunction {
     pub log_error: bool,
 }
 
+/// A references to an (likely) exported symbol used in TS type expression.
+///
+/// Right now, only string enum require this type of anaylsis.
+#[derive(Debug, Clone, PartialEq, Eq, Hash)]
+pub enum TsReference {
+    StringEnum(String),
+}
+
 impl<'a, 'b> Builder<'a, 'b> {
     pub fn new(cx: &'a mut Context<'b>) -> Builder<'a, 'b> {
         Builder {
@@ -246,7 +256,7 @@ impl<'a, 'b> Builder<'a, 'b> {
         // should start from here. Struct fields(Getter) only have one arg, and
         // this is the clue we can infer if a function might be a field.
         let mut might_be_optional_field = false;
-        let (ts_sig, ts_arg_tys, ts_ret_ty) = self.typescript_signature(
+        let (ts_sig, ts_arg_tys, ts_ret_ty, ts_refs) = self.typescript_signature(
             &function_args,
             &arg_tys,
             &adapter.inner_results,
@@ -266,6 +276,7 @@ impl<'a, 'b> Builder<'a, 'b> {
             js_doc,
             ts_arg_tys,
             ts_ret_ty,
+            ts_refs,
             might_be_optional_field,
             catch: self.catch,
             log_error: self.log_error,
@@ -285,11 +296,12 @@ impl<'a, 'b> Builder<'a, 'b> {
         might_be_optional_field: &mut bool,
         asyncness: bool,
         variadic: bool,
-    ) -> (String, Vec<String>, Option<String>) {
+    ) -> (String, Vec<String>, Option<String>, HashSet<TsReference>) {
         // Build up the typescript signature as well
         let mut omittable = true;
         let mut ts_args = Vec::new();
         let mut ts_arg_tys = Vec::new();
+        let mut ts_refs = HashSet::new();
         for (name, ty) in arg_names.iter().zip(arg_tys).rev() {
             // In TypeScript, we can mark optional parameters as omittable
             // using the `?` suffix, but only if they're not followed by
@@ -301,12 +313,12 @@ impl<'a, 'b> Builder<'a, 'b> {
             match ty {
                 AdapterType::Option(ty) if omittable => {
                     arg.push_str("?: ");
-                    adapter2ts(ty, &mut ts);
+                    adapter2ts(ty, &mut ts, Some(&mut ts_refs));
                 }
                 ty => {
                     omittable = false;
                     arg.push_str(": ");
-                    adapter2ts(ty, &mut ts);
+                    adapter2ts(ty, &mut ts, Some(&mut ts_refs));
                 }
             }
             arg.push_str(&ts);
@@ -342,7 +354,7 @@ impl<'a, 'b> Builder<'a, 'b> {
             let mut ret = String::new();
             match result_tys.len() {
                 0 => ret.push_str("void"),
-                1 => adapter2ts(&result_tys[0], &mut ret),
+                1 => adapter2ts(&result_tys[0], &mut ret, Some(&mut ts_refs)),
                 _ => ret.push_str("[any]"),
             }
             if asyncness {
@@ -351,7 +363,7 @@ impl<'a, 'b> Builder<'a, 'b> {
             ts.push_str(&ret);
             ts_ret = Some(ret);
         }
-        (ts, ts_arg_tys, ts_ret)
+        (ts, ts_arg_tys, ts_ret, ts_refs)
     }
 
     /// Returns a helpful JS doc comment which lists types for all parameters
@@ -374,7 +386,7 @@ impl<'a, 'b> Builder<'a, 'b> {
         for (name, ty) in fn_arg_names.iter().zip(arg_tys).rev() {
             let mut arg = "@param {".to_string();
 
-            adapter2ts(ty, &mut arg);
+            adapter2ts(ty, &mut arg, None);
             arg.push_str("} ");
             match ty {
                 AdapterType::Option(..) if omittable => {
@@ -395,7 +407,7 @@ impl<'a, 'b> Builder<'a, 'b> {
 
         if let (Some(name), Some(ty)) = (variadic_arg, arg_tys.last()) {
             ret.push_str("@param {...");
-            adapter2ts(ty, &mut ret);
+            adapter2ts(ty, &mut ret, None);
             ret.push_str("} ");
             ret.push_str(name);
             ret.push('\n');
@@ -1416,7 +1428,7 @@ impl Invocation {
     }
 }
 
-fn adapter2ts(ty: &AdapterType, dst: &mut String) {
+fn adapter2ts(ty: &AdapterType, dst: &mut String, refs: Option<&mut HashSet<TsReference>>) {
     match ty {
         AdapterType::I32
         | AdapterType::S8
@@ -1434,13 +1446,19 @@ fn adapter2ts(ty: &AdapterType, dst: &mut String) {
         AdapterType::Bool => dst.push_str("boolean"),
         AdapterType::Vector(kind) => dst.push_str(&kind.js_ty()),
         AdapterType::Option(ty) => {
-            adapter2ts(ty, dst);
+            adapter2ts(ty, dst, refs);
             dst.push_str(" | undefined");
         }
         AdapterType::NamedExternref(name) => dst.push_str(name),
         AdapterType::Struct(name) => dst.push_str(name),
         AdapterType::Enum(name) => dst.push_str(name),
-        AdapterType::StringEnum => dst.push_str("any"),
+        AdapterType::StringEnum(name) => {
+            if let Some(refs) = refs {
+                refs.insert(TsReference::StringEnum(name.clone()));
+            }
+
+            dst.push_str(name);
+        }
         AdapterType::Function => dst.push_str("any"),
     }
 }

crates/cli-support/src/js/mod.rs

@@ -9,6 +9,7 @@ use crate::wit::{AuxEnum, AuxExport, AuxExportKind, AuxImport, AuxStruct};
 use crate::wit::{JsImport, JsImportName, NonstandardWitSection, WasmBindgenAux};
 use crate::{reset_indentation, Bindgen, EncodeInto, OutputMode, PLACEHOLDER_MODULE};
 use anyhow::{anyhow, bail, Context as _, Error};
+use binding::TsReference;
 use std::borrow::Cow;
 use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
 use std::fmt;
@@ -47,6 +48,10 @@ pub struct Context<'a> {
     /// identifiers.
     defined_identifiers: HashMap<String, usize>,
 
+    /// A set of all (tracked) symbols referenced from within type definitions,
+    /// function signatures, etc.
+    typescript_refs: HashSet<TsReference>,
+
     exported_classes: Option<BTreeMap<String, ExportedClass>>,
 
     /// A map of the name of npm dependencies we've loaded so far to the path
@@ -108,6 +113,7 @@ impl<'a> Context<'a> {
             js_imports: Default::default(),
             defined_identifiers: Default::default(),
             wasm_import_definitions: Default::default(),
+            typescript_refs: Default::default(),
             exported_classes: Some(Default::default()),
             config,
             threads_enabled: config.threads.is_enabled(module),
@@ -2662,6 +2668,7 @@ __wbg_set_wasm(wasm);"
             ts_sig,
             ts_arg_tys,
             ts_ret_ty,
+            ts_refs,
             js_doc,
             code,
             might_be_optional_field,
@@ -2688,6 +2695,8 @@ __wbg_set_wasm(wasm);"
                 Kind::Adapter => "failed to generates bindings for adapter".to_string(),
             })?;
 
+        self.typescript_refs.extend(ts_refs);
+
         // Once we've got all the JS then put it in the right location depending
         // on what's being exported.
         match kind {
@@ -3822,6 +3831,27 @@ __wbg_set_wasm(wasm);"
             .map(|v| format!("\"{v}\""))
             .collect();
 
+        if string_enum.generate_typescript
+            && self
+                .typescript_refs
+                .contains(&TsReference::StringEnum(string_enum.name.clone()))
+        {
+            let docs = format_doc_comments(&string_enum.comments, None);
+
+            self.typescript.push_str(&docs);
+            self.typescript.push_str("type ");
+            self.typescript.push_str(&string_enum.name);
+            self.typescript.push_str(" = ");
+
+            if variants.is_empty() {
+                self.typescript.push_str("never");
+            } else {
+                self.typescript.push_str(&variants.join(" | "));
+            }
+
+            self.typescript.push_str(";\n");
+        }
+
         self.global(&format!(
             "const __wbindgen_enum_{name} = [{values}];\n",
             name = string_enum.name,

crates/cli-support/src/wit/incoming.rs

@@ -110,7 +110,7 @@ impl InstructionBuilder<'_, '_> {
             },
             Descriptor::StringEnum { name, invalid, .. } => {
                 self.instruction(
-                    &[AdapterType::StringEnum],
+                    &[AdapterType::StringEnum(name.clone())],
                     Instruction::StringEnumToWasm {
                         name: name.clone(),
                         invalid: *invalid,
@@ -312,7 +312,7 @@ impl InstructionBuilder<'_, '_> {
                 hole,
             } => {
                 self.instruction(
-                    &[AdapterType::StringEnum.option()],
+                    &[AdapterType::StringEnum(name.clone()).option()],
                     Instruction::OptionStringEnumToWasm {
                         name: name.clone(),
                         invalid: *invalid,

crates/cli-support/src/wit/mod.rs

@@ -868,11 +868,13 @@ impl<'a> Context<'a> {
     fn string_enum(&mut self, string_enum: &decode::StringEnum<'_>) -> Result<(), Error> {
         let aux = AuxStringEnum {
             name: string_enum.name.to_string(),
+            comments: concatenate_comments(&string_enum.comments),
             variant_values: string_enum
                 .variant_values
                 .iter()
                 .map(|v| v.to_string())
                 .collect(),
+            generate_typescript: string_enum.generate_typescript,
         };
         let mut result = Ok(());
         self.aux

crates/cli-support/src/wit/nonstandard.rs

@@ -179,8 +179,12 @@ pub struct AuxEnum {
 pub struct AuxStringEnum {
     /// The name of this enum
     pub name: String,
+    /// The copied Rust comments to forward to JS
+    pub comments: String,
     /// A list of variants values
     pub variant_values: Vec<String>,
+    /// Whether typescript bindings should be generated for this enum.
+    pub generate_typescript: bool,
 }
 
 #[derive(Debug)]

crates/cli-support/src/wit/outgoing.rs

@@ -295,7 +295,7 @@ impl InstructionBuilder<'_, '_> {
                         name: name.clone(),
                         hole: *hole,
                     },
-                    &[AdapterType::StringEnum.option()],
+                    &[AdapterType::StringEnum(name.clone()).option()],
                 );
             }
             Descriptor::RustStruct(name) => {
@@ -538,7 +538,7 @@ impl InstructionBuilder<'_, '_> {
             Instruction::WasmToStringEnum {
                 name: name.to_string(),
             },
-            &[AdapterType::StringEnum],
+            &[AdapterType::StringEnum(name.to_string())],
         );
     }
 

crates/cli-support/src/wit/standard.rs

@@ -86,7 +86,7 @@ pub enum AdapterType {
     Option(Box<AdapterType>),
     Struct(String),
     Enum(String),
-    StringEnum,
+    StringEnum(String),
     NamedExternref(String),
     Function,
     NonNull,

crates/cli/tests/reference/enums.d.ts

@@ -12,14 +12,14 @@ export function enum_echo(color: Color): Color;
 export function option_enum_echo(color?: Color): Color | undefined;
 /**
  * @param {Color} color
- * @returns {any}
+ * @returns {ColorName}
  */
-export function get_name(color: Color): any;
+export function get_name(color: Color): ColorName;
 /**
- * @param {any | undefined} [color]
- * @returns {any | undefined}
+ * @param {ColorName | undefined} [color]
+ * @returns {ColorName | undefined}
  */
-export function option_string_enum_echo(color?: any): any | undefined;
+export function option_string_enum_echo(color?: ColorName): ColorName | undefined;
 /**
  * A color.
  */
@@ -43,3 +43,7 @@ export enum ImplicitDiscriminant {
   C = 42,
   D = 43,
 }
+/**
+ * The name of a color.
+ */
+type ColorName = "green" | "yellow" | "red";

crates/cli/tests/reference/enums.js

@@ -46,16 +46,16 @@ export function option_enum_echo(color) {
 
 /**
  * @param {Color} color
- * @returns {any}
+ * @returns {ColorName}
  */
 export function get_name(color) {
     const ret = wasm.get_name(color);
     return __wbindgen_enum_ColorName[ret];
 }
 
 /**
- * @param {any | undefined} [color]
- * @returns {any | undefined}
+ * @param {ColorName | undefined} [color]
+ * @returns {ColorName | undefined}
  */
 export function option_string_enum_echo(color) {
     const ret = wasm.option_string_enum_echo(isLikeNone(color) ? 4 : ((__wbindgen_enum_ColorName.indexOf(color) + 1 || 4) - 1));
@@ -83,6 +83,10 @@ export const ImplicitDiscriminant = Object.freeze({ A:0,"0":"A",B:1,"1":"B",C:42
 
 const __wbindgen_enum_ColorName = ["green", "yellow", "red"];
 
+const __wbindgen_enum_FooBar = ["foo", "bar"];
+
+const __wbindgen_enum_PrivateStringEnum = ["foo", "bar"];
+
 export function __wbindgen_throw(arg0, arg1) {
     throw new Error(getStringFromWasm0(arg0, arg1));
 };