Skip to content

Commit 6691015

Browse files
lucacasonatolucacasonato
committed
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.
1 parent 89affe4 commit 6691015

File tree

4 files changed

+76
-74
lines changed

4 files changed

+76
-74
lines changed

‎src/graph.rs

+56-43
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ pub enum ModuleGraphError {
133133
actual_media_type: MediaType,
134134
expected_media_type: MediaType,
135135
},
136+
ConflictingAssertions(ModuleSpecifier),
136137
LoadingErr(ModuleSpecifier, Arc<anyhow::Error>),
137138
Missing(ModuleSpecifier),
138139
ParseErr(ModuleSpecifier, deno_ast::Diagnostic),
@@ -163,6 +164,9 @@ impl Clone for ModuleGraphError {
163164
actual_media_type: *actual_media_type,
164165
expected_media_type: *expected_media_type,
165166
},
167+
Self::ConflictingAssertions(specifier) => {
168+
Self::ConflictingAssertions(specifier.clone())
169+
}
166170
Self::UnsupportedImportAssertionType(specifier, kind) => {
167171
Self::UnsupportedImportAssertionType(specifier.clone(), kind.clone())
168172
}
@@ -184,7 +188,8 @@ impl ModuleGraphError {
184188
| Self::InvalidSource(s, _)
185189
| Self::UnsupportedMediaType(s, _)
186190
| Self::UnsupportedImportAssertionType(s, _)
187-
| Self::Missing(s) => s,
191+
| Self::Missing(s)
192+
| Self::ConflictingAssertions(s) => s,
188193
Self::InvalidTypeAssertion { specifier, .. } => specifier,
189194
}
190195
}
@@ -201,6 +206,7 @@ impl fmt::Display for ModuleGraphError {
201206
Self::InvalidSource(specifier, Some(filename)) => write!(f, "The source code is invalid, as it does not match the expected hash in the lock file.\n Specifier: {}\n Lock file: {}", specifier, filename),
202207
Self::InvalidSource(specifier, None) => write!(f, "The source code is invalid, as it does not match the expected hash in the lock file.\n Specifier: {}", specifier),
203208
Self::InvalidTypeAssertion { specifier, actual_media_type, expected_media_type } => write!(f, "Expected a {} module, but identified a {} module.\n Specifier: {}", expected_media_type, actual_media_type, specifier),
209+
Self::ConflictingAssertions(specifier) => write!(f, "Module \"{specifier}\" was imported with conflicting assertions."),
204210
Self::UnsupportedMediaType(specifier, MediaType::Json) => write!(f, "Expected a JavaScript or TypeScript module, but identified a Json module. Consider importing Json modules with an import assertion with the type of \"json\".\n Specifier: {}", specifier),
205211
Self::UnsupportedMediaType(specifier, media_type) => write!(f, "Expected a JavaScript or TypeScript module, but identified a {} module. Importing these types of modules is currently not supported.\n Specifier: {}", media_type, specifier),
206212
Self::UnsupportedImportAssertionType(_, kind) => write!(f, "The import assertion type of \"{}\" is unsupported.", kind),
@@ -1171,7 +1177,7 @@ fn resolve(
11711177
pub(crate) fn parse_module(
11721178
specifier: &ModuleSpecifier,
11731179
maybe_headers: Option<&HashMap<String, String>>,
1174-
content: Arc<String>,
1180+
content: String,
11751181
maybe_assert_type: Option<&str>,
11761182
maybe_kind: Option<&ModuleKind>,
11771183
maybe_resolver: Option<&dyn Resolver>,
@@ -1181,6 +1187,8 @@ pub(crate) fn parse_module(
11811187
) -> ModuleSlot {
11821188
let media_type = get_media_type(specifier, maybe_headers);
11831189

1190+
let content = content.into();
1191+
11841192
// here we check any media types that should have assertions made against them
11851193
// if they aren't the root and add them to the graph, otherwise we continue
11861194
if media_type == MediaType::Json
@@ -1559,15 +1567,7 @@ impl<'a> Builder<'a> {
15591567
Some((specifier, kind, Ok(Some(response)))) => {
15601568
let assert_types =
15611569
self.pending_assert_types.remove(&specifier).unwrap();
1562-
for maybe_assert_type in assert_types {
1563-
self.visit(
1564-
&specifier,
1565-
&kind,
1566-
&response,
1567-
&build_kind,
1568-
maybe_assert_type,
1569-
)
1570-
}
1570+
self.visit(&specifier, &kind, response, &build_kind, assert_types);
15711571
Some(specifier)
15721572
}
15731573
Some((specifier, _, Ok(None))) => {
@@ -1698,50 +1698,63 @@ impl<'a> Builder<'a> {
16981698
&mut self,
16991699
requested_specifier: &ModuleSpecifier,
17001700
kind: &ModuleKind,
1701-
response: &LoadResponse,
1701+
response: LoadResponse,
17021702
build_kind: &BuildKind,
1703-
maybe_assert_type: Option<String>,
1703+
assert_types: HashSet<Option<String>>,
17041704
) {
17051705
let (specifier, module_slot) = match response {
17061706
LoadResponse::BuiltIn { specifier } => {
1707-
self.check_specifier(requested_specifier, specifier);
1708-
let module_slot = ModuleSlot::Module(Module::new_without_source(
1709-
specifier.clone(),
1710-
ModuleKind::BuiltIn,
1711-
));
1707+
self.check_specifier(requested_specifier, &specifier);
1708+
let module_slot = if assert_types.len() != 1 {
1709+
ModuleSlot::Err(ModuleGraphError::ConflictingAssertions(
1710+
specifier.clone(),
1711+
))
1712+
} else {
1713+
ModuleSlot::Module(Module::new_without_source(
1714+
specifier.clone(),
1715+
ModuleKind::BuiltIn,
1716+
))
1717+
};
17121718
(specifier, module_slot)
17131719
}
17141720
LoadResponse::External { specifier } => {
1715-
self.check_specifier(requested_specifier, specifier);
1716-
let module_slot = ModuleSlot::Module(Module::new_without_source(
1717-
specifier.clone(),
1718-
ModuleKind::External,
1719-
));
1721+
self.check_specifier(requested_specifier, &specifier);
1722+
let module_slot = if assert_types.len() != 1 {
1723+
ModuleSlot::Err(ModuleGraphError::ConflictingAssertions(
1724+
specifier.clone(),
1725+
))
1726+
} else {
1727+
ModuleSlot::Module(Module::new_without_source(
1728+
specifier.clone(),
1729+
ModuleKind::External,
1730+
))
1731+
};
17201732
(specifier, module_slot)
17211733
}
17221734
LoadResponse::Module {
17231735
specifier,
17241736
content,
17251737
maybe_headers,
17261738
} => {
1727-
self.check_specifier(requested_specifier, specifier);
1728-
(
1729-
specifier,
1739+
self.check_specifier(requested_specifier, &specifier);
1740+
let module_slot = if assert_types.len() != 1 {
1741+
ModuleSlot::Err(ModuleGraphError::ConflictingAssertions(
1742+
specifier.clone(),
1743+
))
1744+
} else {
17301745
self.visit_module(
1731-
specifier,
1746+
&specifier,
17321747
kind,
17331748
maybe_headers.as_ref(),
1734-
content.clone(),
1749+
content,
17351750
build_kind,
1736-
maybe_assert_type,
1737-
),
1738-
)
1751+
assert_types.into_iter().next().unwrap(),
1752+
)
1753+
};
1754+
(specifier, module_slot)
17391755
}
17401756
};
1741-
self
1742-
.graph
1743-
.module_slots
1744-
.insert(specifier.clone(), module_slot);
1757+
self.graph.module_slots.insert(specifier, module_slot);
17451758
}
17461759

17471760
/// Visit a module, parsing it and resolving any dependencies.
@@ -1750,7 +1763,7 @@ impl<'a> Builder<'a> {
17501763
specifier: &ModuleSpecifier,
17511764
kind: &ModuleKind,
17521765
maybe_headers: Option<&HashMap<String, String>>,
1753-
content: Arc<String>,
1766+
content: String,
17541767
build_kind: &BuildKind,
17551768
maybe_assert_type: Option<String>,
17561769
) -> ModuleSlot {
@@ -2041,7 +2054,7 @@ mod tests {
20412054
fn test_module_dependency_includes() {
20422055
let specifier = ModuleSpecifier::parse("file:///a.ts").unwrap();
20432056
let source_parser = ast::DefaultSourceParser::default();
2044-
let content = Arc::new(r#"import * as b from "./b.ts";"#.to_string());
2057+
let content = r#"import * as b from "./b.ts";"#.to_string();
20452058
let slot = parse_module(
20462059
&specifier,
20472060
None,
@@ -2130,7 +2143,7 @@ mod tests {
21302143
Ok(Some(LoadResponse::Module {
21312144
specifier: specifier.clone(),
21322145
maybe_headers: None,
2133-
content: Arc::new("await import('file:///bar.js')".to_string()),
2146+
content: "await import('file:///bar.js')".to_string(),
21342147
}))
21352148
})
21362149
}
@@ -2141,7 +2154,7 @@ mod tests {
21412154
Ok(Some(LoadResponse::Module {
21422155
specifier: specifier.clone(),
21432156
maybe_headers: None,
2144-
content: Arc::new("import 'file:///baz.js'".to_string()),
2157+
content: "import 'file:///baz.js'".to_string(),
21452158
}))
21462159
})
21472160
}
@@ -2152,7 +2165,7 @@ mod tests {
21522165
Ok(Some(LoadResponse::Module {
21532166
specifier: specifier.clone(),
21542167
maybe_headers: None,
2155-
content: Arc::new("console.log('Hello, world!')".to_string()),
2168+
content: "console.log('Hello, world!')".to_string(),
21562169
}))
21572170
})
21582171
}
@@ -2196,7 +2209,7 @@ mod tests {
21962209
Ok(Some(LoadResponse::Module {
21972210
specifier: specifier.clone(),
21982211
maybe_headers: None,
2199-
content: Arc::new("await import('file:///bar.js')".to_string()),
2212+
content: "await import('file:///bar.js')".to_string(),
22002213
}))
22012214
}),
22022215
"file:///bar.js" => Box::pin(async move { Ok(None) }),
@@ -2256,14 +2269,14 @@ mod tests {
22562269
Ok(Some(LoadResponse::Module {
22572270
specifier: Url::parse("file:///foo_actual.js").unwrap(),
22582271
maybe_headers: None,
2259-
content: Arc::new("import 'file:///bar.js'".to_string()),
2272+
content: "import 'file:///bar.js'".to_string(),
22602273
}))
22612274
}),
22622275
"file:///bar.js" => Box::pin(async move {
22632276
Ok(Some(LoadResponse::Module {
22642277
specifier: Url::parse("file:///bar_actual.js").unwrap(),
22652278
maybe_headers: None,
2266-
content: Arc::new("(".to_string()),
2279+
content: "(".to_string(),
22672280
}))
22682281
}),
22692282
_ => unreachable!(),

‎src/info.rs

+3
Original file line numberDiff line numberDiff line change
@@ -249,6 +249,9 @@ impl ModuleGraphError {
249249
Self::InvalidTypeAssertion { .. } => {
250250
fmt_error_msg(f, prefix, last, specifier, "(invalid import assertion)")
251251
}
252+
Self::ConflictingAssertions { .. } => {
253+
fmt_error_msg(f, prefix, last, specifier, "(conflicting assertions)")
254+
}
252255
Self::LoadingErr(_, _) => {
253256
fmt_error_msg(f, prefix, last, specifier, "(loading error)")
254257
}

‎src/lib.rs

+14-27
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,6 @@ use source::Resolver;
2525
use std::cell::RefCell;
2626
use std::collections::HashMap;
2727
use std::rc::Rc;
28-
use std::sync::Arc;
2928

3029
cfg_if! {
3130
if #[cfg(feature = "rust")] {
@@ -149,7 +148,7 @@ cfg_if! {
149148
pub fn parse_module(
150149
specifier: &ModuleSpecifier,
151150
maybe_headers: Option<&HashMap<String, String>>,
152-
content: Arc<String>,
151+
content: String,
153152
maybe_kind: Option<&ModuleKind>,
154153
maybe_resolver: Option<&dyn Resolver>,
155154
maybe_parser: Option<&dyn SourceParser>,
@@ -294,7 +293,7 @@ cfg_if! {
294293
match graph::parse_module(
295294
&specifier,
296295
maybe_headers.as_ref(),
297-
Arc::new(content),
296+
content,
298297
None,
299298
maybe_kind.as_ref(),
300299
maybe_resolver.as_ref().map(|r| r as &dyn Resolver),
@@ -2817,15 +2816,13 @@ export function a(a) {
28172816
let result = parse_module(
28182817
&specifier,
28192818
None,
2820-
Arc::new(
2821-
r#"
2819+
r#"
28222820
import { a } from "./a.ts";
28232821
import * as b from "./b.ts";
28242822
export { c } from "./c.ts";
28252823
const d = await import("./d.ts");
28262824
"#
2827-
.to_string(),
2828-
),
2825+
.to_string(),
28292826
None,
28302827
None,
28312828
None,
@@ -2843,13 +2840,11 @@ export function a(a) {
28432840
let result = parse_module(
28442841
&specifier,
28452842
None,
2846-
Arc::new(
2847-
r#"
2843+
r#"
28482844
import a from "./a.json" assert { type: "json" };
28492845
await import("./b.json", { assert: { type: "json" } });
28502846
"#
2851-
.to_string(),
2852-
),
2847+
.to_string(),
28532848
Some(&ModuleKind::Esm),
28542849
None,
28552850
None,
@@ -2910,16 +2905,14 @@ export function a(a) {
29102905
let result = parse_module(
29112906
&specifier,
29122907
None,
2913-
Arc::new(
2914-
r#"
2908+
r#"
29152909
/** @jsxImportSource https://example.com/preact */
29162910
29172911
export function A() {
29182912
return <div>Hello Deno</div>;
29192913
}
29202914
"#
2921-
.to_string(),
2922-
),
2915+
.to_string(),
29232916
Some(&ModuleKind::Esm),
29242917
None,
29252918
None,
@@ -2956,12 +2949,10 @@ export function a(a) {
29562949
let result = parse_module(
29572950
&specifier,
29582951
maybe_headers,
2959-
Arc::new(
2960-
r#"declare interface A {
2952+
r#"declare interface A {
29612953
a: string;
29622954
}"#
2963-
.to_string(),
2964-
),
2955+
.to_string(),
29652956
Some(&ModuleKind::Esm),
29662957
None,
29672958
None,
@@ -2975,8 +2966,7 @@ export function a(a) {
29752966
let result = parse_module(
29762967
&specifier,
29772968
None,
2978-
Arc::new(
2979-
r#"
2969+
r#"
29802970
/**
29812971
* Some js doc
29822972
*
@@ -2987,8 +2977,7 @@ export function a(a) {
29872977
return;
29882978
}
29892979
"#
2990-
.to_string(),
2991-
),
2980+
.to_string(),
29922981
Some(&ModuleKind::Esm),
29932982
None,
29942983
None,
@@ -3046,8 +3035,7 @@ export function a(a) {
30463035
let result = parse_module(
30473036
&specifier,
30483037
None,
3049-
Arc::new(
3050-
r#"
3038+
r#"
30513039
/**
30523040
* Some js doc
30533041
*
@@ -3058,8 +3046,7 @@ export function a(a: A): B {
30583046
return;
30593047
}
30603048
"#
3061-
.to_string(),
3062-
),
3049+
.to_string(),
30633050
Some(&ModuleKind::Esm),
30643051
None,
30653052
None,

0 commit comments

Comments
 (0)