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.

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

fix: resolve jsxImportSource relative to module #15561

Merged
merged 4 commits into from
Aug 24, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 8 additions & 8 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 4 additions & 4 deletions cli/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -48,9 +48,9 @@ winres = "=0.1.12"
[dependencies]
deno_ast = { version = "0.17.0", features = ["bundler", "cjs", "codegen", "dep_graph", "module_specifier", "proposal", "react", "sourcemap", "transforms", "transpiling", "typescript", "view", "visit"] }
deno_core = { version = "0.147.0", path = "../core" }
deno_doc = "0.42.0"
deno_emit = "0.6.0"
deno_graph = "0.31.0"
deno_doc = "0.43.0"
deno_emit = "0.7.0"
deno_graph = "0.32.0"
deno_lint = { version = "0.32.0", features = ["docs"] }
deno_runtime = { version = "0.73.0", path = "../runtime" }
deno_task_shell = "0.5.0"
Expand All @@ -69,7 +69,7 @@ dprint-plugin-markdown = "=0.14.0"
dprint-plugin-typescript = "=0.71.2"
encoding_rs = "=0.8.31"
env_logger = "=0.9.0"
eszip = "=0.24.0"
eszip = "=0.25.0"
fancy-regex = "=0.10.0"
flate2 = "=1.0.24"
http = "=0.2.6"
Expand Down
31 changes: 15 additions & 16 deletions cli/args/config_file.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ use crate::fs_util::specifier_to_file_path;
use deno_core::anyhow::anyhow;
use deno_core::anyhow::bail;
use deno_core::anyhow::Context;
use deno_core::error::custom_error;
use deno_core::error::AnyError;
use deno_core::serde::Deserialize;
use deno_core::serde::Serialize;
Expand All @@ -30,6 +29,11 @@ use std::path::PathBuf;
pub type MaybeImportsResult =
Result<Option<Vec<(ModuleSpecifier, Vec<String>)>>, AnyError>;

pub struct JsxImportSourceConfig {
pub default_specifier: Option<String>,
pub module: String,
}

