From 3acb3f6461ddc5f7da6d9c2821ee29ebf6d20645 Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Thu, 29 Aug 2024 00:38:02 +0000 Subject: [PATCH] fix(transformer/react): mismatch output caused by incorrect transformation ordering (#5255) close: #4767 --- crates/oxc_transformer/src/lib.rs | 1 + crates/oxc_transformer/src/react/mod.rs | 10 + crates/oxc_transformer/src/react/refresh.rs | 341 +++++++++----------- tasks/transform_conformance/oxc.snap.md | 39 ++- 4 files changed, 184 insertions(+), 207 deletions(-) diff --git a/crates/oxc_transformer/src/lib.rs b/crates/oxc_transformer/src/lib.rs index 7a475bfb4d3b5..cbde6cfa7a6f6 100644 --- a/crates/oxc_transformer/src/lib.rs +++ b/crates/oxc_transformer/src/lib.rs @@ -215,6 +215,7 @@ impl<'a> Traverse<'a> for Transformer<'a> { fn exit_function(&mut self, func: &mut Function<'a>, ctx: &mut TraverseCtx<'a>) { self.x0_typescript.transform_function(func); + self.x1_react.transform_function_on_exit(func, ctx); self.x3_es2015.exit_function(func, ctx); } diff --git a/crates/oxc_transformer/src/react/mod.rs b/crates/oxc_transformer/src/react/mod.rs index 35ec6f74afeed..78f1c77885193 100644 --- a/crates/oxc_transformer/src/react/mod.rs +++ b/crates/oxc_transformer/src/react/mod.rs @@ -155,4 +155,14 @@ impl<'a> React<'a> { self.refresh.transform_expression_on_exit(expr, ctx); } } + + pub fn transform_function_on_exit( + &mut self, + func: &mut Function<'a>, + ctx: &mut TraverseCtx<'a>, + ) { + if self.refresh_plugin { + self.refresh.transform_function_on_exit(func, ctx); + } + } } diff --git a/crates/oxc_transformer/src/react/refresh.rs b/crates/oxc_transformer/src/react/refresh.rs index adad21ec62ed1..e2cfcc823f73c 100644 --- a/crates/oxc_transformer/src/react/refresh.rs +++ b/crates/oxc_transformer/src/react/refresh.rs @@ -1,4 +1,4 @@ -use std::cell::Cell; +use std::{cell::Cell, iter::once}; use oxc_allocator::CloneIn; use oxc_ast::{ @@ -8,6 +8,7 @@ use oxc_semantic::{ReferenceFlags, ScopeFlags, ScopeId, SymbolFlags, SymbolId}; use oxc_span::{Atom, GetSpan, Span, SPAN}; use oxc_syntax::operator::AssignmentOperator; use oxc_traverse::{Ancestor, TraverseCtx}; +use rustc_hash::FxHashMap; use super::options::ReactRefreshOptions; @@ -31,6 +32,7 @@ pub struct ReactRefresh<'a> { /// Used to wrap call expression with signature. /// (eg: hoc(() => {}) -> _s1(hoc(_s1(() => {})))) last_signature: Option<(BindingIdentifier<'a>, oxc_allocator::Vec<'a, Argument<'a>>)>, + extra_statements: FxHashMap>>, } impl<'a> ReactRefresh<'a> { @@ -44,6 +46,7 @@ impl<'a> ReactRefresh<'a> { registrations: Vec::default(), ctx, last_signature: None, + extra_statements: FxHashMap::default(), } } @@ -411,117 +414,6 @@ impl<'a> ReactRefresh<'a> { // --------------------------- refresh sig --------------------------- - /// Modify a function to insert a signature call, - /// and return a statement to insert it after current statement - /// - /// ```js - /// function Foo() {}; - /// ``` - /// to - /// ```js - /// function Foo() { _s() }; _s(Foo, ...); - /// ``` - fn modify_function_for_signature( - &mut self, - func: &mut Function<'a>, - ctx: &mut TraverseCtx<'a>, - ) -> Option> { - let id = func.id.as_ref()?; - let (binding_identifier, mut arguments) = - self.create_signature_call_expression(func.scope_id.get()?, func.body.as_mut()?, ctx)?; - - arguments.insert( - 0, - Argument::from(Self::create_identifier_reference_from_binding_identifier(id, ctx)), - ); - - Some(ctx.ast.statement_expression( - SPAN, - ctx.ast.expression_call( - SPAN, - Self::create_identifier_reference_from_binding_identifier(&binding_identifier, ctx), - Option::::None, - arguments, - false, - ), - )) - } - - /// Modify all variable declarations to insert a signature call, - /// and return a set of statements to insert it after current statement - /// - /// ```js - /// let Foo = () => {}; - /// let Foo = function() {}; - /// let Foo = styled.div`` - /// ``` - /// to - /// ```js - /// let Foo = () => {_s()}; _s(Foo, ...); - /// let Foo = function() {_s()}; _s(Foo, ...); - /// let Foo = styled.div``; _s(Foo, ...); - /// ``` - fn modify_variable_declaration_for_signature( - &mut self, - decl: &mut VariableDeclaration<'a>, - ctx: &mut TraverseCtx<'a>, - ) -> Vec> { - decl.declarations - .iter_mut() - .filter_map(|declarator| { - let id = declarator.id.get_binding_identifier()?; - let init = declarator.init.as_mut()?; - - let (scope_id, body) = match init { - Expression::FunctionExpression(func) => { - (func.scope_id.get(), func.body.as_mut()?) - } - Expression::ArrowFunctionExpression(arrow) => { - (arrow.scope_id.get(), &mut arrow.body) - } - _ => { - return None; - } - }; - - // 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, ...); - - let (binding_identifier, mut arguments) = - self.create_signature_call_expression(scope_id.unwrap(), body, ctx)?; - if let Expression::ArrowFunctionExpression(arrow) = init { - Self::transform_arrow_function_to_block(arrow, ctx); - } - - arguments.insert( - 0, - Argument::from(Self::create_identifier_reference_from_binding_identifier( - id, ctx, - )), - ); - - Some(ctx.ast.statement_expression( - SPAN, - ctx.ast.expression_call( - SPAN, - Self::create_identifier_reference_from_binding_identifier( - &binding_identifier, - ctx, - ), - Option::::None, - arguments, - false, - ), - )) - }) - .collect() - } - /// Convert arrow function expression to normal arrow function /// /// ```js @@ -659,72 +551,24 @@ impl<'a> ReactRefresh<'a> { let mut new_stmts = ctx.ast.vec_with_capacity(stmts.len() + 1); - for mut stmt in stmts.drain(..) { - match &mut stmt { - Statement::FunctionDeclaration(func) => { - let call_signature_statement = self.modify_function_for_signature(func, ctx); - new_stmts.push(stmt); - new_stmts.extend(call_signature_statement); - continue; - } - Statement::VariableDeclaration(decl) => { - let call_signature_statements = - self.modify_variable_declaration_for_signature(decl, ctx); - new_stmts.push(stmt); - new_stmts.extend(call_signature_statements); - continue; - } - Statement::ExportNamedDeclaration(export_decl) => { - if let Some(Declaration::FunctionDeclaration(func)) = - &mut export_decl.declaration - { - let call_signature_statement = - self.modify_function_for_signature(func, ctx); - new_stmts.push(stmt); - new_stmts.extend(call_signature_statement); - continue; - } else if let Some(Declaration::VariableDeclaration(decl)) = - &mut export_decl.declaration - { - let call_signature_statements = - self.modify_variable_declaration_for_signature(decl, ctx); - new_stmts.push(stmt); - new_stmts.extend(call_signature_statements); - continue; - } - } - Statement::ExportDefaultDeclaration(export_decl) => { - if let ExportDefaultDeclarationKind::FunctionDeclaration(func) = - &mut export_decl.declaration - { - if func.id.is_some() { - if let Some(bind_sig_statement) = - self.modify_function_for_signature(func, ctx) - { - new_stmts.push(stmt); - new_stmts.push(bind_sig_statement); - continue; - } - } - } - } - _ => {} - }; - new_stmts.push(stmt); - } - let declarations = self.signature_declarator_items.pop().unwrap(); if !declarations.is_empty() { - new_stmts.insert( - 0, - Statement::from(ctx.ast.declaration_variable( - SPAN, - VariableDeclarationKind::Var, - declarations, - false, - )), - ); + new_stmts.push(Statement::from(ctx.ast.declaration_variable( + SPAN, + VariableDeclarationKind::Var, + declarations, + false, + ))); } + new_stmts.extend(stmts.drain(..).flat_map(move |stmt| { + let symbol_ids = get_symbol_id_from_function_and_declarator(&stmt); + let extra_stmts = symbol_ids + .into_iter() + .filter_map(|symbol_id| self.extra_statements.remove(&symbol_id)) + .flatten() + .collect::>(); + once(stmt).chain(extra_stmts) + })); *stmts = new_stmts; } @@ -746,16 +590,13 @@ impl<'a> ReactRefresh<'a> { expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>, ) { - let is_variable_declarator = matches!(ctx.parent(), Ancestor::VariableDeclaratorInit(_)); - let signature = match expr { - Expression::FunctionExpression(func) if !is_variable_declarator => self - .create_signature_call_expression( - func.scope_id.get().unwrap(), - func.body.as_mut().unwrap(), - ctx, - ), - Expression::ArrowFunctionExpression(arrow) if !is_variable_declarator => { + Expression::FunctionExpression(func) => self.create_signature_call_expression( + func.scope_id.get().unwrap(), + func.body.as_mut().unwrap(), + ctx, + ), + Expression::ArrowFunctionExpression(arrow) => { let call_fn = self.create_signature_call_expression( arrow.scope_id.get().unwrap(), &mut arrow.body, @@ -776,10 +617,49 @@ impl<'a> ReactRefresh<'a> { _ => None, }; - let Some(signature) = signature else { + let Some((binding_identifier, mut arguments)) = signature else { return; }; + if !matches!(expr, Expression::CallExpression(_)) { + if let Ancestor::VariableDeclaratorInit(declarator) = ctx.parent() { + // 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, ...); + let id = declarator.id().get_binding_identifier().unwrap(); + let symbol_id = id.symbol_id.get().unwrap(); + let first_argument = Argument::from(ctx.ast.expression_from_identifier_reference( + ctx.create_reference_id( + SPAN, + id.name.clone(), + Some(symbol_id), + ReferenceFlags::Read, + ), + )); + arguments.insert(0, first_argument); + + let statement = ctx.ast.statement_expression( + SPAN, + ctx.ast.expression_call( + SPAN, + Self::create_identifier_reference_from_binding_identifier( + &binding_identifier, + ctx, + ), + Option::::None, + arguments, + false, + ), + ); + self.extra_statements.entry(symbol_id).or_insert(ctx.ast.vec()).push(statement); + return; + } + } + let mut found_call_expression = false; for ancestor in ctx.ancestors() { if ancestor.is_assignment_expression() { @@ -793,10 +673,9 @@ impl<'a> ReactRefresh<'a> { if found_call_expression { self.last_signature = - Some((signature.0.clone(), signature.1.clone_in(ctx.ast.allocator))); + Some((binding_identifier.clone(), arguments.clone_in(ctx.ast.allocator))); } - let (binding_identifier, mut arguments) = signature; arguments.insert(0, Argument::from(ctx.ast.move_expression(expr))); *expr = self.ctx.ast.expression_call( SPAN, @@ -806,6 +685,59 @@ impl<'a> ReactRefresh<'a> { false, ); } + + /// Modify a function to insert a signature call, + /// and return a statement to insert it after current statement + /// + /// ```js + /// function Foo() {}; + /// ``` + /// to + /// ```js + /// function Foo() { _s() }; _s(Foo, ...); + /// ``` + pub fn transform_function_on_exit( + &mut self, + func: &mut Function<'a>, + ctx: &mut TraverseCtx<'a>, + ) { + if !func.is_function_declaration() { + return; + } + + let Some((binding_identifier, mut arguments)) = self.create_signature_call_expression( + func.scope_id.get().unwrap(), + func.body.as_mut().unwrap(), + ctx, + ) else { + return; + }; + + let Some(id) = func.id.as_ref() else { + return; + }; + + arguments.insert( + 0, + Argument::from(Self::create_identifier_reference_from_binding_identifier(id, ctx)), + ); + + self.extra_statements.entry(id.symbol_id.get().unwrap()).or_insert(ctx.ast.vec()).push( + ctx.ast.statement_expression( + SPAN, + ctx.ast.expression_call( + SPAN, + Self::create_identifier_reference_from_binding_identifier( + &binding_identifier, + ctx, + ), + Option::::None, + arguments, + false, + ), + ), + ); + } } // TODO: Try to remove this struct, avoid double visit @@ -1034,3 +966,38 @@ fn is_builtin_hook(hook_name: &str) -> bool { "useOptimistic" ) } + +fn get_symbol_id_from_function_and_declarator(stmt: &Statement<'_>) -> Vec { + let mut symbol_ids = vec![]; + match stmt { + Statement::FunctionDeclaration(ref func) => { + symbol_ids.push(func.id.as_ref().unwrap().symbol_id.get().unwrap()); + } + Statement::VariableDeclaration(ref decl) => { + symbol_ids.extend(decl.declarations.iter().filter_map(|decl| { + decl.id.get_binding_identifier().and_then(|id| id.symbol_id.get()) + })); + } + Statement::ExportNamedDeclaration(ref export_decl) => { + if let Some(Declaration::FunctionDeclaration(func)) = &export_decl.declaration { + symbol_ids.push(func.id.as_ref().unwrap().symbol_id.get().unwrap()); + } else if let Some(Declaration::VariableDeclaration(decl)) = &export_decl.declaration { + symbol_ids.extend(decl.declarations.iter().filter_map(|decl| { + decl.id.get_binding_identifier().and_then(|id| id.symbol_id.get()) + })); + } + } + Statement::ExportDefaultDeclaration(ref export_decl) => { + if let ExportDefaultDeclarationKind::FunctionDeclaration(func) = + &export_decl.declaration + { + if let Some(id) = func.id.as_ref() { + symbol_ids.push(id.symbol_id.get().unwrap()); + } + } + } + _ => {} + }; + + symbol_ids +} diff --git a/tasks/transform_conformance/oxc.snap.md b/tasks/transform_conformance/oxc.snap.md index 66aaea01797a7..4ccf5b80f070b 100644 --- a/tasks/transform_conformance/oxc.snap.md +++ b/tasks/transform_conformance/oxc.snap.md @@ -212,7 +212,6 @@ Passed: 10/37 # babel-plugin-transform-react-jsx (6/27) * refresh/can-handle-implicit-arrow-returns/input.jsx - x Output mismatch x Symbol reference IDs mismatch: | after transform: SymbolId(9): [ReferenceId(23), ReferenceId(24), | ReferenceId(25)] @@ -220,24 +219,24 @@ Passed: 10/37 x Symbol reference IDs mismatch: | after transform: SymbolId(10): [ReferenceId(26), ReferenceId(27), - | ReferenceId(28)] - | rebuilt : SymbolId(1): [ReferenceId(18), ReferenceId(19)] + | ReferenceId(29)] + | rebuilt : SymbolId(1): [ReferenceId(10), ReferenceId(13)] x Symbol reference IDs mismatch: - | after transform: SymbolId(11): [ReferenceId(29), ReferenceId(30), - | ReferenceId(31), ReferenceId(32)] - | rebuilt : SymbolId(2): [ReferenceId(29), ReferenceId(32), - | ReferenceId(33)] + | after transform: SymbolId(11): [ReferenceId(30), ReferenceId(31), + | ReferenceId(32)] + | rebuilt : SymbolId(2): [ReferenceId(18), ReferenceId(19)] x Symbol reference IDs mismatch: | after transform: SymbolId(12): [ReferenceId(33), ReferenceId(34), | ReferenceId(36)] - | rebuilt : SymbolId(3): [ReferenceId(10), ReferenceId(13)] + | rebuilt : SymbolId(3): [ReferenceId(22), ReferenceId(25)] x Symbol reference IDs mismatch: | after transform: SymbolId(13): [ReferenceId(37), ReferenceId(38), - | ReferenceId(40)] - | rebuilt : SymbolId(4): [ReferenceId(22), ReferenceId(25)] + | ReferenceId(39), ReferenceId(40)] + | rebuilt : SymbolId(4): [ReferenceId(29), ReferenceId(32), + | ReferenceId(33)] x Symbol reference IDs mismatch: | after transform: SymbolId(14): [ReferenceId(41), ReferenceId(42), @@ -278,7 +277,7 @@ Passed: 10/37 | rebuilt : ReferenceId(1): None x Reference symbol mismatch: - | after transform: ReferenceId(29): Some("_s3") + | after transform: ReferenceId(30): Some("_s3") | rebuilt : ReferenceId(2): None x Reference symbol mismatch: @@ -520,8 +519,8 @@ Passed: 10/37 | rebuilt : SymbolId(0): [ReferenceId(1), ReferenceId(16)] x Symbol reference IDs mismatch: - | after transform: SymbolId(9): [ReferenceId(12), ReferenceId(13), - | ReferenceId(15)] + | after transform: SymbolId(8): [ReferenceId(11), ReferenceId(12), + | ReferenceId(14)] | rebuilt : SymbolId(4): [ReferenceId(3), ReferenceId(7)] x Symbol reference IDs mismatch: @@ -534,7 +533,7 @@ Passed: 10/37 | rebuilt : ReferenceId(0): None x Reference symbol mismatch: - | after transform: ReferenceId(12): Some("_s") + | after transform: ReferenceId(11): Some("_s") | rebuilt : ReferenceId(2): None x Reference symbol mismatch: @@ -553,13 +552,13 @@ Passed: 10/37 x Missing ScopeId x Symbol reference IDs mismatch: - | after transform: SymbolId(8): [ReferenceId(10), ReferenceId(11), - | ReferenceId(13)] + | after transform: SymbolId(7): [ReferenceId(9), ReferenceId(10), + | ReferenceId(12)] | rebuilt : SymbolId(1): [ReferenceId(3), ReferenceId(7)] x Symbol reference IDs mismatch: - | after transform: SymbolId(9): [ReferenceId(14), ReferenceId(15), - | ReferenceId(17)] + | after transform: SymbolId(8): [ReferenceId(13), ReferenceId(14), + | ReferenceId(16)] | rebuilt : SymbolId(2): [ReferenceId(10), ReferenceId(12)] x Symbol reference IDs mismatch: @@ -573,11 +572,11 @@ Passed: 10/37 | rebuilt : SymbolId(10): [ReferenceId(21), ReferenceId(24)] x Reference symbol mismatch: - | after transform: ReferenceId(10): Some("_s") + | after transform: ReferenceId(9): Some("_s") | rebuilt : ReferenceId(0): None x Reference symbol mismatch: - | after transform: ReferenceId(14): Some("_s2") + | after transform: ReferenceId(13): Some("_s2") | rebuilt : ReferenceId(1): None x Reference symbol mismatch: