refactor: load module content an owned value

This is a pre-requisite for WASM imports that additionally allows us to minimize clones when doing things like BOM stripping.
denoland · May 20, 2022 · 024c08e · 024c08e
1 parent cbf7134
commit 024c08e
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 47 deletions.
diff --git a/src/graph.rs b/src/graph.rs
@@ -1177,7 +1177,7 @@ fn resolve(
 pub(crate) fn parse_module(
   specifier: &ModuleSpecifier,
   maybe_headers: Option<&HashMap<String, String>>,
-  content: Arc<String>,
+  content: String,
   maybe_assert_type: Option<&str>,
   maybe_kind: Option<&ModuleKind>,
   maybe_resolver: Option<&dyn Resolver>,
@@ -1187,6 +1187,8 @@ pub(crate) fn parse_module(
 ) -> ModuleSlot {
   let media_type = get_media_type(specifier, maybe_headers);
 
+  let content = content.into();
+
   // here we check any media types that should have assertions made against them
   // if they aren't the root and add them to the graph, otherwise we continue
   if media_type == MediaType::Json
@@ -1565,7 +1567,7 @@ impl<'a> Builder<'a> {
         Some((specifier, kind, Ok(Some(response)))) => {
           let assert_types =
             self.pending_assert_types.remove(&specifier).unwrap();
-          self.visit(&specifier, &kind, &response, &build_kind, assert_types);
+          self.visit(&specifier, &kind, response, &build_kind, assert_types);
           Some(specifier)
         }
         Some((specifier, _, Ok(None))) => {
@@ -1696,7 +1698,7 @@ impl<'a> Builder<'a> {
     &mut self,
     requested_specifier: &ModuleSpecifier,
     kind: &ModuleKind,
-    response: &LoadResponse,
+    response: LoadResponse,
     build_kind: &BuildKind,
     assert_types: HashSet<Option<String>>,
   ) {
@@ -1744,18 +1746,15 @@ impl<'a> Builder<'a> {
             &specifier,
             kind,
             maybe_headers.as_ref(),
-            content.clone(),
+            content,
             build_kind,
             assert_types.into_iter().next().unwrap(),
           )
         };
         (specifier, module_slot)
       }
     };
-    self
-      .graph
-      .module_slots
-      .insert(specifier.clone(), module_slot);
+    self.graph.module_slots.insert(specifier, module_slot);
   }
 
   /// Visit a module, parsing it and resolving any dependencies.
@@ -1764,7 +1763,7 @@ impl<'a> Builder<'a> {
     specifier: &ModuleSpecifier,
     kind: &ModuleKind,
     maybe_headers: Option<&HashMap<String, String>>,
-    content: Arc<String>,
+    content: String,
     build_kind: &BuildKind,
     maybe_assert_type: Option<String>,
   ) -> ModuleSlot {
@@ -2055,7 +2054,7 @@ mod tests {
   fn test_module_dependency_includes() {
     let specifier = ModuleSpecifier::parse("file:///a.ts").unwrap();
     let source_parser = ast::DefaultSourceParser::default();
-    let content = Arc::new(r#"import * as b from "./b.ts";"#.to_string());
+    let content = r#"import * as b from "./b.ts";"#.to_string();
     let slot = parse_module(
       &specifier,
       None,
@@ -2144,7 +2143,7 @@ mod tests {
               Ok(Some(LoadResponse::Module {
                 specifier: specifier.clone(),
                 maybe_headers: None,
-                content: Arc::new("await import('file:///bar.js')".to_string()),
+                content: "await import('file:///bar.js')".to_string(),
               }))
             })
           }
@@ -2155,7 +2154,7 @@ mod tests {
               Ok(Some(LoadResponse::Module {
                 specifier: specifier.clone(),
                 maybe_headers: None,
-                content: Arc::new("import 'file:///baz.js'".to_string()),
+                content: "import 'file:///baz.js'".to_string(),
               }))
             })
           }
@@ -2166,7 +2165,7 @@ mod tests {
               Ok(Some(LoadResponse::Module {
                 specifier: specifier.clone(),
                 maybe_headers: None,
-                content: Arc::new("console.log('Hello, world!')".to_string()),
+                content: "console.log('Hello, world!')".to_string(),
               }))
             })
           }
@@ -2210,7 +2209,7 @@ mod tests {
             Ok(Some(LoadResponse::Module {
               specifier: specifier.clone(),
               maybe_headers: None,
-              content: Arc::new("await import('file:///bar.js')".to_string()),
+              content: "await import('file:///bar.js')".to_string(),
             }))
           }),
           "file:///bar.js" => Box::pin(async move { Ok(None) }),
@@ -2270,14 +2269,14 @@ mod tests {
             Ok(Some(LoadResponse::Module {
               specifier: Url::parse("file:///foo_actual.js").unwrap(),
               maybe_headers: None,
-              content: Arc::new("import 'file:///bar.js'".to_string()),
+              content: "import 'file:///bar.js'".to_string(),
             }))
           }),
           "file:///bar.js" => Box::pin(async move {
             Ok(Some(LoadResponse::Module {
               specifier: Url::parse("file:///bar_actual.js").unwrap(),
               maybe_headers: None,
-              content: Arc::new("(".to_string()),
+              content: "(".to_string(),
             }))
           }),
           _ => unreachable!(),

