Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

refactor: load module content an owned value #154

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

refactor: load module content an owned value #154

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
cbf7134
fix: error on conflicting assertions
lucacasonato May 20, 2022
024c08e
refactor: load module content an owned value
lucacasonato May 20, 2022
69643d5
Merge branch 'main' into load_owned_value
bartlomieju May 30, 2022
6b6c5d7
lint
bartlomieju May 30, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
101 changes: 57 additions & 44 deletions src/graph.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,7 @@ pub enum ModuleGraphError {
actual_media_type: MediaType,
expected_media_type: MediaType,
},
ConflictingAssertions(ModuleSpecifier),
LoadingErr(ModuleSpecifier, Arc<anyhow::Error>),
Missing(ModuleSpecifier),
ParseErr(ModuleSpecifier, deno_ast::Diagnostic),
Expand Down Expand Up @@ -162,6 +163,9 @@ impl Clone for ModuleGraphError {
actual_media_type: *actual_media_type,
expected_media_type: *expected_media_type,
},
Self::ConflictingAssertions(specifier) => {
Self::ConflictingAssertions(specifier.clone())
}
Self::UnsupportedImportAssertionType(specifier, kind) => {
Self::UnsupportedImportAssertionType(specifier.clone(), kind.clone())
}
Expand All @@ -183,7 +187,8 @@ impl ModuleGraphError {
| Self::InvalidSource(s, _)
| Self::UnsupportedMediaType(s, _)
| Self::UnsupportedImportAssertionType(s, _)
| Self::Missing(s) => s,
| Self::Missing(s)
| Self::ConflictingAssertions(s) => s,
Self::InvalidTypeAssertion { specifier, .. } => specifier,
}
}
Expand All @@ -200,6 +205,7 @@ impl fmt::Display for ModuleGraphError {
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),
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),
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),
Self::ConflictingAssertions(specifier) => write!(f, "Module \"{specifier}\" was imported with conflicting assertions."),
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),
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),
Self::UnsupportedImportAssertionType(_, kind) => write!(f, "The import assertion type of \"{}\" is unsupported.", kind),
Expand Down Expand Up @@ -1168,7 +1174,7 @@ fn resolve(
pub(crate) fn parse_module(
specifier: &ModuleSpecifier,
maybe_headers: Option<&HashMap<String, String>>,
content: Arc<str>,
content: String,
maybe_assert_type: Option<&str>,
maybe_kind: Option<&ModuleKind>,
maybe_resolver: Option<&dyn Resolver>,
Expand All @@ -1178,6 +1184,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
Expand Down Expand Up @@ -1559,15 +1567,7 @@ impl<'a> Builder<'a> {
Some((specifier, kind, Ok(Some(response)))) => {
let assert_types =
self.pending_assert_types.remove(&specifier).unwrap();
for maybe_assert_type in assert_types {
self.visit(
&specifier,
&kind,
&response,
&build_kind,
maybe_assert_type,
)
}
self.visit(&specifier, &kind, response, &build_kind, assert_types);
Some(specifier)
}
Some((specifier, _, Ok(None))) => {
Expand Down Expand Up @@ -1698,50 +1698,63 @@ impl<'a> Builder<'a> {
&mut self,
requested_specifier: &ModuleSpecifier,
kind: &ModuleKind,
response: &LoadResponse,
response: LoadResponse,
build_kind: &BuildKind,
maybe_assert_type: Option<String>,
assert_types: HashSet<Option<String>>,
) {
let (specifier, module_slot) = match response {
LoadResponse::BuiltIn { specifier } => {
self.check_specifier(requested_specifier, specifier);
let module_slot = ModuleSlot::Module(Module::new_without_source(
specifier.clone(),
ModuleKind::BuiltIn,
));
self.check_specifier(requested_specifier, &specifier);
let module_slot = if assert_types.len() != 1 {
ModuleSlot::Err(ModuleGraphError::ConflictingAssertions(
specifier.clone(),
))
} else {
ModuleSlot::Module(Module::new_without_source(
specifier.clone(),
ModuleKind::BuiltIn,
))
};
(specifier, module_slot)
}
LoadResponse::External { specifier } => {
self.check_specifier(requested_specifier, specifier);
let module_slot = ModuleSlot::Module(Module::new_without_source(
specifier.clone(),
ModuleKind::External,
));
self.check_specifier(requested_specifier, &specifier);
let module_slot = if assert_types.len() != 1 {
ModuleSlot::Err(ModuleGraphError::ConflictingAssertions(
specifier.clone(),
))
} else {
ModuleSlot::Module(Module::new_without_source(
specifier.clone(),
ModuleKind::External,
))
};
(specifier, module_slot)
}
LoadResponse::Module {
specifier,
content,
maybe_headers,
} => {
self.check_specifier(requested_specifier, specifier);
(
specifier,
self.check_specifier(requested_specifier, &specifier);
let module_slot = if assert_types.len() != 1 {
ModuleSlot::Err(ModuleGraphError::ConflictingAssertions(
specifier.clone(),
))
} else {
self.visit_module(
specifier,
&specifier,
kind,
maybe_headers.as_ref(),
content.clone(),
content,
build_kind,
maybe_assert_type,
),
)
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.
Expand All @@ -1750,7 +1763,7 @@ impl<'a> Builder<'a> {
specifier: &ModuleSpecifier,
kind: &ModuleKind,
maybe_headers: Option<&HashMap<String, String>>,
content: Arc<str>,
content: String,
build_kind: &BuildKind,
maybe_assert_type: Option<String>,
) -> ModuleSlot {
Expand Down Expand Up @@ -2041,11 +2054,11 @@ mod tests {
fn test_module_dependency_includes() {
let specifier = ModuleSpecifier::parse("file:///a.ts").unwrap();
let source_parser = ast::DefaultSourceParser::default();
let content = r#"import * as b from "./b.ts";"#;
let content = r#"import * as b from "./b.ts";"#.to_string();
let slot = parse_module(
&specifier,
None,
content.into(),
content,
None,
Some(&ModuleKind::Esm),
None,
Expand Down Expand Up @@ -2130,7 +2143,7 @@ mod tests {
Ok(Some(LoadResponse::Module {
specifier: specifier.clone(),
maybe_headers: None,
content: "await import('file:///bar.js')".into(),
content: "await import('file:///bar.js')".to_string(),
}))
})
}
Expand All @@ -2141,7 +2154,7 @@ mod tests {
Ok(Some(LoadResponse::Module {
specifier: specifier.clone(),
maybe_headers: None,
content: "import 'file:///baz.js'".into(),
content: "import 'file:///baz.js'".to_string(),
}))
})
}
Expand All @@ -2152,7 +2165,7 @@ mod tests {
Ok(Some(LoadResponse::Module {
specifier: specifier.clone(),
maybe_headers: None,
content: "console.log('Hello, world!')".into(),
content: "console.log('Hello, world!')".to_string(),
}))
})
}
Expand Down Expand Up @@ -2196,7 +2209,7 @@ mod tests {
Ok(Some(LoadResponse::Module {
specifier: specifier.clone(),
maybe_headers: None,
content: "await import('file:///bar.js')".into(),
content: "await import('file:///bar.js')".to_string(),
}))
}),
"file:///bar.js" => Box::pin(async move { Ok(None) }),
Expand Down Expand Up @@ -2256,14 +2269,14 @@ mod tests {
Ok(Some(LoadResponse::Module {
specifier: Url::parse("file:///foo_actual.js").unwrap(),
maybe_headers: None,
content: "import 'file:///bar.js'".into(),
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: "(".into(),
content: "(".to_string(),
}))
}),
_ => unreachable!(),
Expand Down
3 changes: 3 additions & 0 deletions src/info.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -249,6 +249,9 @@ impl ModuleGraphError {
Self::InvalidTypeAssertion { .. } => {
fmt_error_msg(f, prefix, last, specifier, "(invalid import assertion)")
}
Self::ConflictingAssertions { .. } => {
fmt_error_msg(f, prefix, last, specifier, "(conflicting assertions)")
}
Self::LoadingErr(_, _) => {
fmt_error_msg(f, prefix, last, specifier, "(loading error)")
}
Expand Down
17 changes: 8 additions & 9 deletions src/lib.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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")] {
Expand Down Expand Up @@ -149,7 +148,7 @@ cfg_if! {
pub fn parse_module(
specifier: &ModuleSpecifier,
maybe_headers: Option<&HashMap<String, String>>,
content: Arc<str>,
content: String,
maybe_kind: Option<&ModuleKind>,
maybe_resolver: Option<&dyn Resolver>,
maybe_parser: Option<&dyn SourceParser>,
Expand Down Expand Up @@ -294,7 +293,7 @@ cfg_if! {
match graph::parse_module(
&specifier,
maybe_headers.as_ref(),
content.into(),
content,
None,
maybe_kind.as_ref(),
maybe_resolver.as_ref().map(|r| r as &dyn Resolver),
Expand Down Expand Up @@ -2821,7 +2820,7 @@ export function a(a) {
export { c } from "./c.ts";
const d = await import("./d.ts");
"#
.into(),
.to_string(),
None,
None,
None,
Expand All @@ -2843,7 +2842,7 @@ export function a(a) {
import a from "./a.json" assert { type: "json" };
await import("./b.json", { assert: { type: "json" } });
"#
.into(),
.to_string(),
Some(&ModuleKind::Esm),
None,
None,
Expand Down Expand Up @@ -2911,7 +2910,7 @@ export function a(a) {
return <div>Hello Deno</div>;
}
"#
.into(),
.to_string(),
Some(&ModuleKind::Esm),
None,
None,
Expand Down Expand Up @@ -2951,7 +2950,7 @@ export function a(a) {
r#"declare interface A {
a: string;
}"#
.into(),
.to_string(),
Some(&ModuleKind::Esm),
None,
None,
Expand All @@ -2976,7 +2975,7 @@ export function a(a) {
return;
}
"#
.into(),
.to_string(),
Some(&ModuleKind::Esm),
None,
None,
Expand Down Expand Up @@ -3045,7 +3044,7 @@ export function a(a: A): B {
return;
}
"#
.into(),
.to_string(),
Some(&ModuleKind::Esm),
None,
None,
Expand Down
7 changes: 3 additions & 4 deletions src/source.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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";

Expand Down Expand Up @@ -56,7 +55,7 @@ pub enum LoadResponse {
/// A loaded module.
Module {
/// The content of the remote module.
content: Arc<str>,
content: String,
/// The final specifier of the module.
specifier: ModuleSpecifier,
/// If the module is a remote module, the headers should be returned as a
Expand Down Expand Up @@ -213,7 +212,7 @@ pub fn load_data_url(
Ok(Some(LoadResponse::Module {
specifier: specifier.clone(),
maybe_headers: Some(headers),
content: content.into(),
content,
}))
}

Expand Down Expand Up @@ -265,7 +264,7 @@ impl MemoryLoader {
})
.collect()
}),
content: content.as_ref().into(),
content: content.as_ref().to_string(),
}),
Source::BuiltIn(specifier) => Ok(LoadResponse::BuiltIn {
specifier: ModuleSpecifier::parse(specifier.as_ref()).unwrap(),
Expand Down