fix(vendor): do not panic vendoring with jsxImportSource and no jsx files (#19837)

Closes #19833
This commit is contained in:
David Sherret 2023-07-14 18:10:42 -04:00 committed by GitHub
parent b83dac3b14
commit 306b51d772
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 248 additions and 94 deletions

View file

@ -32,6 +32,16 @@ pub type MaybeImportsResult =
pub struct JsxImportSourceConfig {
pub default_specifier: Option<String>,
pub module: String,
pub base_url: ModuleSpecifier,
}
impl JsxImportSourceConfig {
pub fn maybe_specifier_text(&self) -> Option<String> {
self
.default_specifier
.as_ref()
.map(|default_specifier| format!("{}/{}", default_specifier, self.module))
}
}
/// The transpile options that are significant out of a user provided tsconfig
@ -1035,6 +1045,7 @@ impl ConfigFile {
module.map(|module| JsxImportSourceConfig {
default_specifier: compiler_options.jsx_import_source,
module,
base_url: self.specifier.clone(),
})
}

View file

@ -9,6 +9,7 @@ use deno_ast::ModuleSpecifier;
use deno_core::anyhow::bail;
use deno_core::anyhow::Context;
use deno_core::error::AnyError;
use deno_core::futures::future::LocalBoxFuture;
use deno_core::parking_lot::Mutex;
use deno_graph::EsmModule;
use deno_graph::Module;
@ -21,6 +22,7 @@ use crate::args::Lockfile;
use crate::cache::ParsedSourceCache;
use crate::graph_util;
use crate::graph_util::graph_lock_or_exit;
use crate::tools::vendor::import_map::BuildImportMapInput;
use super::analyze::has_default_export;
use super::import_map::build_import_map;
@ -57,16 +59,47 @@ impl VendorEnvironment for RealVendorEnvironment {
}
}
type BuildGraphFuture = LocalBoxFuture<'static, Result<ModuleGraph, AnyError>>;
pub struct BuildInput<
'a,
TBuildGraphFn: FnOnce(Vec<ModuleSpecifier>) -> BuildGraphFuture,
TEnvironment: VendorEnvironment,
> {
pub entry_points: Vec<ModuleSpecifier>,
pub build_graph: TBuildGraphFn,
pub parsed_source_cache: &'a ParsedSourceCache,
pub output_dir: &'a Path,
pub maybe_original_import_map: Option<&'a ImportMap>,
pub maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
pub maybe_jsx_import_source: Option<&'a JsxImportSourceConfig>,
pub resolver: &'a dyn deno_graph::source::Resolver,
pub environment: &'a TEnvironment,
}
pub struct BuildOutput {
pub vendored_count: usize,
pub graph: ModuleGraph,
}
/// Vendors remote modules and returns how many were vendored.
pub fn build(
graph: ModuleGraph,
parsed_source_cache: &ParsedSourceCache,
output_dir: &Path,
original_import_map: Option<&ImportMap>,
maybe_lockfile: Option<Arc<Mutex<Lockfile>>>,
jsx_import_source: Option<&JsxImportSourceConfig>,
environment: &impl VendorEnvironment,
) -> Result<usize, AnyError> {
pub async fn build<
TBuildGraphFn: FnOnce(Vec<ModuleSpecifier>) -> BuildGraphFuture,
TEnvironment: VendorEnvironment,
>(
input: BuildInput<'_, TBuildGraphFn, TEnvironment>,
) -> Result<BuildOutput, AnyError> {
let BuildInput {
mut entry_points,
build_graph,
parsed_source_cache,
output_dir,
maybe_original_import_map: original_import_map,
maybe_lockfile,
maybe_jsx_import_source: jsx_import_source,
resolver,
environment,
} = input;
assert!(output_dir.is_absolute());
let output_dir_specifier =
ModuleSpecifier::from_directory_path(output_dir).unwrap();
@ -75,6 +108,19 @@ pub fn build(
validate_original_import_map(original_im, &output_dir_specifier)?;
}
// add the jsx import source to the entry points to ensure it is always vendored
if let Some(jsx_import_source) = jsx_import_source {
if let Some(specifier_text) = jsx_import_source.maybe_specifier_text() {
if let Ok(specifier) =
resolver.resolve(&specifier_text, &jsx_import_source.base_url)
{
entry_points.push(specifier);
}
}
}
let graph = build_graph(entry_points).await?;
// check the lockfile
if let Some(lockfile) = maybe_lockfile {
graph_lock_or_exit(&graph, &mut lockfile.lock());
@ -130,19 +176,23 @@ pub fn build(
// create the import map if necessary
if !remote_modules.is_empty() {
let import_map_path = output_dir.join("import_map.json");
let import_map_text = build_import_map(
&output_dir_specifier,
&graph,
&all_modules,
&mappings,
let import_map_text = build_import_map(BuildImportMapInput {
base_dir: &output_dir_specifier,
graph: &graph,
modules: &all_modules,
mappings: &mappings,
original_import_map,
jsx_import_source,
resolver,
parsed_source_cache,
)?;
})?;
environment.write_file(&import_map_path, &import_map_text)?;
}
Ok(remote_modules.len())
Ok(BuildOutput {
vendored_count: remote_modules.len(),
graph,
})
}
fn validate_original_import_map(
@ -1152,12 +1202,13 @@ mod test {
}
#[tokio::test]
async fn existing_import_map_jsx_import_source() {
async fn existing_import_map_jsx_import_source_jsx_files() {
let mut builder = VendorTestBuilder::default();
builder.add_entry_point("/mod.tsx");
builder.set_jsx_import_source_config(JsxImportSourceConfig {
default_specifier: Some("preact".to_string()),
module: "jsx-runtime".to_string(),
base_url: builder.resolve_to_url("/deno.json"),
});
let mut original_import_map = builder.new_import_map("/import_map.json");
let imports = original_import_map.imports_mut();
@ -1199,6 +1250,59 @@ mod test {
);
}
#[tokio::test]
async fn existing_import_map_jsx_import_source_no_jsx_files() {
let mut builder = VendorTestBuilder::default();
builder.add_entry_point("/mod.ts");
builder.set_jsx_import_source_config(JsxImportSourceConfig {
default_specifier: Some("preact".to_string()),
module: "jsx-runtime".to_string(),
base_url: builder.resolve_to_url("/deno.json"),
});
let mut original_import_map = builder.new_import_map("/import_map.json");
let imports = original_import_map.imports_mut();
imports
.append(
"preact/".to_string(),
"https://localhost/preact/".to_string(),
)
.unwrap();
let output = builder
.with_loader(|loader| {
loader.add("/mod.ts", "import 'https://localhost/mod.ts';");
loader.add("https://localhost/mod.ts", "console.log(1)");
loader.add_with_headers(
"https://localhost/preact/jsx-runtime",
"export function stuff() {}",
&[("content-type", "application/typescript")],
);
})
.set_original_import_map(original_import_map)
.build()
.await
.unwrap();
assert_eq!(
output.import_map,
Some(json!({
"imports": {
"https://localhost/": "./localhost/",
"preact/jsx-runtime": "./localhost/preact/jsx-runtime.ts"
},
}))
);
assert_eq!(
output.files,
to_file_vec(&[
("/vendor/localhost/mod.ts", "console.log(1)"),
(
"/vendor/localhost/preact/jsx-runtime.ts",
"export function stuff() {}"
),
]),
);
}
#[tokio::test]
async fn vendor_file_fails_loading_dynamic_import() {
let mut builder = VendorTestBuilder::with_default_setup();

View file

@ -177,15 +177,30 @@ impl<'a> ImportsBuilder<'a> {
}
}
pub struct BuildImportMapInput<'a> {
pub base_dir: &'a ModuleSpecifier,
pub modules: &'a [&'a Module],
pub graph: &'a ModuleGraph,
pub mappings: &'a Mappings,
pub original_import_map: Option<&'a ImportMap>,
pub jsx_import_source: Option<&'a JsxImportSourceConfig>,
pub resolver: &'a dyn deno_graph::source::Resolver,
pub parsed_source_cache: &'a ParsedSourceCache,
}
pub fn build_import_map(
base_dir: &ModuleSpecifier,
graph: &ModuleGraph,
modules: &[&Module],
mappings: &Mappings,
original_import_map: Option<&ImportMap>,
jsx_import_source: Option<&JsxImportSourceConfig>,
parsed_source_cache: &ParsedSourceCache,
input: BuildImportMapInput<'_>,
) -> Result<String, AnyError> {
let BuildImportMapInput {
base_dir,
modules,
graph,
mappings,
original_import_map,
jsx_import_source,
resolver,
parsed_source_cache,
} = input;
let mut builder = ImportMapBuilder::new(base_dir, mappings);
visit_modules(graph, modules, mappings, &mut builder, parsed_source_cache)?;
@ -196,14 +211,10 @@ pub fn build_import_map(
}
// add the jsx import source to the destination import map, if mapped in the original import map
if let (Some(import_map), Some(jsx_import_source)) =
(original_import_map, jsx_import_source)
{
if let Some(default_specifier) = &jsx_import_source.default_specifier {
let specifier_text =
format!("{}/{}", default_specifier, jsx_import_source.module);
if let Some(jsx_import_source) = jsx_import_source {
if let Some(specifier_text) = jsx_import_source.maybe_specifier_text() {
if let Ok(resolved_url) =
import_map.resolve(&specifier_text, import_map.base_url())
resolver.resolve(&specifier_text, &jsx_import_source.base_url)
{
builder.imports.add(specifier_text, &resolved_url);
}

View file

@ -9,6 +9,7 @@ use deno_ast::TextChange;
use deno_core::anyhow::bail;
use deno_core::anyhow::Context;
use deno_core::error::AnyError;
use deno_core::futures::FutureExt;
use deno_core::resolve_url_or_path;
use deno_graph::GraphKind;
use log::warn;
@ -19,7 +20,6 @@ use crate::args::Flags;
use crate::args::FmtOptionsConfig;
use crate::args::VendorFlags;
use crate::factory::CliFactory;
use crate::graph_util::ModuleGraphBuilder;
use crate::tools::fmt::format_json;
use crate::util::fs::canonicalize_path;
use crate::util::fs::resolve_from_cwd;
@ -48,25 +48,35 @@ pub async fn vendor(
validate_options(&mut cli_options, &output_dir)?;
let factory = CliFactory::from_cli_options(Arc::new(cli_options));
let cli_options = factory.cli_options();
let graph = create_graph(
factory.module_graph_builder().await?,
&vendor_flags,
cli_options.initial_cwd(),
)
let entry_points =
resolve_entry_points(&vendor_flags, cli_options.initial_cwd())?;
let jsx_import_source = cli_options.to_maybe_jsx_import_source_config();
let module_graph_builder = factory.module_graph_builder().await?.clone();
let output = build::build(build::BuildInput {
entry_points,
build_graph: move |entry_points| {
async move {
module_graph_builder
.create_graph(GraphKind::All, entry_points)
.await
}
.boxed_local()
},
parsed_source_cache: factory.parsed_source_cache()?,
output_dir: &output_dir,
maybe_original_import_map: factory.maybe_import_map().await?.as_deref(),
maybe_lockfile: factory.maybe_lockfile().clone(),
maybe_jsx_import_source: jsx_import_source.as_ref(),
resolver: factory.resolver().await?.as_graph_resolver(),
environment: &build::RealVendorEnvironment,
})
.await?;
let vendored_count = output.vendored_count;
let graph = output.graph;
let npm_package_count = graph.npm_packages.len();
let try_add_node_modules_dir = npm_package_count > 0
&& cli_options.node_modules_dir_enablement().unwrap_or(true);
let jsx_import_source = cli_options.to_maybe_jsx_import_source_config();
let vendored_count = build::build(
graph,
factory.parsed_source_cache()?,
&output_dir,
factory.maybe_import_map().await?.as_deref(),
factory.maybe_lockfile().clone(),
jsx_import_source.as_ref(),
&build::RealVendorEnvironment,
)?;
log::info!(
concat!("Vendored {} {} into {} directory.",),
@ -363,20 +373,15 @@ fn is_dir_empty(dir_path: &Path) -> Result<bool, AnyError> {
}
}
async fn create_graph(
module_graph_builder: &ModuleGraphBuilder,
fn resolve_entry_points(
flags: &VendorFlags,
initial_cwd: &Path,
) -> Result<deno_graph::ModuleGraph, AnyError> {
let entry_points = flags
) -> Result<Vec<ModuleSpecifier>, AnyError> {
flags
.specifiers
.iter()
.map(|p| resolve_url_or_path(p, initial_cwd))
.collect::<Result<Vec<_>, _>>()?;
module_graph_builder
.create_graph(GraphKind::All, entry_points)
.await
.map(|p| resolve_url_or_path(p, initial_cwd).map_err(|e| e.into()))
.collect::<Result<Vec<_>, _>>()
}
#[cfg(test)]

View file

@ -12,6 +12,7 @@ use deno_core::anyhow::anyhow;
use deno_core::anyhow::bail;
use deno_core::error::AnyError;
use deno_core::futures;
use deno_core::futures::FutureExt;
use deno_core::serde_json;
use deno_graph::source::LoadFuture;
use deno_graph::source::LoadResponse;
@ -195,8 +196,12 @@ impl VendorTestBuilder {
builder
}
pub fn resolve_to_url(&self, path: &str) -> ModuleSpecifier {
ModuleSpecifier::from_file_path(make_path(path)).unwrap()
}
pub fn new_import_map(&self, base_path: &str) -> ImportMap {
let base = ModuleSpecifier::from_file_path(make_path(base_path)).unwrap();
let base = self.resolve_to_url(base_path);
ImportMap::new(base)
}
@ -226,27 +231,42 @@ impl VendorTestBuilder {
pub async fn build(&mut self) -> Result<VendorOutput, AnyError> {
let output_dir = make_path("/vendor");
let roots = self.entry_points.clone();
let entry_points = self.entry_points.clone();
let loader = self.loader.clone();
let parsed_source_cache = ParsedSourceCache::new_in_memory();
let analyzer = parsed_source_cache.as_analyzer();
let graph = build_test_graph(
roots,
let resolver = Arc::new(build_resolver(
self.jsx_import_source_config.clone(),
self.original_import_map.clone(),
loader,
&*analyzer,
)
.await;
super::build::build(
graph,
&parsed_source_cache,
&output_dir,
self.original_import_map.as_ref(),
None,
self.jsx_import_source_config.as_ref(),
&self.environment,
)?;
));
super::build::build(super::build::BuildInput {
entry_points,
build_graph: {
let resolver = resolver.clone();
move |entry_points| {
async move {
Ok(
build_test_graph(
entry_points,
loader,
resolver.as_graph_resolver(),
&*analyzer,
)
.await,
)
}
.boxed_local()
}
},
parsed_source_cache: &parsed_source_cache,
output_dir: &output_dir,
maybe_original_import_map: self.original_import_map.as_ref(),
maybe_lockfile: None,
maybe_jsx_import_source: self.jsx_import_source_config.as_ref(),
resolver: resolver.as_graph_resolver(),
environment: &self.environment,
})
.await?;
let mut files = self.environment.files.borrow_mut();
let import_map = files.remove(&output_dir.join("import_map.json"));
@ -269,37 +289,40 @@ impl VendorTestBuilder {
}
}
async fn build_test_graph(
roots: Vec<ModuleSpecifier>,
fn build_resolver(
jsx_import_source_config: Option<JsxImportSourceConfig>,
original_import_map: Option<ImportMap>,
) -> CliGraphResolver {
let npm_registry_api = Arc::new(CliNpmRegistryApi::new_uninitialized());
let npm_resolution = Arc::new(NpmResolution::from_serialized(
npm_registry_api.clone(),
None,
None,
));
CliGraphResolver::new(
jsx_import_source_config,
original_import_map.map(Arc::new),
false,
npm_registry_api,
npm_resolution,
Default::default(),
Default::default(),
)
}
async fn build_test_graph(
roots: Vec<ModuleSpecifier>,
mut loader: TestLoader,
resolver: &dyn deno_graph::source::Resolver,
analyzer: &dyn deno_graph::ModuleAnalyzer,
) -> ModuleGraph {
let resolver = original_import_map.map(|original_import_map| {
let npm_registry_api = Arc::new(CliNpmRegistryApi::new_uninitialized());
let npm_resolution = Arc::new(NpmResolution::from_serialized(
npm_registry_api.clone(),
None,
None,
));
CliGraphResolver::new(
jsx_import_source_config,
Some(Arc::new(original_import_map)),
false,
npm_registry_api,
npm_resolution,
Default::default(),
Default::default(),
)
});
let mut graph = ModuleGraph::new(GraphKind::All);
graph
.build(
roots,
&mut loader,
deno_graph::BuildOptions {
resolver: resolver.as_ref().map(|r| r.as_graph_resolver()),
resolver: Some(resolver),
module_analyzer: Some(analyzer),
..Default::default()
},