diff --git a/src/lib.rs b/src/lib.rs
@@ -25,7 +25,6 @@ use source::Resolver;
 use std::cell::RefCell;
 use std::collections::HashMap;
 use std::rc::Rc;
-use std::sync::Arc;
 
 cfg_if! {
   if #[cfg(feature = "rust")] {
@@ -149,7 +148,7 @@ cfg_if! {
     pub fn parse_module(
       specifier: &ModuleSpecifier,
       maybe_headers: Option<&HashMap<String, String>>,
-      content: Arc<String>,
+      content: String,
       maybe_kind: Option<&ModuleKind>,
       maybe_resolver: Option<&dyn Resolver>,
       maybe_parser: Option<&dyn SourceParser>,
@@ -294,7 +293,7 @@ cfg_if! {
       match graph::parse_module(
         &specifier,
         maybe_headers.as_ref(),
-        Arc::new(content),
+        content,
         None,
         maybe_kind.as_ref(),
         maybe_resolver.as_ref().map(|r| r as &dyn Resolver),
@@ -2817,15 +2816,13 @@ export function a(a) {
     let result = parse_module(
       &specifier,
       None,
-      Arc::new(
-        r#"
+      r#"
     import { a } from "./a.ts";
     import * as b from "./b.ts";
     export { c } from "./c.ts";
     const d = await import("./d.ts");
     "#
-        .to_string(),
-      ),
+      .to_string(),
       None,
       None,
       None,
@@ -2843,13 +2840,11 @@ export function a(a) {
     let result = parse_module(
       &specifier,
       None,
-      Arc::new(
-        r#"
+      r#"
     import a from "./a.json" assert { type: "json" };
     await import("./b.json", { assert: { type: "json" } });
     "#
-        .to_string(),
-      ),
+      .to_string(),
       Some(&ModuleKind::Esm),
       None,
       None,
@@ -2910,16 +2905,14 @@ export function a(a) {
     let result = parse_module(
       &specifier,
       None,
-      Arc::new(
-        r#"
+      r#"
     /** @jsxImportSource https://example.com/preact */
 
     export function A() {
       return <div>Hello Deno</div>;
     }
     "#
-        .to_string(),
-      ),
+      .to_string(),
       Some(&ModuleKind::Esm),
       None,
       None,
@@ -2956,12 +2949,10 @@ export function a(a) {
     let result = parse_module(
       &specifier,
       maybe_headers,
-      Arc::new(
-        r#"declare interface A {
+      r#"declare interface A {
   a: string;
 }"#
-          .to_string(),
-      ),
+        .to_string(),
       Some(&ModuleKind::Esm),
       None,
       None,
@@ -2975,8 +2966,7 @@ export function a(a) {
     let result = parse_module(
       &specifier,
       None,
-      Arc::new(
-        r#"
+      r#"
 /**
  * Some js doc
  *
@@ -2987,8 +2977,7 @@ export function a(a) {
   return;
 }
 "#
-        .to_string(),
-      ),
+      .to_string(),
       Some(&ModuleKind::Esm),
       None,
       None,
@@ -3046,8 +3035,7 @@ export function a(a) {
     let result = parse_module(
       &specifier,
       None,
-      Arc::new(
-        r#"
+      r#"
 /**
  * Some js doc
  *
@@ -3058,8 +3046,7 @@ export function a(a: A): B {
   return;
 }
 "#
-        .to_string(),
-      ),
+      .to_string(),
       Some(&ModuleKind::Esm),
       None,
       None,

diff --git a/src/source.rs b/src/source.rs
@@ -19,7 +19,6 @@ use std::collections::HashMap;
 use std::fmt;
 use std::path::PathBuf;
 use std::pin::Pin;
-use std::sync::Arc;
 
 pub static DEFAULT_JSX_IMPORT_SOURCE_MODULE: &str = "jsx-runtime";
 
@@ -56,7 +55,7 @@ pub enum LoadResponse {
   /// A loaded module.
   Module {
     /// The content of the remote module.
-    content: Arc<String>,
+    content: String,
     /// The final specifier of the module.
     specifier: ModuleSpecifier,
     /// If the module is a remote module, the headers should be returned as a
@@ -213,7 +212,7 @@ pub fn load_data_url(
   Ok(Some(LoadResponse::Module {
     specifier: specifier.clone(),
     maybe_headers: Some(headers),
-    content: Arc::new(content),
+    content,
   }))
 }
 
@@ -265,7 +264,7 @@ impl MemoryLoader {
                   })
                   .collect()
               }),
-              content: Arc::new(content.as_ref().to_string()),
+              content: content.as_ref().to_string(),
             }),
             Source::BuiltIn(specifier) => Ok(LoadResponse::BuiltIn {
               specifier: ModuleSpecifier::parse(specifier.as_ref()).unwrap(),