refactor(transformer/react-refresh): avoid panic for init of VariableDeclarator isn't a BindingIdentifier (#6937)

related: https://github.com/oxc-project/oxc/pull/6881#discussion_r1816597462

crates/oxc_transformer/src/jsx/refresh.rs

@@ -2,7 +2,7 @@ use base64::prelude::{Engine, BASE64_STANDARD};
 use rustc_hash::FxHashMap;
 use sha1::{Digest, Sha1};
 
-use oxc_allocator::{CloneIn, GetAddress};
+use oxc_allocator::{CloneIn, GetAddress, Vec as ArenaVec};
 use oxc_ast::{ast::*, match_expression, AstBuilder, NONE};
 use oxc_semantic::{Reference, ReferenceFlags, ScopeFlags, ScopeId, SymbolFlags};
 use oxc_span::{Atom, GetSpan, SPAN};
@@ -103,7 +103,7 @@ pub struct ReactRefresh<'a, 'ctx> {
     registrations: Vec<(BoundIdentifier<'a>, Atom<'a>)>,
     /// Used to wrap call expression with signature.
     /// (eg: hoc(() => {}) -> _s1(hoc(_s1(() => {}))))
-    last_signature: Option<(BindingIdentifier<'a>, oxc_allocator::Vec<'a, Argument<'a>>)>,
+    last_signature: Option<(BindingIdentifier<'a>, ArenaVec<'a, Argument<'a>>)>,
     // (function_scope_id, (hook_name, hook_key, custom_hook_callee)
     hook_calls: FxHashMap<ScopeId, Vec<(Atom<'a>, Atom<'a>)>>,
     non_builtin_hooks_callee: FxHashMap<ScopeId, Vec<Option<Expression<'a>>>>,
@@ -210,50 +210,14 @@ impl<'a, 'ctx> Traverse<'a> for ReactRefresh<'a, 'ctx> {
         let binding = BoundIdentifier::from_binding_ident(&binding_identifier);
 
         if !matches!(expr, Expression::CallExpression(_)) {
-            if let Ancestor::VariableDeclaratorInit(declarator) = ctx.parent() {
+            // Try to get binding from parent VariableDeclarator
-                // Special case when a function would get an inferred name:
+            let id_binding = if let Ancestor::VariableDeclaratorInit(declarator) = ctx.parent() {
-                // let Foo = () => {}
+                declarator.id().get_binding_identifier().map(BoundIdentifier::from_binding_ident)
-                // let Foo = function() {}
+            } else {
-                // We'll add signature it on next line so that
+                None
-                // we don't mess up the inferred 'Foo' function name.
+            };
-
+            if let Some(id_binding) = id_binding {
-                // Result: let Foo = () => {}; __signature(Foo, ...);
+                self.handle_function_in_variable_declarator(&id_binding, &binding, arguments, ctx);
-                let id = declarator.id().get_binding_identifier().unwrap();
-                let id_binding = BoundIdentifier::from_binding_ident(id);
-
-                let first_argument = Argument::from(id_binding.create_read_expression(ctx));
-                arguments.insert(0, first_argument);
-                let statement = ctx.ast.statement_expression(
-                    SPAN,
-                    ctx.ast.expression_call(
-                        SPAN,
-                        binding.create_read_expression(ctx),
-                        NONE,
-                        arguments,
-                        false,
-                    ),
-                );
-
-                // Get the address of the statement containing this `VariableDeclarator`
-                #[allow(clippy::single_match_else)]
-                let address = match ctx.ancestor(2) {
-                    // For `export const Foo = () => {}`
-                    // which is a `VariableDeclaration` inside a `Statement::ExportNamedDeclaration`
-                    Ancestor::ExportNamedDeclarationDeclaration(export_decl) => {
-                        export_decl.address()
-                    }
-                    // Otherwise just a `const Foo = () => {}`
-                    // which is a `Statement::VariableDeclaration`
-                    _ => {
-                        let var_decl = ctx.ancestor(1);
-                        debug_assert!(matches!(
-                            var_decl,
-                            Ancestor::VariableDeclarationDeclarations(_)
-                        ));
-                        var_decl.address()
-                    }
-                };
-                self.ctx.statement_injector.insert_after(&address, statement);
                 return;
             }
         }
@@ -542,7 +506,7 @@ impl<'a, 'ctx> ReactRefresh<'a, 'ctx> {
         scope_id: ScopeId,
         body: &mut FunctionBody<'a>,
         ctx: &mut TraverseCtx<'a>,
-    ) -> Option<(BindingIdentifier<'a>, oxc_allocator::Vec<'a, Argument<'a>>)> {
+    ) -> Option<(BindingIdentifier<'a>, ArenaVec<'a, Argument<'a>>)> {
         let fn_hook_calls = self.hook_calls.remove(&scope_id)?;
 
         let mut key = fn_hook_calls
@@ -821,6 +785,50 @@ impl<'a, 'ctx> ReactRefresh<'a, 'ctx> {
         Some(self.create_assignment_expression(id, ctx))
     }
 
+    /// Handle `export const Foo = () => {}` or `const Foo = function() {}`
+    fn handle_function_in_variable_declarator(
+        &self,
+        id_binding: &BoundIdentifier<'a>,
+        binding: &BoundIdentifier<'a>,
+        mut arguments: ArenaVec<'a, Argument<'a>>,
+        ctx: &mut TraverseCtx<'a>,
+    ) {
+        // Special case when a function would get an inferred name:
+        // let Foo = () => {}
+        // let Foo = function() {}
+        // We'll add signature it on next line so that
+        // we don't mess up the inferred 'Foo' function name.
+
+        // Result: let Foo = () => {}; __signature(Foo, ...);
+        arguments.insert(0, Argument::from(id_binding.create_read_expression(ctx)));
+        let statement = ctx.ast.statement_expression(
+            SPAN,
+            ctx.ast.expression_call(
+                SPAN,
+                binding.create_read_expression(ctx),
+                NONE,
+                arguments,
+                false,
+            ),
+        );
+
+        // Get the address of the statement containing this `VariableDeclarator`
+        #[allow(clippy::single_match_else)]
+        let address = match ctx.ancestor(2) {
+            // For `export const Foo = () => {}`
+            // which is a `VariableDeclaration` inside a `Statement::ExportNamedDeclaration`
+            Ancestor::ExportNamedDeclarationDeclaration(export_decl) => export_decl.address(),
+            // Otherwise just a `const Foo = () => {}`
+            // which is a `Statement::VariableDeclaration`
+            _ => {
+                let var_decl = ctx.ancestor(1);
+                debug_assert!(matches!(var_decl, Ancestor::VariableDeclarationDeclarations(_)));
+                var_decl.address()
+            }
+        };
+        self.ctx.statement_injector.insert_after(&address, statement);
+    }
+
     /// Convert arrow function expression to normal arrow function
     ///
     /// ```js

tasks/transform_conformance/snapshots/oxc.snap.md

@@ -1,6 +1,6 @@
 commit: d20b314c
 
-Passed: 73/82
+Passed: 74/83
 
 # All Passed:
 * babel-plugin-transform-class-static-block
@@ -167,7 +167,7 @@ rebuilt        : SymbolId(2): []
 x Output mismatch
 
 
-# babel-plugin-transform-react-jsx (30/32)
+# babel-plugin-transform-react-jsx (31/33)
 * refresh/does-not-transform-it-because-it-is-not-used-in-the-AST/input.jsx
 x Output mismatch
 

tasks/transform_conformance/tests/babel-plugin-transform-react-jsx/test/fixtures/refresh/variable-declarator-with-function/input.js

@@ -0,0 +1,4 @@
+const { name, length } = function (X) {
+  useContext(X);
+}
+const { ...args } = () => useContext(X);

tasks/transform_conformance/tests/babel-plugin-transform-react-jsx/test/fixtures/refresh/variable-declarator-with-function/output.js

@@ -0,0 +1,9 @@
+var _s = $RefreshSig$(), _s2 = $RefreshSig$();
+const { name, length } = _s(function(X) {
+        _s();
+        useContext(X);
+}, "useContext{}");
+const { ...args } = _s2(() => {
+        _s2();
+        return useContext(X);
+}, "useContext{}");