Skip to content

Commit

Permalink
feat(unstable): ability to resolve specifiers with no extension, spec…
Browse files Browse the repository at this point in the history
…ifiers for a directory, and TS files from JS extensions (denoland#21464)

Adds an `--unstable-sloppy-imports` flag which supports the
following for `file:` specifiers:

* Allows writing `./mod` in a specifier to do extension probing.
- ex. `import { Example } from "./example"` instead of `import { Example
} from "./example.ts"`
* Allows writing `./routes` to do directory extension probing for files
like `./routes/index.ts`
* Allows writing `./mod.js` for *mod.ts* files.

This functionality is **NOT RECOMMENDED** for general use with Deno:

1. It's not as optimal for perf:
https://marvinh.dev/blog/speeding-up-javascript-ecosystem-part-2/
1. It makes tooling in the ecosystem more complex in order to have to
understand this.
1. The "Deno way" is to be explicit about what you're doing. It's better
in the long run.
1. It doesn't work if published to the Deno registry because doing stuff
like extension probing with remote specifiers would be incredibly slow.

This is instead only recommended to help with migrating existing
projects to Deno. For example, it's very useful for getting CJS projects
written with import/export declaration working in Deno without modifying
module specifiers and for supporting TS ESM projects written with
`./mod.js` specifiers.

This feature will output warnings to guide the user towards correcting
their specifiers. Additionally, quick fixes are provided in the LSP to
update these specifiers:
  • Loading branch information
dsherret committed Dec 7, 2023
1 parent 9314928 commit 890780a
Show file tree
Hide file tree
Showing 18 changed files with 816 additions and 47 deletions.
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 @@ -60,16 +60,16 @@ deno_ast = { workspace = true, features = ["bundler", "cjs", "codegen", "dep_gra
deno_cache_dir = "=0.6.1"
deno_config = "=0.6.5"
deno_core = { workspace = true, features = ["include_js_files_for_snapshotting"] }
deno_doc = { version = "=0.73.5", features = ["html"] }
deno_emit = "=0.31.5"
deno_graph = "=0.61.5"
deno_doc = { version = "=0.74.0", features = ["html"] }
deno_emit = "=0.32.0"
deno_graph = "=0.62.0"
deno_lint = { version = "=0.52.2", features = ["docs"] }
deno_lockfile.workspace = true
deno_npm = "0.15.3"
deno_runtime = { workspace = true, features = ["dont_create_runtime_snapshot", "include_js_files_for_snapshotting"] }
deno_semver = "0.5.1"
deno_task_shell = "=0.14.0"
eszip = "=0.55.5"
eszip = "=0.56.0"
napi_sym.workspace = true

async-trait.workspace = true
Expand Down
13 changes: 13 additions & 0 deletions cli/args/flags.rs
Original file line number Diff line number Diff line change
Expand Up @@ -455,6 +455,7 @@ pub struct Flags {
pub unstable: bool,
pub unstable_bare_node_builtins: bool,
pub unstable_byonm: bool,
pub unstable_sloppy_imports: bool,
pub unstable_workspaces: bool,
pub unstable_features: Vec<String>,
pub unsafely_ignore_certificate_errors: Option<Vec<String>>,
Expand Down Expand Up @@ -862,6 +863,7 @@ pub fn flags_from_vec(args: Vec<String>) -> clap::error::Result<Flags> {
flags.unstable_bare_node_builtins =
matches.get_flag("unstable-bare-node-builtins");
flags.unstable_byonm = matches.get_flag("unstable-byonm");
flags.unstable_sloppy_imports = matches.get_flag("unstable-sloppy-imports");
flags.unstable_workspaces = matches.get_flag("unstable-workspaces");

if matches.get_flag("quiet") {
Expand Down Expand Up @@ -981,6 +983,17 @@ fn clap_root() -> Command {
.action(ArgAction::SetTrue)
.global(true),
)
.arg(
Arg::new("unstable-sloppy-imports")
.long("unstable-sloppy-imports")
.help(
"Enable unstable resolving of specifiers by extension probing, .js to .ts, and directory probing.",
)
.env("DENO_UNSTABLE_SLOPPY_IMPORTS")
.value_parser(FalseyValueParser::new())
.action(ArgAction::SetTrue)
.global(true),
)
.arg(
Arg::new("unstable-workspaces")
.long("unstable-workspaces")
Expand Down
13 changes: 11 additions & 2 deletions cli/args/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1342,7 +1342,7 @@ impl CliOptions {
|| self
.maybe_config_file()
.as_ref()
.map(|c| c.json.unstable.contains(&"bare-node-builtins".to_string()))
.map(|c| c.has_unstable("bare-node-builtins"))
.unwrap_or(false)
}

Expand All @@ -1355,7 +1355,16 @@ impl CliOptions {
|| self
.maybe_config_file()
.as_ref()
.map(|c| c.json.unstable.iter().any(|c| c == "byonm"))
.map(|c| c.has_unstable("byonm"))
.unwrap_or(false)
}

pub fn unstable_sloppy_imports(&self) -> bool {
self.flags.unstable_sloppy_imports
|| self
.maybe_config_file()
.as_ref()
.map(|c| c.has_unstable("sloppy-imports"))
.unwrap_or(false)
}

Expand Down
6 changes: 6 additions & 0 deletions cli/factory.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ use crate::npm::CliNpmResolverManagedPackageJsonInstallerOption;
use crate::npm::CliNpmResolverManagedSnapshotOption;
use crate::resolver::CliGraphResolver;
use crate::resolver::CliGraphResolverOptions;
use crate::resolver::UnstableSloppyImportsResolver;
use crate::standalone::DenoCompileBinaryWriter;
use crate::tools::check::TypeChecker;
use crate::util::file_watcher::WatcherCommunicator;
Expand Down Expand Up @@ -381,6 +382,11 @@ impl CliFactory {
Ok(Arc::new(CliGraphResolver::new(CliGraphResolverOptions {
fs: self.fs().clone(),
cjs_resolutions: Some(self.cjs_resolutions().clone()),
sloppy_imports_resolver: if self.options.unstable_sloppy_imports() {
Some(UnstableSloppyImportsResolver::new(self.fs().clone()))
} else {
None
},
node_resolver: Some(self.node_resolver().await?.clone()),
npm_resolver: if self.options.no_npm() {
None
Expand Down
2 changes: 1 addition & 1 deletion cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -857,7 +857,7 @@ mod test {
use deno_graph::ResolutionError;
use deno_graph::SpecifierError;

use crate::graph_util::get_resolution_error_bare_node_specifier;
use super::*;

#[test]
fn import_map_node_resolution_error() {
Expand Down
51 changes: 50 additions & 1 deletion cli/lsp/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1097,7 +1097,10 @@ impl DenoDiagnostic {
changes: Some(HashMap::from([(
specifier.clone(),
vec![lsp::TextEdit {
new_text: format!("\"{}\"", data.redirect),
new_text: format!(
"\"{}\"",
specifier_text_for_redirected(&data.redirect, specifier)
),
range: diagnostic.range,
}],
)])),
Expand Down Expand Up @@ -1193,6 +1196,27 @@ impl DenoDiagnostic {
}
}

fn specifier_text_for_redirected(
redirect: &lsp::Url,
referrer: &lsp::Url,
) -> String {
if redirect.scheme() == "file" && referrer.scheme() == "file" {
// use a relative specifier when it's going to a file url
match referrer.make_relative(redirect) {
Some(relative) => {
if relative.starts_with('.') {
relative
} else {
format!("./{}", relative)
}
}
None => redirect.to_string(),
}
} else {
redirect.to_string()
}
}

fn diagnose_resolution(
snapshot: &language_server::StateSnapshot,
dependency_key: &str,
Expand Down Expand Up @@ -1833,4 +1857,29 @@ let c: number = "a";
])
);
}

#[test]
fn test_specifier_text_for_redirected() {
#[track_caller]
fn run_test(specifier: &str, referrer: &str, expected: &str) {
let result = specifier_text_for_redirected(
&ModuleSpecifier::parse(specifier).unwrap(),
&ModuleSpecifier::parse(referrer).unwrap(),
);
assert_eq!(result, expected);
}

run_test("file:https:///a/a.ts", "file:https:///a/mod.ts", "./a.ts");
run_test("file:https:///a/a.ts", "file:https:///a/sub_dir/mod.ts", "../a.ts");
run_test(
"file:https:///a/sub_dir/a.ts",
"file:https:///a/mod.ts",
"./sub_dir/a.ts",
);
run_test(
"https://deno.land/x/example/mod.ts",
"file:https:///a/sub_dir/a.ts",
"https://deno.land/x/example/mod.ts",
);
}
}
Loading

0 comments on commit 890780a

Please sign in to comment.