From 3c5a048fe8f9d2d956d419ba5ac48df1095eeb3c Mon Sep 17 00:00:00 2001 From: Dunqing Date: Wed, 15 May 2024 15:01:25 +0800 Subject: [PATCH] feat(transformer): do not add self attribute in react/jsx plugin --- crates/oxc_transformer/src/lib.rs | 4 +- crates/oxc_transformer/src/react/jsx/mod.rs | 61 +++++++++++++------ .../oxc_transformer/src/react/jsx_self/mod.rs | 24 ++++++++ crates/oxc_transformer/src/react/mod.rs | 32 +++------- tasks/transform_conformance/babel.snap.md | 5 +- 5 files changed, 80 insertions(+), 46 deletions(-) diff --git a/crates/oxc_transformer/src/lib.rs b/crates/oxc_transformer/src/lib.rs index 54cbf7e3bf6bb..ded0e4072e366 100644 --- a/crates/oxc_transformer/src/lib.rs +++ b/crates/oxc_transformer/src/lib.rs @@ -140,9 +140,9 @@ impl<'a> Traverse<'a> for Transformer<'a> { self.x0_typescript.transform_export_named_declaration(decl); } - fn enter_expression(&mut self, expr: &mut Expression<'a>, _ctx: &TraverseCtx<'a>) { + fn enter_expression(&mut self, expr: &mut Expression<'a>, ctx: &TraverseCtx<'a>) { self.x0_typescript.transform_expression(expr); - self.x1_react.transform_expression(expr); + self.x1_react.transform_expression(expr, ctx); self.x3_es2015.transform_expression(expr); } diff --git a/crates/oxc_transformer/src/react/jsx/mod.rs b/crates/oxc_transformer/src/react/jsx/mod.rs index e142bce374409..8ca5d9cf096ef 100644 --- a/crates/oxc_transformer/src/react/jsx/mod.rs +++ b/crates/oxc_transformer/src/react/jsx/mod.rs @@ -9,6 +9,7 @@ use oxc_syntax::{ identifier::{is_irregular_whitespace, is_line_terminator}, xml_entities::XML_ENTITIES, }; +use oxc_traverse::TraverseCtx; use crate::{context::Ctx, helpers::module_imports::NamedImport}; @@ -82,12 +83,20 @@ impl<'a> ReactJsx<'a> { self.add_runtime_imports(program); } - pub fn transform_jsx_element(&mut self, e: &JSXElement<'a>) -> Expression<'a> { - self.transform_jsx(&JSXElementOrFragment::Element(e)) + pub fn transform_jsx_element( + &mut self, + e: &JSXElement<'a>, + ctx: &TraverseCtx<'a>, + ) -> Expression<'a> { + self.transform_jsx(&JSXElementOrFragment::Element(e), ctx) } - pub fn transform_jsx_fragment(&mut self, e: &JSXFragment<'a>) -> Expression<'a> { - self.transform_jsx(&JSXElementOrFragment::Fragment(e)) + pub fn transform_jsx_fragment( + &mut self, + e: &JSXFragment<'a>, + ctx: &TraverseCtx<'a>, + ) -> Expression<'a> { + self.transform_jsx(&JSXElementOrFragment::Fragment(e), ctx) } fn is_script(&self) -> bool { @@ -307,7 +316,11 @@ impl<'a> ReactJsx<'a> { /// ### Fragment /// React.createElement(React.Fragment, null, ...children) /// - fn transform_jsx<'b>(&mut self, e: &JSXElementOrFragment<'a, 'b>) -> Expression<'a> { + fn transform_jsx<'b>( + &mut self, + e: &JSXElementOrFragment<'a, 'b>, + ctx: &TraverseCtx<'a>, + ) -> Expression<'a> { let is_fragment = e.is_fragment(); let has_key_after_props_spread = e.has_key_after_props_spread(); // If has_key_after_props_spread is true, we need to fallback to `createElement` same behavior as classic runtime @@ -372,7 +385,7 @@ impl<'a> ReactJsx<'a> { } // Add attribute to prop object - self.transform_jsx_attribute_item(&mut properties, attribute); + self.transform_jsx_attribute_item(&mut properties, attribute, ctx); } } @@ -384,7 +397,7 @@ impl<'a> ReactJsx<'a> { if is_automatic { let allocator = self.ast().allocator; let mut children = Vec::from_iter_in( - children.iter().filter_map(|child| self.transform_jsx_child(child)), + children.iter().filter_map(|child| self.transform_jsx_child(child, ctx)), allocator, ); let children_len = children.len(); @@ -411,7 +424,9 @@ impl<'a> ReactJsx<'a> { // React.createElement's second argument if !is_fragment && is_classic { - if self.options.is_jsx_self_plugin_enabled() { + if self.options.is_jsx_self_plugin_enabled() + && self.jsx_self.can_add_self_attribute(ctx) + { if let Some(span) = self_attr_span { self.jsx_self.report_error(span); } else { @@ -447,7 +462,7 @@ impl<'a> ReactJsx<'a> { if is_automatic { // key if key_prop.is_some() { - arguments.push(Argument::from(self.transform_jsx_attribute_value(key_prop))); + arguments.push(Argument::from(self.transform_jsx_attribute_value(key_prop, ctx))); } else if is_development { arguments.push(Argument::from(self.ctx.ast.void_0())); } @@ -475,7 +490,9 @@ impl<'a> ReactJsx<'a> { } // this - if self.options.is_jsx_self_plugin_enabled() { + if self.options.is_jsx_self_plugin_enabled() + && self.jsx_self.can_add_self_attribute(ctx) + { if let Some(span) = self_attr_span { self.jsx_self.report_error(span); } else { @@ -489,7 +506,7 @@ impl<'a> ReactJsx<'a> { arguments.extend( children .iter() - .filter_map(|child| self.transform_jsx_child(child)) + .filter_map(|child| self.transform_jsx_child(child, ctx)) .map(Argument::from), ); } @@ -652,12 +669,13 @@ impl<'a> ReactJsx<'a> { &mut self, properties: &mut Vec<'a, ObjectPropertyKind<'a>>, attribute: &JSXAttributeItem<'a>, + ctx: &TraverseCtx<'a>, ) { match attribute { JSXAttributeItem::Attribute(attr) => { let kind = PropertyKind::Init; let key = self.get_attribute_name(&attr.name); - let value = self.transform_jsx_attribute_value(attr.value.as_ref()); + let value = self.transform_jsx_attribute_value(attr.value.as_ref(), ctx); let object_property = self.ast().object_property(SPAN, kind, key, value, None, false, false, false); let object_property = ObjectPropertyKind::ObjectProperty(object_property); @@ -680,6 +698,7 @@ impl<'a> ReactJsx<'a> { fn transform_jsx_attribute_value( &mut self, value: Option<&JSXAttributeValue<'a>>, + ctx: &TraverseCtx<'a>, ) -> Expression<'a> { match value { Some(JSXAttributeValue::StringLiteral(s)) => { @@ -688,10 +707,10 @@ impl<'a> ReactJsx<'a> { self.ast().literal_string_expression(literal) } Some(JSXAttributeValue::Element(e)) => { - self.transform_jsx(&JSXElementOrFragment::Element(e)) + self.transform_jsx(&JSXElementOrFragment::Element(e), ctx) } Some(JSXAttributeValue::Fragment(e)) => { - self.transform_jsx(&JSXElementOrFragment::Fragment(e)) + self.transform_jsx(&JSXElementOrFragment::Fragment(e), ctx) } Some(JSXAttributeValue::ExpressionContainer(c)) => match &c.expression { e @ match_expression!(JSXExpression) => self.ast().copy(e.to_expression()), @@ -703,15 +722,23 @@ impl<'a> ReactJsx<'a> { } } - fn transform_jsx_child(&mut self, child: &JSXChild<'a>) -> Option> { + fn transform_jsx_child( + &mut self, + child: &JSXChild<'a>, + ctx: &TraverseCtx<'a>, + ) -> Option> { match child { JSXChild::Text(text) => self.transform_jsx_text(text.value.as_str()), JSXChild::ExpressionContainer(e) => match &e.expression { e @ match_expression!(JSXExpression) => Some(self.ast().copy(e.to_expression())), JSXExpression::EmptyExpression(_) => None, }, - JSXChild::Element(e) => Some(self.transform_jsx(&JSXElementOrFragment::Element(e))), - JSXChild::Fragment(e) => Some(self.transform_jsx(&JSXElementOrFragment::Fragment(e))), + JSXChild::Element(e) => { + Some(self.transform_jsx(&JSXElementOrFragment::Element(e), ctx)) + } + JSXChild::Fragment(e) => { + Some(self.transform_jsx(&JSXElementOrFragment::Fragment(e), ctx)) + } JSXChild::Spread(e) => { self.ctx.error(diagnostics::spread_children_are_not_supported(e.span)); None diff --git a/crates/oxc_transformer/src/react/jsx_self/mod.rs b/crates/oxc_transformer/src/react/jsx_self/mod.rs index dab555a5a33bf..80df7da2a143c 100644 --- a/crates/oxc_transformer/src/react/jsx_self/mod.rs +++ b/crates/oxc_transformer/src/react/jsx_self/mod.rs @@ -3,6 +3,7 @@ use std::rc::Rc; use oxc_ast::ast::*; use oxc_diagnostics::OxcDiagnostic; use oxc_span::{Span, SPAN}; +use oxc_traverse::{Ancestor, FinderRet, TraverseCtx}; use crate::context::Ctx; @@ -46,6 +47,29 @@ impl<'a> ReactJsxSelf<'a> { let error = OxcDiagnostic::warning("Duplicate __self prop found.").with_label(span); self.ctx.error(error); } + + #[allow(clippy::unused_self)] + fn is_inside_constructor(&self, ctx: &TraverseCtx<'a>) -> bool { + ctx.find_scope(|scope| { + if scope.is_block() || scope.is_arrow() { + return FinderRet::Continue; + } + FinderRet::Found(scope.is_constructor()) + }) + .unwrap_or(false) + } + + fn has_no_super_class(ctx: &TraverseCtx<'a>) -> bool { + ctx.find_ancestor(|ancestor| match ancestor { + Ancestor::ClassBody(class) => FinderRet::Found(class.super_class().is_none()), + _ => FinderRet::Continue, + }) + .unwrap_or(true) + } + + pub fn can_add_self_attribute(&self, ctx: &TraverseCtx<'a>) -> bool { + !self.is_inside_constructor(ctx) || Self::has_no_super_class(ctx) + } } impl<'a> ReactJsxSelf<'a> { diff --git a/crates/oxc_transformer/src/react/mod.rs b/crates/oxc_transformer/src/react/mod.rs index e4bc4dce577db..a03833192bbd7 100644 --- a/crates/oxc_transformer/src/react/mod.rs +++ b/crates/oxc_transformer/src/react/mod.rs @@ -8,7 +8,7 @@ mod utils; use std::rc::Rc; use oxc_ast::ast::*; -use oxc_traverse::{Ancestor, FinderRet, TraverseCtx}; +use oxc_traverse::TraverseCtx; use crate::context::Ctx; @@ -52,16 +52,16 @@ impl<'a> React<'a> { } } - pub fn transform_expression(&mut self, expr: &mut Expression<'a>) { + pub fn transform_expression(&mut self, expr: &mut Expression<'a>, ctx: &TraverseCtx<'a>) { match expr { Expression::JSXElement(e) => { if self.options.is_jsx_plugin_enabled() { - *expr = self.jsx.transform_jsx_element(e); + *expr = self.jsx.transform_jsx_element(e, ctx); } } Expression::JSXFragment(e) => { if self.options.is_jsx_plugin_enabled() { - *expr = self.jsx.transform_jsx_fragment(e); + *expr = self.jsx.transform_jsx_fragment(e, ctx); } } _ => {} @@ -83,26 +83,10 @@ impl<'a> React<'a> { elem: &mut JSXOpeningElement<'a>, ctx: &TraverseCtx<'a>, ) { - if self.options.is_jsx_self_plugin_enabled() { - let is_constructor = ctx - .find_scope(|scope| { - if scope.is_block() || scope.is_arrow() { - return FinderRet::Continue; - } - FinderRet::Found(scope.is_constructor()) - }) - .unwrap_or(false); - if !is_constructor - // no super class - || ctx - .find_ancestor(|ancestor| match ancestor { - Ancestor::ClassBody(class) => FinderRet::Found(class.super_class().is_none()), - _ => FinderRet::Continue, - }) - .unwrap_or(true) - { - self.jsx.jsx_self.transform_jsx_opening_element(elem); - } + if self.options.is_jsx_self_plugin_enabled() + && self.jsx.jsx_self.can_add_self_attribute(ctx) + { + self.jsx.jsx_self.transform_jsx_opening_element(elem); } if self.options.is_jsx_source_plugin_enabled() { self.jsx.jsx_source.transform_jsx_opening_element(elem); diff --git a/tasks/transform_conformance/babel.snap.md b/tasks/transform_conformance/babel.snap.md index fead6a775b13c..7bbec6d006a6e 100644 --- a/tasks/transform_conformance/babel.snap.md +++ b/tasks/transform_conformance/babel.snap.md @@ -1,6 +1,6 @@ commit: 4bd1b2c2 -Passed: 309/362 +Passed: 310/362 # All Passed: * babel-preset-react @@ -66,9 +66,8 @@ Passed: 309/362 * autoImport/complicated-scope-module/input.js * react-automatic/should-throw-when-filter-is-specified/input.js -# babel-plugin-transform-react-jsx-development (8/12) +# babel-plugin-transform-react-jsx-development (9/12) * cross-platform/self-inside-arrow/input.mjs * cross-platform/source-and-self-defined/input.js -* cross-platform/within-derived-classes-constructor/input.js * cross-platform/within-ts-module-block/input.ts