From f91ec60737dddf1092d0491a0cfe8ce96a7f9c70 Mon Sep 17 00:00:00 2001 From: Igor Unanua Date: Mon, 13 Jan 2025 11:16:47 +0100 Subject: [PATCH] Avoid clonning input js code if status is notmodified (#112) * Avoid clonning input code if status is notmodified * return original code if notmodified * remove TransformOutputWithStatus struct * clean up * clean up * clean up chain_source_maps method * Fix conflict --- main.js | 9 ++- src/lib_wasm.rs | 8 ++- src/rewriter.rs | 104 +++++++++++++--------------- src/tests/binary_expression_test.rs | 11 +-- src/tests/mod.rs | 12 ++++ src/tests/source_map_test.rs | 24 +++++-- src/tests/string_method_test.rs | 47 +++++++++---- 7 files changed, 132 insertions(+), 83 deletions(-) diff --git a/main.js b/main.js index 8fe02f3..1b255d1 100644 --- a/main.js +++ b/main.js @@ -31,7 +31,14 @@ class NonCacheRewriter { } rewrite (code, file) { - return this.nativeRewriter.rewrite(code, file) + const response = this.nativeRewriter.rewrite(code, file) + + // rewrite returns an empty content when for the 'notmodified' status + if (response?.metrics?.status === 'notmodified') { + response.content = code + } + + return response } csiMethods () { diff --git a/src/lib_wasm.rs b/src/lib_wasm.rs index 496dfec..9b3f0b5 100644 --- a/src/lib_wasm.rs +++ b/src/lib_wasm.rs @@ -184,7 +184,13 @@ impl Rewriter { rewrite_js(code, &file, &self.config, &source_map_reader) .map(|result| Result { - content: print_js(&result, &self.config), + content: print_js( + &result.code, + &result.source_map, + &result.original_source_map, + &self.config, + ) + .into_owned(), metrics: get_metrics(result.transform_status, &file), literals_result: result.literals_result, }) diff --git a/src/rewriter.rs b/src/rewriter.rs index 8abc566..c4d6d48 100644 --- a/src/rewriter.rs +++ b/src/rewriter.rs @@ -16,6 +16,7 @@ use anyhow::{Error, Result}; use base64::{engine::general_purpose::STANDARD, Engine as _}; use log::debug; use std::{ + borrow::Cow, collections::HashMap, io::Read, path::{Path, PathBuf}, @@ -25,7 +26,7 @@ use std::{ use swc::{ config::{IsModule, SourceMapsConfig}, sourcemap::{decode, decode_data_url, DecodedMap, SourceMap, SourceMapBuilder}, - try_with_handler, Compiler, HandlerOpts, PrintArgs, SwcComments, TransformOutput, + try_with_handler, Compiler, HandlerOpts, PrintArgs, SwcComments, }; use swc_common::{ comments::Comments, @@ -53,12 +54,6 @@ pub struct OriginalSourceMap { pub source_map_comment: Option, } -pub struct TransformOutputWithStatus { - pub output: TransformOutput, - pub status: TransformStatus, - pub literals_result: Option, -} - pub struct Config { pub chain_source_map: bool, pub print_comments: bool, @@ -99,42 +94,30 @@ pub fn rewrite_js( .cm .new_source_file(Arc::new(FileName::Real(PathBuf::from(file))), code); - let program = parse_js(&source_file, handler, &compiler)?; - - // extract sourcemap before printing otherwise comments are consumed - // and looks like it is not possible to read them after compiler.print() invocation - let original_map = extract_source_map(file, compiler.comments(), file_reader); - - let result = transform_js(program, &source_file, file, config, &compiler); - - result.map(|transformed| RewrittenOutput { - code: transformed.output.code, - source_map: transformed.output.map.unwrap_or_default(), - original_source_map: original_map, - transform_status: Some(transformed.status), - literals_result: transformed.literals_result, - }) + parse_js(&source_file, handler, &compiler) + .and_then(|program| transform_js(program, file, file_reader, config, &compiler)) }) } -pub fn print_js(output: &RewrittenOutput, config: &Config) -> String { - let mut final_source_map: String = String::from(&output.source_map); - let original_source_map = &output.original_source_map; - if config.chain_source_map { - final_source_map = chain_source_maps(&output.source_map, &original_source_map.source) - .unwrap_or_else(|_| String::from(&output.source_map)); - } +pub fn print_js<'a>( + code: &'a str, + source_map: &str, + original_source_map: &OriginalSourceMap, + config: &Config, +) -> Cow<'a, str> { + let final_source_map = chain_source_maps(source_map, &original_source_map.source, config) + .unwrap_or_else(|| String::from(source_map)); let final_code = if config.print_comments { match &original_source_map.source_map_comment { Some(comment) => { debug!("Replacing original sourceMappingUrl comment: {comment}"); - output.code.replace(comment.as_str(), "") + code.replace(comment.as_str(), "").into() } - _ => output.code.clone(), + _ => code.into(), } } else { - output.code.clone() + code.into() }; if final_source_map.is_empty() { @@ -149,6 +132,7 @@ pub fn print_js(output: &RewrittenOutput, config: &Config) -> String { SOURCE_MAP_URL, STANDARD.encode(final_source_map) ) + .into() } } @@ -187,13 +171,13 @@ fn parse_js( ) } -fn transform_js( +fn transform_js( mut program: Program, - source_file: &SourceFile, file: &str, + file_reader: &impl FileReader, config: &Config, compiler: &Compiler, -) -> Result { +) -> Result { let mut transform_status = TransformStatus::not_modified(config); let mut block_transform_visitor = BlockTransformVisitor::default(&mut transform_status, config); @@ -212,22 +196,29 @@ fn transform_js( match transform_status.status { Status::Modified => { + // extract sourcemap before printing otherwise comments are consumed + // and looks like it is not possible to read them after compiler.print() invocation + let original_source_map = extract_source_map(file, compiler.comments(), file_reader); + compiler .print(&program, print_args) - .map(|output| TransformOutputWithStatus { - output, - status: transform_status, + .map(|output| RewrittenOutput { + code: output.code, + source_map: output.map.unwrap_or_default(), + original_source_map, + transform_status: Some(transform_status), literals_result, }) } - Status::NotModified => Ok(TransformOutputWithStatus { - output: TransformOutput { - code: source_file.src.to_string(), - map: None, - output: None, + Status::NotModified => Ok(RewrittenOutput { + code: String::default(), + source_map: String::default(), + original_source_map: OriginalSourceMap { + source: None, + source_map_comment: None, }, - status: transform_status, + transform_status: Some(transform_status), literals_result, }), @@ -242,14 +233,15 @@ fn transform_js( } fn chain_source_maps( - source_map: &String, + source_map: &str, original_map: &Option, -) -> Result { - debug!("Chaining sourcemaps"); + config: &Config, +) -> Option { + config.chain_source_map.then(|| { + debug!("Chaining sourcemaps"); - if let Some(new_source) = parse_source_map(Some(source_map.as_str())) { - match original_map { - Some(original_source) => { + original_map.as_ref().and_then(|original_source| { + parse_source_map(Some(source_map)).and_then(|new_source| { let mut builder = SourceMapBuilder::new(None); let mut sources: HashMap = HashMap::new(); let mut names: HashMap = HashMap::new(); @@ -286,6 +278,7 @@ fn chain_source_maps( ); } } + let mut source_map_output: Vec = vec![]; builder .into_sourcemap() @@ -296,14 +289,11 @@ fn chain_source_maps( }) .map_err(|err| { debug!("Error chaining sourcemaps {err:?}"); - Error::new(err) }) - } - None => Result::Ok(String::from(source_map)), - } - } else { - Result::Ok(String::from(source_map)) - } + .ok() + }) + }) + })? } fn extract_source_map( diff --git a/src/tests/binary_expression_test.rs b/src/tests/binary_expression_test.rs index 965fdb1..7e39407 100644 --- a/src/tests/binary_expression_test.rs +++ b/src/tests/binary_expression_test.rs @@ -10,14 +10,17 @@ mod tests { use anyhow::Error; - use crate::{rewriter::debug_js, tests::rewrite_js}; + use crate::{ + rewriter::debug_js, + tests::{assert_not_modified, rewrite_js}, + }; #[test] fn test_simple_plus_literal() -> Result<(), String> { let original_code = "{const result = 'a' + 'b'}".to_string(); let js_file = "test.js".to_string(); let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?; - assert_that(&rewritten.code).contains("const result = 'a' + 'b'"); + assert_not_modified(&rewritten); Ok(()) } @@ -26,7 +29,7 @@ mod tests { let original_code = "{const result = 'a' + 'b' + 'c'}".to_string(); let js_file = "test.js".to_string(); let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?; - assert_that(&rewritten.code).contains("const result = 'a' + 'b' + 'c'"); + assert_not_modified(&rewritten); Ok(()) } @@ -45,7 +48,7 @@ mod tests { let original_code = "{const result = 1 + 2}".to_string(); let js_file = "test.js".to_string(); let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?; - assert_that(&rewritten.code).contains("const result = 1 + 2"); + assert_not_modified(&rewritten); Ok(()) } diff --git a/src/tests/mod.rs b/src/tests/mod.rs index ba56d9c..a471062 100644 --- a/src/tests/mod.rs +++ b/src/tests/mod.rs @@ -5,10 +5,12 @@ use crate::{ rewriter::{Config, RewrittenOutput}, telemetry::TelemetryVerbosity, + transform::transform_status::Status, util::DefaultFileReader, visitor::csi_methods::{CsiMethod, CsiMethods}, }; use anyhow::Error; +use speculoos::{assert_that, prelude::BooleanAssertions}; use std::path::PathBuf; mod arrow_func_tests; @@ -145,3 +147,13 @@ fn csi_op_from_str(src: &str, dst: Option<&str>) -> CsiMethod { }; CsiMethod::new(String::from(src), dst_string, true, false) } + +fn assert_not_modified(output: &RewrittenOutput) { + assert_that( + &output + .transform_status + .as_ref() + .is_some_and(|transform_status| transform_status.status == Status::NotModified), + ) + .is_true(); +} diff --git a/src/tests/source_map_test.rs b/src/tests/source_map_test.rs index 7b37efb..4dae488 100644 --- a/src/tests/source_map_test.rs +++ b/src/tests/source_map_test.rs @@ -11,7 +11,7 @@ mod tests { use swc::sourcemap::{decode_data_url, DecodedMap}; use crate::{ - rewriter::{print_js, rewrite_js, RewrittenOutput}, + rewriter::{print_js, rewrite_js, Config, RewrittenOutput}, tests::{ get_chained_and_print_comments_config, get_default_config, get_test_resources_folder, }, @@ -116,10 +116,20 @@ mod tests { } } + fn print(result: &RewrittenOutput, config: &Config) -> String { + print_js( + &result.code, + &result.source_map, + &result.original_source_map, + config, + ) + .into_owned() + } + #[test] fn test_source_maps_unchained_embedded() -> Result<(), String> { let rewritten = get_rewritten_js("StrUtil_embedded.js")?; - let result = print_js(&rewritten, &get_default_config(false)); + let result = print(&rewritten, &get_default_config(false)); assert_that(&result).contains(SOURCE_MAP_URL); check_sourcemap_tokens(result, UNCHAINED_TOKENS.to_vec()); Ok(()) @@ -128,7 +138,7 @@ mod tests { #[test] fn test_source_maps_unchained_external() -> Result<(), String> { let rewritten = get_rewritten_js("StrUtil_external.js")?; - let result = print_js(&rewritten, &get_default_config(false)); + let result = print(&rewritten, &get_default_config(false)); assert_that(&result).contains(SOURCE_MAP_URL); check_sourcemap_tokens(result, UNCHAINED_TOKENS.to_vec()); Ok(()) @@ -137,7 +147,7 @@ mod tests { #[test] fn test_source_maps_unchained_without_original_source_map() -> Result<(), String> { let rewritten = get_rewritten_js("StrUtil_without_sm.js")?; - let result = print_js(&rewritten, &get_default_config(false)); + let result = print(&rewritten, &get_default_config(false)); assert_that(&result).contains(SOURCE_MAP_URL); check_sourcemap_tokens(result, UNCHAINED_TOKENS.to_vec()); Ok(()) @@ -146,7 +156,7 @@ mod tests { #[test] fn test_source_maps_chained_embedded() -> Result<(), String> { let rewritten = get_rewritten_js("StrUtil_embedded.js")?; - let result = print_js(&rewritten, &get_chained_and_print_comments_config()); + let result = print(&rewritten, &get_chained_and_print_comments_config()); assert_that(&result).contains(SOURCE_MAP_URL); check_sourcemap_tokens(result, CHAINED_TOKENS.to_vec()); Ok(()) @@ -155,7 +165,7 @@ mod tests { #[test] fn test_source_maps_chained_external() -> Result<(), String> { let rewritten = get_rewritten_js("StrUtil_external.js")?; - let result = print_js(&rewritten, &get_chained_and_print_comments_config()); + let result = print(&rewritten, &get_chained_and_print_comments_config()); assert_that(&result).contains(SOURCE_MAP_URL); check_sourcemap_tokens(result, CHAINED_TOKENS.to_vec()); Ok(()) @@ -164,7 +174,7 @@ mod tests { #[test] fn test_source_maps_chained_without_original_source_map() -> Result<(), String> { let rewritten = get_rewritten_js("StrUtil_without_sm.js")?; - let result = print_js(&rewritten, &get_chained_and_print_comments_config()); + let result = print(&rewritten, &get_chained_and_print_comments_config()); assert_that(&result).contains(SOURCE_MAP_URL); check_sourcemap_tokens(result, UNCHAINED_TOKENS.to_vec()); Ok(()) diff --git a/src/tests/string_method_test.rs b/src/tests/string_method_test.rs index dd1533a..74be90a 100644 --- a/src/tests/string_method_test.rs +++ b/src/tests/string_method_test.rs @@ -9,8 +9,8 @@ mod tests { use crate::{ rewriter::print_js, tests::{ - csi_from_str, get_chained_and_print_comments_config, get_default_config, rewrite_js, - rewrite_js_with_csi_methods, + assert_not_modified, csi_from_str, get_chained_and_print_comments_config, + get_default_config, rewrite_js, rewrite_js_with_csi_methods, }, visitor::csi_methods::CsiMethods, }; @@ -65,7 +65,7 @@ mod tests { let original_code = "{const a = \"b\".substring(1);}".to_string(); let js_file = "test.js".to_string(); let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?; - assert_that(&rewritten.code).contains("const a = \"b\".substring(1)"); + assert_not_modified(&rewritten); Ok(()) } @@ -74,8 +74,7 @@ mod tests { let original_code = "{const a = String.prototype.substring.call('hello', 2);}".to_string(); let js_file = "test.js".to_string(); let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?; - assert_that(&rewritten.code) - .contains("const a = String.prototype.substring.call('hello', 2)"); + assert_not_modified(&rewritten); Ok(()) } @@ -125,7 +124,7 @@ mod tests { let original_code = "{const a = String.prototype.concat.call(1,2,3)}".to_string(); let js_file = "test.js".to_string(); let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?; - assert_that(&rewritten.code).contains("const a = String.prototype.concat.call(1,2,3)"); + assert_not_modified(&rewritten); Ok(()) } @@ -190,7 +189,7 @@ mod tests { let original_code = "{const a = \"b\".trim();}".to_string(); let js_file = "test.js".to_string(); let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?; - assert_that(&rewritten.code).contains("const a = \"b\".trim();"); + assert_not_modified(&rewritten); Ok(()) } @@ -223,7 +222,7 @@ mod tests { let js_file = "test.js".to_string(); let rewritten = rewrite_js_with_csi_methods(original_code, js_file, &CsiMethods::empty()) .map_err(|e| e.to_string())?; - assert_that(&rewritten.code).contains("const a = b.concat('hello')"); + assert_not_modified(&rewritten); Ok(()) } @@ -232,7 +231,7 @@ mod tests { let original_code = "{const a = b.plusOperator('hello')}".to_string(); let js_file = "test.js".to_string(); let rewritten = rewrite_js(original_code, js_file).map_err(|e| e.to_string())?; - assert_that(&rewritten.code).contains("const a = b.plusOperator('hello')"); + assert_not_modified(&rewritten); Ok(()) } @@ -273,7 +272,15 @@ mod tests { let js_file = "test.js".to_string(); let rewritten = rewrite_js(original_code, js_file) .map_err(|e| e.to_string()) - .map(|rewrite_output| print_js(&rewrite_output, &get_default_config(false)))?; + .map(|result| { + print_js( + &result.code, + &result.source_map, + &result.original_source_map, + &get_default_config(false), + ) + .into_owned() + })?; assert_that(&rewritten).contains("const a = {\n __html: (__datadog_test_2 = (__datadog_test_0 = b, \ __datadog_test_1 = __datadog_test_0.replace, _ddiast.replace(__datadog_test_1.call(__datadog_test_0, /\\/\\*# sourceMappingURL=.*\\*\\//g, ''), \ @@ -292,8 +299,14 @@ mod tests { let js_file = "test.js".to_string(); let rewritten = rewrite_js(original_code, js_file) .map_err(|e| e.to_string()) - .map(|rewrite_output| { - print_js(&rewrite_output, &get_chained_and_print_comments_config()) + .map(|result| { + print_js( + &result.code, + &result.source_map, + &result.original_source_map, + &get_chained_and_print_comments_config(), + ) + .into_owned() })?; assert_that(&rewritten).contains("const a = {\n __html: (__datadog_test_2 = (__datadog_test_0 = b, \ @@ -311,7 +324,15 @@ mod tests { let js_file = "test.js".to_string(); let rewritten = rewrite_js(original_code, js_file) .map_err(|e| e.to_string()) - .map(|rewrite_output| print_js(&rewrite_output, &get_default_config(false)))?; + .map(|result| { + print_js( + &result.code, + &result.source_map, + &result.original_source_map, + &get_default_config(false), + ) + .into_owned() + })?; assert_that(&rewritten).contains("const a = {\n __html: (__datadog_test_2 = (__datadog_test_0 = b, \ __datadog_test_1 = __datadog_test_0.replace, _ddiast.replace(__datadog_test_1.call(__datadog_test_0, /\\/\\*# sourceMappingURL=.*\\*\\//g, ''), \