From 37cdc1303061c3ff24315238a0fb5c7a9cbe930b Mon Sep 17 00:00:00 2001 From: overlookmotel <557937+overlookmotel@users.noreply.github.com> Date: Fri, 7 Jun 2024 03:16:41 +0000 Subject: [PATCH] perf(transformer): faster checks if JSX plugin enabled (#3577) Checks for whether JSX transforms are enabled are on a hot path. Make them as fast as possible: 1. Only check a single bool. 2. Store flags directly in `self` rather than behind a reference. --- .../src/options/transformer.rs | 4 +- crates/oxc_transformer/src/react/jsx/mod.rs | 16 +++---- crates/oxc_transformer/src/react/mod.rs | 46 +++++++++++-------- crates/oxc_transformer/src/react/options.rs | 16 +++---- 4 files changed, 41 insertions(+), 41 deletions(-) diff --git a/crates/oxc_transformer/src/options/transformer.rs b/crates/oxc_transformer/src/options/transformer.rs index 7e1da2848fbf..22dd8f912b8a 100644 --- a/crates/oxc_transformer/src/options/transformer.rs +++ b/crates/oxc_transformer/src/options/transformer.rs @@ -86,8 +86,8 @@ impl TransformOptions { ReactOptions::default() }) }; - react_options.development = options.has_plugin("transform-react-jsx-development"); - react_options.jsx_plugin = has_jsx_plugin || has_jsx_development_plugin; + react_options.development = has_jsx_development_plugin; + react_options.jsx_plugin = has_jsx_plugin; react_options.display_name_plugin = options.has_plugin("transform-react-display-name"); react_options.jsx_self_plugin = options.has_plugin("transform-react-jsx-self"); react_options.jsx_source_plugin = options.has_plugin("transform-react-jsx-source"); diff --git a/crates/oxc_transformer/src/react/jsx/mod.rs b/crates/oxc_transformer/src/react/jsx/mod.rs index 6ea703a9dbdd..3cacfd0788fb 100644 --- a/crates/oxc_transformer/src/react/jsx/mod.rs +++ b/crates/oxc_transformer/src/react/jsx/mod.rs @@ -36,7 +36,7 @@ pub use super::{ /// * /// * pub struct ReactJsx<'a> { - options: Rc, + options: ReactOptions, ctx: Ctx<'a>, @@ -310,7 +310,7 @@ impl<'a> Pragma<'a> { // Transforms impl<'a> ReactJsx<'a> { - pub fn new(options: Rc, ctx: Ctx<'a>) -> Self { + pub fn new(options: ReactOptions, ctx: Ctx<'a>) -> Self { let bindings = match options.runtime { ReactJsxRuntime::Classic => { if options.import_source.is_some() { @@ -612,9 +612,7 @@ impl<'a> ReactJsx<'a> { // React.createElement's second argument if !is_fragment && is_classic { - if self.options.is_jsx_self_plugin_enabled() - && self.jsx_self.can_add_self_attribute(ctx) - { + if self.options.jsx_self_plugin && self.jsx_self.can_add_self_attribute(ctx) { if let Some(span) = self_attr_span { self.jsx_self.report_error(span); } else { @@ -622,7 +620,7 @@ impl<'a> ReactJsx<'a> { } } - if self.options.is_jsx_source_plugin_enabled() { + if self.options.jsx_source_plugin { if let Some(span) = source_attr_span { self.jsx_source.report_error(span); } else { @@ -678,7 +676,7 @@ impl<'a> ReactJsx<'a> { // Fragment doesn't have source and self if !is_fragment { // { __source: { fileName, lineNumber, columnNumber } } - if self.options.is_jsx_source_plugin_enabled() { + if self.options.jsx_source_plugin { if let Some(span) = source_attr_span { self.jsx_source.report_error(span); } else { @@ -690,9 +688,7 @@ impl<'a> ReactJsx<'a> { } // this - if self.options.is_jsx_self_plugin_enabled() - && self.jsx_self.can_add_self_attribute(ctx) - { + if self.options.jsx_self_plugin && self.jsx_self.can_add_self_attribute(ctx) { if let Some(span) = self_attr_span { self.jsx_self.report_error(span); } else { diff --git a/crates/oxc_transformer/src/react/mod.rs b/crates/oxc_transformer/src/react/mod.rs index 3c3ff6324db5..4df990f1ac4f 100644 --- a/crates/oxc_transformer/src/react/mod.rs +++ b/crates/oxc_transformer/src/react/mod.rs @@ -27,23 +27,35 @@ pub use self::{ /// * [plugin-transform-react-jsx-source](https://babel.dev/docs/babel-plugin-transform-react-jsx-source) /// * [plugin-transform-react-display-name](https://babeljs.io/docs/babel-plugin-transform-react-display-name) pub struct React<'a> { - options: Rc, jsx: ReactJsx<'a>, display_name: ReactDisplayName<'a>, + jsx_plugin: bool, + display_name_plugin: bool, + jsx_self_plugin: bool, + jsx_source_plugin: bool, } // Constructors impl<'a> React<'a> { - pub fn new(options: ReactOptions, ctx: Ctx<'a>) -> Self { - let mut options = options; - if options.is_jsx_plugin_enabled() { + pub fn new(mut options: ReactOptions, ctx: Ctx<'a>) -> Self { + if options.jsx_plugin || options.development { options.update_with_comments(&ctx); + options.conform(); } - let options = Rc::new(options); + let ReactOptions { + jsx_plugin, + display_name_plugin, + jsx_self_plugin, + jsx_source_plugin, + .. + } = options; Self { - options: Rc::clone(&options), jsx: ReactJsx::new(options, Rc::clone(&ctx)), display_name: ReactDisplayName::new(ctx), + jsx_plugin, + display_name_plugin, + jsx_self_plugin, + jsx_source_plugin, } } } @@ -51,24 +63,22 @@ impl<'a> React<'a> { // Transforms impl<'a> React<'a> { pub fn transform_program_on_exit(&mut self, program: &mut Program<'a>) { - if self.options.is_jsx_plugin_enabled() { + if self.jsx_plugin { self.jsx.transform_program_on_exit(program); } } pub fn transform_expression(&mut self, expr: &mut Expression<'a>, ctx: &mut TraverseCtx<'a>) { - match expr { - Expression::JSXElement(e) => { - if self.options.is_jsx_plugin_enabled() { + if self.jsx_plugin { + match expr { + Expression::JSXElement(e) => { *expr = self.jsx.transform_jsx_element(e, ctx); } - } - Expression::JSXFragment(e) => { - if self.options.is_jsx_plugin_enabled() { + Expression::JSXFragment(e) => { *expr = self.jsx.transform_jsx_fragment(e, ctx); } + _ => {} } - _ => {} } } @@ -77,7 +87,7 @@ impl<'a> React<'a> { call_expr: &mut CallExpression<'a>, ctx: &TraverseCtx<'a>, ) { - if self.options.display_name_plugin { + if self.display_name_plugin { self.display_name.transform_call_expression(call_expr, ctx); } } @@ -87,12 +97,10 @@ impl<'a> React<'a> { elem: &mut JSXOpeningElement<'a>, ctx: &TraverseCtx<'a>, ) { - if self.options.is_jsx_self_plugin_enabled() - && self.jsx.jsx_self.can_add_self_attribute(ctx) - { + if self.jsx_self_plugin && 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() { + if self.jsx_source_plugin { self.jsx.jsx_source.transform_jsx_opening_element(elem); } } diff --git a/crates/oxc_transformer/src/react/options.rs b/crates/oxc_transformer/src/react/options.rs index 9a43969e96e4..3f184dde311e 100644 --- a/crates/oxc_transformer/src/react/options.rs +++ b/crates/oxc_transformer/src/react/options.rs @@ -127,16 +127,12 @@ impl Default for ReactOptions { } impl ReactOptions { - pub fn is_jsx_plugin_enabled(&self) -> bool { - self.jsx_plugin || self.development - } - - pub fn is_jsx_self_plugin_enabled(&self) -> bool { - self.jsx_self_plugin || self.development - } - - pub fn is_jsx_source_plugin_enabled(&self) -> bool { - self.jsx_source_plugin || self.development + pub fn conform(&mut self) { + if self.development { + self.jsx_plugin = true; + self.jsx_self_plugin = true; + self.jsx_source_plugin = true; + } } /// Scan through all comments and find the following pragmas