/// The transpile options that are significant out of a user provided tsconfig
/// file, that we want to deserialize out of the final config for a transpile.
#[derive(Debug, Deserialize)]
Expand Down Expand Up @@ -680,17 +684,6 @@ impl ConfigFile {
if let Some(types) = compiler_options.types {
imports.extend(types);
}
if compiler_options.jsx == Some("react-jsx".to_string()) {
imports.push(format!(
"{}/jsx-runtime",
compiler_options.jsx_import_source.ok_or_else(|| custom_error("TypeError", "Compiler option 'jsx' set to 'react-jsx', but no 'jsxImportSource' defined."))?
));
} else if compiler_options.jsx == Some("react-jsxdev".to_string()) {
imports.push(format!(
"{}/jsx-dev-runtime",
compiler_options.jsx_import_source.ok_or_else(|| custom_error("TypeError", "Compiler option 'jsx' set to 'react-jsxdev', but no 'jsxImportSource' defined."))?
));
}
if !imports.is_empty() {
let referrer = self.specifier.clone();
Ok(Some(vec![(referrer, imports)]))
Expand All @@ -700,16 +693,22 @@ impl ConfigFile {
}

/// Based on the compiler options in the configuration file, return the
/// implied JSX import source module.
pub fn to_maybe_jsx_import_source_module(&self) -> Option<String> {
/// JSX import source configuration.
pub fn to_maybe_jsx_import_source_config(
&self,
) -> Option<JsxImportSourceConfig> {
let compiler_options_value = self.json.compiler_options.as_ref()?;
let compiler_options: CompilerOptions =
serde_json::from_value(compiler_options_value.clone()).ok()?;
match compiler_options.jsx.as_deref() {
let module = match compiler_options.jsx.as_deref() {
Some("react-jsx") => Some("jsx-runtime".to_string()),
Some("react-jsxdev") => Some("jsx-dev-runtime".to_string()),
_ => None,
}
};
module.map(|module| JsxImportSourceConfig {
default_specifier: compiler_options.jsx_import_source,
module,
})
}

pub fn to_fmt_config(&self) -> Result<Option<FmtConfig>, AnyError> {
Expand Down
9 changes: 6 additions & 3 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ use std::env;
use std::net::SocketAddr;
use std::path::PathBuf;

use crate::args::config_file::JsxImportSourceConfig;
use crate::compat;
use crate::deno_dir::DenoDir;
use crate::emit::get_ts_config_for_emit;
Expand Down Expand Up @@ -210,12 +211,14 @@ impl CliOptions {
}
}

/// Return the implied JSX import source module.
pub fn to_maybe_jsx_import_source_module(&self) -> Option<String> {
/// Return the JSX import source configuration.
pub fn to_maybe_jsx_import_source_config(
&self,
) -> Option<JsxImportSourceConfig> {
self
.maybe_config_file
.as_ref()
.and_then(|c| c.to_maybe_jsx_import_source_module())
.and_then(|c| c.to_maybe_jsx_import_source_config())
}

/// Return any imports that should be brought into the scope of the module
Expand Down
4 changes: 2 additions & 2 deletions cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -998,8 +998,8 @@ impl Documents {
// TODO(@kitsonk) update resolved dependencies?
self.maybe_import_map = maybe_import_map.map(ImportMapResolver::new);
self.maybe_jsx_resolver = maybe_config_file.and_then(|cf| {
cf.to_maybe_jsx_import_source_module()
.map(|im| JsxResolver::new(im, self.maybe_import_map.clone()))
cf.to_maybe_jsx_import_source_config()
.map(|cfg| JsxResolver::new(cfg, self.maybe_import_map.clone()))
});
self.imports = Arc::new(
if let Some(Ok(Some(imports))) =
Expand Down
4 changes: 2 additions & 2 deletions cli/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -457,8 +457,8 @@ async fn create_graph_and_maybe_check(
ps.maybe_import_map.clone().map(ImportMapResolver::new);
let maybe_jsx_resolver = ps
.options
.to_maybe_jsx_import_source_module()
.map(|im| JsxResolver::new(im, maybe_import_map_resolver.clone()));
.to_maybe_jsx_import_source_config()
.map(|cfg| JsxResolver::new(cfg, maybe_import_map_resolver.clone()));
let maybe_resolver = if maybe_jsx_resolver.is_some() {
maybe_jsx_resolver.as_ref().map(|jr| jr.as_resolver())
} else {
Expand Down
8 changes: 4 additions & 4 deletions cli/proc_state.rs
Original file line number Diff line number Diff line change
Expand Up @@ -192,8 +192,8 @@ impl ProcState {
let maybe_import_map_resolver =
maybe_import_map.clone().map(ImportMapResolver::new);
let maybe_jsx_resolver = cli_options
.to_maybe_jsx_import_source_module()
.map(|im| JsxResolver::new(im, maybe_import_map_resolver.clone()));
.to_maybe_jsx_import_source_config()
.map(|cfg| JsxResolver::new(cfg, maybe_import_map_resolver.clone()));
let maybe_resolver: Option<
Arc<dyn deno_graph::source::Resolver + Send + Sync>,
> = if cli_options.compat() {
Expand Down Expand Up @@ -643,8 +643,8 @@ impl ProcState {
let maybe_imports = self.options.to_maybe_imports()?;
let maybe_jsx_resolver = self
.options
.to_maybe_jsx_import_source_module()
.map(|im| JsxResolver::new(im, maybe_import_map_resolver.clone()));
.to_maybe_jsx_import_source_config()
.map(|cfg| JsxResolver::new(cfg, maybe_import_map_resolver.clone()));
let maybe_resolver = if maybe_jsx_resolver.is_some() {
maybe_jsx_resolver.as_ref().map(|jr| jr.as_resolver())
} else {
Expand Down
12 changes: 10 additions & 2 deletions cli/resolver.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ use deno_graph::source::Resolver;
use import_map::ImportMap;
use std::sync::Arc;

use crate::args::config_file::JsxImportSourceConfig;

/// Wraps an import map to be used when building a deno_graph module graph.
/// This is done to avoid having `import_map` be a direct dependency of
/// `deno_graph`.
Expand Down Expand Up @@ -38,17 +40,19 @@ impl Resolver for ImportMapResolver {

#[derive(Debug, Default, Clone)]
pub struct JsxResolver {
default_jsx_import_source: Option<String>,
jsx_import_source_module: String,
maybe_import_map_resolver: Option<ImportMapResolver>,
}

impl JsxResolver {
pub fn new(
jsx_import_source_module: String,
jsx_import_source_config: JsxImportSourceConfig,
maybe_import_map_resolver: Option<ImportMapResolver>,
) -> Self {
Self {
jsx_import_source_module,
default_jsx_import_source: jsx_import_source_config.default_specifier,
jsx_import_source_module: jsx_import_source_config.module,
maybe_import_map_resolver,
}
}
Expand All @@ -59,6 +63,10 @@ impl JsxResolver {
}

impl Resolver for JsxResolver {
fn default_jsx_import_source(&self) -> Option<String> {
self.default_jsx_import_source.clone()
}

fn jsx_import_source_module(&self) -> &str {
self.jsx_import_source_module.as_str()
}
Expand Down
12 changes: 12 additions & 0 deletions cli/tests/integration/run_tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1337,6 +1337,18 @@ itest!(jsx_import_source_import_map_dev {
http_server: true,
});

itest!(jsx_import_source_import_map_scoped {
args: "run --reload --import-map jsx/import-map-scoped.json --config jsx/deno-jsx-import-map.jsonc subdir/jsx_import_source_no_pragma.tsx",
output: "jsx_import_source_import_map.out",
http_server: true,
});

itest!(jsx_import_source_import_map_scoped_dev {
args: "run --reload --import-map jsx/import-map-scoped.json --config jsx/deno-jsxdev-import-map.jsonc subdir/jsx_import_source_no_pragma.tsx",
output: "jsx_import_source_import_map_dev.out",
http_server: true,
});

itest!(jsx_import_source_pragma_no_check {
args: "run --reload --no-check jsx_import_source_pragma.tsx",
output: "jsx_import_source.out",
Expand Down
8 changes: 8 additions & 0 deletions cli/tests/testdata/jsx/import-map-scoped.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"scopes": {
"../subdir/": {
"jsx/jsx-runtime": "http://localhost:4545/jsx/jsx-runtime/index.ts",
"jsx/jsx-dev-runtime": "http://localhost:4545/jsx/jsx-dev-runtime/index.ts"
}
}
}
2 changes: 1 addition & 1 deletion cli/tests/testdata/jsx_import_source_error.out
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
error: Module not found "file:///[WILDCARD]/nonexistent/jsx-runtime".
at file:///[WILDCARD]/deno-jsx-error.jsonc:1:1
at file:///[WILDCARD]/jsx_import_source_no_pragma.tsx:1:1
7 changes: 7 additions & 0 deletions cli/tests/testdata/subdir/jsx_import_source_no_pragma.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
function A() {
return "hello";
}

export function B() {
return <A></A>;
}