Merge branch 'main' into issue4317

rustwasm · Dec 8, 2024 · 6de403d · 6de403d
2 parents 9698538 + 54f97c9
commit 6de403d
Show file tree

Hide file tree

Showing 23 changed files with 308 additions and 154 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -29,7 +29,7 @@ jobs:
     - run: rustup update --no-self-update stable && rustup default stable
     - run: rustup component add rustfmt
     - run: cargo fmt --all -- --check
-  
+
   # Check TOML style by using Taplo.
   taplo:
     name: Taplo
@@ -240,6 +240,22 @@ jobs:
   #       WBINDGEN_I_PROMISE_JS_SYNTAX_WORKS_IN_NODE: 1
   #   - run: cargo build --manifest-path crates/web-sys/Cargo.toml --target wasm32-unknown-unknown --features "Node Window Document"
 
+  # This checks that the output of the CLI is actually valid JavaScript and TypeScript
+  test_cli_reference_typescript:
+    name: Run CLI reference TypeScript check
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v4
+    - uses: actions/setup-node@v4
+      with:
+        node-version: 'lts/*'
+    - run: npm i -g typescript
+    - run: npm i --save @types/node @types/deno
+    - name: Check TypeScript output
+      run: tsc --noEmit --skipLibCheck --lib esnext,dom $(echo crates/cli/tests/reference/*.d.ts)
+    - name: Check JavaScript output
+      run: tsc --noEmit --skipLibCheck --lib esnext,dom --module esnext --allowJs $(echo crates/cli/tests/reference/*.js)
+
   test_native:
     name: Run native tests
     runs-on: ubuntu-latest

diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -3,9 +3,14 @@
 
 ## Unreleased
 
+### Changed
+
+* Optional parameters are now typed as `T | undefined | null` to reflect the actual JS behavior.
+  [#4188](https://github.com/rustwasm/wasm-bindgen/pull/4188)
+
 ### Fixed
 
-- Fixed JS keyword-like identifiers not being handled correctly for imports and exports.
+- Fixed JS keyword-like identifiers not being handled correctly. Keywords on imports will no longer be escaped. Using keyword identifiers in places that caused invalid JS code gen will now error at compile time.
   [#4329](https://github.com/rustwasm/wasm-bindgen/pull/4329)
 
 --------------------------------------------------------------------------------

diff --git a/crates/cli-support/src/js/binding.rs b/crates/cli-support/src/js/binding.rs
@@ -321,13 +321,15 @@ impl<'a, 'b> Builder<'a, 'b> {
             let mut ts = String::new();
             match ty {
                 AdapterType::Option(ty) if omittable => {
+                    // e.g. `foo?: string | null`
                     arg.push_str("?: ");
-                    adapter2ts(ty, &mut ts, Some(&mut ts_refs));
+                    adapter2ts(ty, TypePosition::Argument, &mut ts, Some(&mut ts_refs));
+                    ts.push_str(" | null");
                 }
                 ty => {
                     omittable = false;
                     arg.push_str(": ");
-                    adapter2ts(ty, &mut ts, Some(&mut ts_refs));
+                    adapter2ts(ty, TypePosition::Argument, &mut ts, Some(&mut ts_refs));
                 }
             }
             arg.push_str(&ts);
@@ -363,7 +365,12 @@ 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, Some(&mut ts_refs)),
+                1 => adapter2ts(
+                    &result_tys[0],
+                    TypePosition::Return,
+                    &mut ret,
+                    Some(&mut ts_refs),
+                ),
                 _ => ret.push_str("[any]"),
             }
             if asyncness {
@@ -395,16 +402,18 @@ 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, None);
-            arg.push_str("} ");
             match ty {
-                AdapterType::Option(..) if omittable => {
+                AdapterType::Option(ty) if omittable => {
+                    adapter2ts(ty, TypePosition::Argument, &mut arg, None);
+                    arg.push_str(" | null} ");
                     arg.push('[');
                     arg.push_str(name);
                     arg.push(']');
                 }
                 _ => {
                     omittable = false;
+                    adapter2ts(ty, TypePosition::Argument, &mut arg, None);
+                    arg.push_str("} ");
                     arg.push_str(name);
                 }
             }
@@ -416,7 +425,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, None);
+            adapter2ts(ty, TypePosition::Argument, &mut ret, None);
             ret.push_str("} ");
             ret.push_str(name);
             ret.push('\n');
@@ -1542,7 +1551,18 @@ impl Invocation {
     }
 }
 
-fn adapter2ts(ty: &AdapterType, dst: &mut String, refs: Option<&mut HashSet<TsReference>>) {
+#[derive(Debug, Clone, Copy)]
+enum TypePosition {
+    Argument,
+    Return,
+}
+
+fn adapter2ts(
+    ty: &AdapterType,
+    position: TypePosition,
+    dst: &mut String,
+    refs: Option<&mut HashSet<TsReference>>,
+) {
     match ty {
         AdapterType::I32
         | AdapterType::S8
@@ -1564,8 +1584,11 @@ fn adapter2ts(ty: &AdapterType, dst: &mut String, refs: Option<&mut HashSet<TsRe
         AdapterType::Bool => dst.push_str("boolean"),
         AdapterType::Vector(kind) => dst.push_str(&kind.js_ty()),
         AdapterType::Option(ty) => {
-            adapter2ts(ty, dst, refs);
-            dst.push_str(" | undefined");
+            adapter2ts(ty, position, dst, refs);
+            dst.push_str(match position {
+                TypePosition::Argument => " | null | undefined",
+                TypePosition::Return => " | undefined",
+            });
         }
         AdapterType::NamedExternref(name) => dst.push_str(name),
         AdapterType::Struct(name) => dst.push_str(name),

diff --git a/crates/cli-support/src/js/mod.rs b/crates/cli-support/src/js/mod.rs
@@ -120,6 +120,18 @@ struct FieldAccessor {
     is_optional: bool,
 }
 
+/// Different JS constructs that can be exported.
+enum ExportJs<'a> {
+    /// A class of the form `class Name {...}`.
+    Class(&'a str),
+    /// An anonymous function expression of the form `function(...) {...}`.
+    ///
+    /// Note that the function name is not included in the string.
+    Function(&'a str),
+    /// An arbitrary JS expression.
+    Expression(&'a str),
+}
+
 const INITIAL_HEAP_VALUES: &[&str] = &["undefined", "null", "true", "false"];
 // Must be kept in sync with `src/lib.rs` of the `wasm-bindgen` crate
 const INITIAL_HEAP_OFFSET: usize = 128;
@@ -163,38 +175,46 @@ impl<'a> Context<'a> {
     fn export(
         &mut self,
         export_name: &str,
-        contents: &str,
+        export: ExportJs,
         comments: Option<&str>,
     ) -> Result<(), Error> {
         let definition_name = self.generate_identifier(export_name);
-        if contents.starts_with("class") && definition_name != export_name {
+        if matches!(export, ExportJs::Class(_)) && definition_name != export_name {
             bail!("cannot shadow already defined class `{}`", export_name);
         }
 
-        let contents = contents.trim();
+        // write out comments
         if let Some(c) = comments {
             self.globals.push_str(c);
         }
+
         let global = match self.config.mode {
-            OutputMode::Node { module: false } => {
-                if contents.starts_with("class") {
-                    format!("{}\nmodule.exports.{1} = {1};\n", contents, export_name)
-                } else {
-                    format!("module.exports.{} = {};\n", export_name, contents)
+            OutputMode::Node { module: false } => match export {
+                ExportJs::Class(class) => {
+                    format!("{}\nmodule.exports.{1} = {1};\n", class, export_name)
                 }
-            }
-            OutputMode::NoModules { .. } => {
-                if contents.starts_with("class") {
-                    format!("{}\n__exports.{1} = {1};\n", contents, export_name)
-                } else {
-                    format!("__exports.{} = {};\n", export_name, contents)
+                ExportJs::Function(expr) | ExportJs::Expression(expr) => {
+                    format!("module.exports.{} = {};\n", export_name, expr)
                 }
-            }
+            },
+            OutputMode::NoModules { .. } => match export {
+                ExportJs::Class(class) => {
+                    format!("{}\n__exports.{1} = {1};\n", class, export_name)
+                }
+                ExportJs::Function(expr) | ExportJs::Expression(expr) => {
+                    format!("__exports.{} = {};\n", export_name, expr)
+                }
+            },
             OutputMode::Bundler { .. }
             | OutputMode::Node { module: true }
             | OutputMode::Web
-            | OutputMode::Deno => {
-                if let Some(body) = contents.strip_prefix("function") {
+            | OutputMode::Deno => match export {
+                ExportJs::Class(class) => {
+                    assert_eq!(export_name, definition_name);
+                    format!("export {}\n", class)
+                }
+                ExportJs::Function(function) => {
+                    let body = function.strip_prefix("function").unwrap();
                     if export_name == definition_name {
                         format!("export function {}{}\n", export_name, body)
                     } else {
@@ -203,14 +223,12 @@ impl<'a> Context<'a> {
                             definition_name, body, definition_name, export_name,
                         )
                     }
-                } else if contents.starts_with("class") {
-                    assert_eq!(export_name, definition_name);
-                    format!("export {}\n", contents)
-                } else {
+                }
+                ExportJs::Expression(expr) => {
                     assert_eq!(export_name, definition_name);
-                    format!("export const {} = {};\n", export_name, contents)
+                    format!("export const {} = {};\n", export_name, expr)
                 }
-            }
+            },
         };
         self.global(&global);
         Ok(())
@@ -1169,10 +1187,10 @@ __wbg_set_wasm(wasm);"
 
         self.write_class_field_types(class, &mut ts_dst);
 
-        dst.push_str("}\n");
+        dst.push('}');
         ts_dst.push_str("}\n");
 
-        self.export(name, &dst, Some(&class.comments))?;
+        self.export(name, ExportJs::Class(&dst), Some(&class.comments))?;
 
         if class.generate_typescript {
             self.typescript.push_str(&class.comments);
@@ -2872,7 +2890,11 @@ __wbg_set_wasm(wasm);"
                             self.typescript.push_str(";\n");
                         }
 
-                        self.export(name, &format!("function{}", code), Some(&js_docs))?;
+                        self.export(
+                            name,
+                            ExportJs::Function(&format!("function{}", code)),
+                            Some(&js_docs),
+                        )?;
                         self.globals.push('\n');
                     }
                     AuxExportKind::Constructor(class) => {
@@ -3995,7 +4017,7 @@ __wbg_set_wasm(wasm);"
 
         self.export(
             &enum_.name,
-            &format!("Object.freeze({{\n{}}})", variants),
+            ExportJs::Expression(&format!("Object.freeze({{\n{}}})", variants)),
             Some(&docs),
         )?;
 

diff --git a/crates/cli/tests/reference/echo.d.ts b/crates/cli/tests/reference/echo.d.ts
@@ -36,42 +36,42 @@ export function echo_vec_uninit_i64(a: BigInt64Array): BigInt64Array;
 export function echo_vec_string(a: (string)[]): (string)[];
 export function echo_struct(a: Foo): Foo;
 export function echo_vec_struct(a: (Foo)[]): (Foo)[];
-export function echo_option_u8(a?: number): number | undefined;
-export function echo_option_i8(a?: number): number | undefined;
-export function echo_option_u16(a?: number): number | undefined;
-export function echo_option_i16(a?: number): number | undefined;
-export function echo_option_u32(a?: number): number | undefined;
-export function echo_option_i32(a?: number): number | undefined;
-export function echo_option_u64(a?: bigint): bigint | undefined;
-export function echo_option_i64(a?: bigint): bigint | undefined;
-export function echo_option_u128(a?: bigint): bigint | undefined;
-export function echo_option_i128(a?: bigint): bigint | undefined;
-export function echo_option_usize(a?: number): number | undefined;
-export function echo_option_isize(a?: number): number | undefined;
-export function echo_option_f32(a?: number): number | undefined;
-export function echo_option_f64(a?: number): number | undefined;
-export function echo_option_bool(a?: boolean): boolean | undefined;
-export function echo_option_char(a?: string): string | undefined;
-export function echo_option_string(a?: string): string | undefined;
-export function echo_option_vec_u8(a?: Uint8Array): Uint8Array | undefined;
-export function echo_option_vec_i8(a?: Int8Array): Int8Array | undefined;
-export function echo_option_vec_u16(a?: Uint16Array): Uint16Array | undefined;
-export function echo_option_vec_i16(a?: Int16Array): Int16Array | undefined;
-export function echo_option_vec_u32(a?: Uint32Array): Uint32Array | undefined;
-export function echo_option_vec_i32(a?: Int32Array): Int32Array | undefined;
-export function echo_option_vec_u64(a?: BigUint64Array): BigUint64Array | undefined;
-export function echo_option_vec_i64(a?: BigInt64Array): BigInt64Array | undefined;
-export function echo_option_vec_uninit_u8(a?: Uint8Array): Uint8Array | undefined;
-export function echo_option_vec_uninit_i8(a?: Int8Array): Int8Array | undefined;
-export function echo_option_vec_uninit_u16(a?: Uint16Array): Uint16Array | undefined;
-export function echo_option_vec_uninit_i16(a?: Int16Array): Int16Array | undefined;
-export function echo_option_vec_uninit_u32(a?: Uint32Array): Uint32Array | undefined;
-export function echo_option_vec_uninit_i32(a?: Int32Array): Int32Array | undefined;
-export function echo_option_vec_uninit_u64(a?: BigUint64Array): BigUint64Array | undefined;
-export function echo_option_vec_uninit_i64(a?: BigInt64Array): BigInt64Array | undefined;
-export function echo_option_vec_string(a?: (string)[]): (string)[] | undefined;
-export function echo_option_struct(a?: Foo): Foo | undefined;
-export function echo_option_vec_struct(a?: (Foo)[]): (Foo)[] | undefined;
+export function echo_option_u8(a?: number | null): number | undefined;
+export function echo_option_i8(a?: number | null): number | undefined;
+export function echo_option_u16(a?: number | null): number | undefined;
+export function echo_option_i16(a?: number | null): number | undefined;
+export function echo_option_u32(a?: number | null): number | undefined;
+export function echo_option_i32(a?: number | null): number | undefined;
+export function echo_option_u64(a?: bigint | null): bigint | undefined;
+export function echo_option_i64(a?: bigint | null): bigint | undefined;
+export function echo_option_u128(a?: bigint | null): bigint | undefined;
+export function echo_option_i128(a?: bigint | null): bigint | undefined;
+export function echo_option_usize(a?: number | null): number | undefined;
+export function echo_option_isize(a?: number | null): number | undefined;
+export function echo_option_f32(a?: number | null): number | undefined;
+export function echo_option_f64(a?: number | null): number | undefined;
+export function echo_option_bool(a?: boolean | null): boolean | undefined;
+export function echo_option_char(a?: string | null): string | undefined;
+export function echo_option_string(a?: string | null): string | undefined;
+export function echo_option_vec_u8(a?: Uint8Array | null): Uint8Array | undefined;
+export function echo_option_vec_i8(a?: Int8Array | null): Int8Array | undefined;
+export function echo_option_vec_u16(a?: Uint16Array | null): Uint16Array | undefined;
+export function echo_option_vec_i16(a?: Int16Array | null): Int16Array | undefined;
+export function echo_option_vec_u32(a?: Uint32Array | null): Uint32Array | undefined;
+export function echo_option_vec_i32(a?: Int32Array | null): Int32Array | undefined;
+export function echo_option_vec_u64(a?: BigUint64Array | null): BigUint64Array | undefined;
+export function echo_option_vec_i64(a?: BigInt64Array | null): BigInt64Array | undefined;
+export function echo_option_vec_uninit_u8(a?: Uint8Array | null): Uint8Array | undefined;
+export function echo_option_vec_uninit_i8(a?: Int8Array | null): Int8Array | undefined;
+export function echo_option_vec_uninit_u16(a?: Uint16Array | null): Uint16Array | undefined;
+export function echo_option_vec_uninit_i16(a?: Int16Array | null): Int16Array | undefined;
+export function echo_option_vec_uninit_u32(a?: Uint32Array | null): Uint32Array | undefined;
+export function echo_option_vec_uninit_i32(a?: Int32Array | null): Int32Array | undefined;
+export function echo_option_vec_uninit_u64(a?: BigUint64Array | null): BigUint64Array | undefined;
+export function echo_option_vec_uninit_i64(a?: BigInt64Array | null): BigInt64Array | undefined;
+export function echo_option_vec_string(a?: (string)[] | null): (string)[] | undefined;
+export function echo_option_struct(a?: Foo | null): Foo | undefined;
+export function echo_option_vec_struct(a?: (Foo)[] | null): (Foo)[] | undefined;
 export class Foo {
   private constructor();
   free(): void;