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

refactor(core): remove ext: modules from the module map #19040

Merged
merged 28 commits into from
May 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
e57d139
refactor(core): move ext: import check to cli
nayeemrmn May 8, 2023
fc41726
remove core unit test
nayeemrmn May 8, 2023
fc2a85c
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 8, 2023
cbf2cc2
reset ci
nayeemrmn May 8, 2023
6212571
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 8, 2023
6cd6722
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 9, 2023
061441d
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 11, 2023
c916482
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 15, 2023
b161148
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 15, 2023
0f94a34
preserve snapshot-evaluated extension module names
nayeemrmn May 15, 2023
5d2a004
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 15, 2023
396247b
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 16, 2023
47c3004
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 16, 2023
f3b7422
add some comments and list runtime dep on cli
nayeemrmn May 16, 2023
ad165c0
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 18, 2023
bbf5f0a
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 22, 2023
3327463
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 22, 2023
e74a0a3
explicitly clear module map and re-inject node: built-ins
nayeemrmn May 22, 2023
a626f05
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 22, 2023
a72de27
less allocation
nayeemrmn May 22, 2023
670afc5
don't forget web workers
nayeemrmn May 23, 2023
7e09ffe
rename function
nayeemrmn May 23, 2023
ca856ec
update fixtures
nayeemrmn May 23, 2023
f0951e6
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 24, 2023
1692a99
revert extension macro changes
nayeemrmn May 24, 2023
83405b7
review
nayeemrmn May 24, 2023
92db565
move aliasing to core
nayeemrmn May 24, 2023
8505a64
Merge remote-tracking branch 'upstream/main' into ext-imports-check-i…
nayeemrmn May 24, 2023
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
2 changes: 1 addition & 1 deletion cli/build.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ mod ts {

let node_built_in_module_names = SUPPORTED_BUILTIN_NODE_MODULES
.iter()
.map(|s| s.name)
.map(|p| p.module_name())
.collect::<Vec<&str>>();
let build_libs = state.borrow::<Vec<&str>>();
json!({
Expand Down
5 changes: 2 additions & 3 deletions cli/graph_util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -378,9 +378,8 @@ pub fn enhanced_resolution_error_message(error: &ResolutionError) -> String {
pub fn get_resolution_error_bare_node_specifier(
error: &ResolutionError,
) -> Option<&str> {
get_resolution_error_bare_specifier(error).filter(|specifier| {
deno_node::resolve_builtin_node_module(specifier).is_ok()
})
get_resolution_error_bare_specifier(error)
.filter(|specifier| deno_node::is_builtin_node_module(specifier))
}

fn get_resolution_error_bare_specifier(
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -915,7 +915,7 @@ fn diagnose_resolution(
}
} else if let Some(module_name) = specifier.as_str().strip_prefix("node:")
{
if deno_node::resolve_builtin_node_module(module_name).is_err() {
if !deno_node::is_builtin_node_module(module_name) {
diagnostics
.push(DenoDiagnostic::InvalidNodeSpecifier(specifier.clone()));
} else if let Some(npm_resolver) = &snapshot.maybe_npm_resolver {
Expand Down
2 changes: 1 addition & 1 deletion cli/lsp/documents.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1090,7 +1090,7 @@ impl Documents {
}
}
if let Some(module_name) = specifier.strip_prefix("node:") {
if deno_node::resolve_builtin_node_module(module_name).is_ok() {
if deno_node::is_builtin_node_module(module_name) {
// return itself for node: specifiers because during type checking
// we resolve to the ambient modules in the @types/node package
// rather than deno_std/node
Expand Down
12 changes: 1 addition & 11 deletions cli/module_loader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ use deno_graph::Module;
use deno_graph::Resolution;
use deno_lockfile::Lockfile;
use deno_runtime::deno_fs;
use deno_runtime::deno_node;
use deno_runtime::deno_node::NodeResolution;
use deno_runtime::deno_node::NodeResolutionMode;
use deno_runtime::deno_node::NodeResolver;
Expand Down Expand Up @@ -496,9 +495,7 @@ impl ModuleLoader for CliModuleLoader {
.shared
.npm_module_loader
.resolve_nv_ref(&module.nv_reference, permissions),
Some(Module::Node(module)) => {
deno_node::resolve_builtin_node_module(&module.module_name)
}
Some(Module::Node(module)) => Ok(module.specifier.clone()),
Some(Module::Esm(module)) => Ok(module.specifier.clone()),
Some(Module::Json(module)) => Ok(module.specifier.clone()),
Some(Module::External(module)) => {
Expand All @@ -517,11 +514,6 @@ impl ModuleLoader for CliModuleLoader {
}
}

// Built-in Node modules
if let Some(module_name) = specifier.strip_prefix("node:") {
return deno_node::resolve_builtin_node_module(module_name);
}

// FIXME(bartlomieju): this is a hacky way to provide compatibility with REPL
// and `Deno.core.evalContext` API. Ideally we should always have a referrer filled
// but sadly that's not the case due to missing APIs in V8.
Expand Down Expand Up @@ -802,8 +794,6 @@ impl NpmModuleLoader {
if let NodeResolution::CommonJs(specifier) = &response {
// remember that this was a common js resolution
self.cjs_resolutions.insert(specifier.clone());
} else if let NodeResolution::BuiltIn(specifier) = &response {
return deno_node::resolve_builtin_node_module(specifier);
}
Ok(response.into_url())
}
Expand Down
6 changes: 0 additions & 6 deletions cli/standalone/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ use deno_core::ModuleType;
use deno_core::ResolutionKind;
use deno_npm::NpmSystemInfo;
use deno_runtime::deno_fs;
use deno_runtime::deno_node;
use deno_runtime::deno_node::analyze::NodeCodeTranslator;
use deno_runtime::deno_node::NodeResolver;
use deno_runtime::deno_tls::rustls::RootCertStore;
Expand Down Expand Up @@ -128,11 +127,6 @@ impl ModuleLoader for EmbeddedModuleLoader {
.resolve_req_reference(&reference, permissions);
}

// Built-in Node modules
if let Some(module_name) = specifier_text.strip_prefix("node:") {
return deno_node::resolve_builtin_node_module(module_name);
}

match maybe_mapped {
Some(resolved) => Ok(resolved),
None => deno_core::resolve_import(specifier, referrer.as_str())
Expand Down
10 changes: 8 additions & 2 deletions cli/tests/testdata/run/extension_dynamic_import.ts.out
Original file line number Diff line number Diff line change
@@ -1,4 +1,10 @@
error: Uncaught (in promise) TypeError: Cannot load extension module from external code
error: Uncaught (in promise) TypeError: Unsupported scheme "ext" for module "ext:runtime/01_errors.js". Supported schemes: [
"data",
"blob",
"file",
"http",
"https",
]
await import("ext:runtime/01_errors.js");
^
at [WILDCARD]/extension_dynamic_import.ts:1:1
at async [WILDCARD]/extension_dynamic_import.ts:1:1
2 changes: 1 addition & 1 deletion cli/tests/testdata/run/extension_import.ts.out
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,4 @@ error: Unsupported scheme "ext" for module "ext:runtime/01_errors.js". Supported
"http",
"https",
]
at [WILDCARD]
at [WILDCARD]/extension_import.ts:1:8
2 changes: 1 addition & 1 deletion cli/tsc/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -538,7 +538,7 @@ fn op_resolve(
};
for specifier in args.specifiers {
if let Some(module_name) = specifier.strip_prefix("node:") {
if deno_node::resolve_builtin_node_module(module_name).is_ok() {
if deno_node::is_builtin_node_module(module_name) {
// return itself for node: specifiers because during type checking
// we resolve to the ambient modules in the @types/node package
// rather than deno_std/node
Expand Down
10 changes: 0 additions & 10 deletions core/extensions.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,16 +471,6 @@ impl Extension {
pub fn disable(self) -> Self {
self.enabled(false)
}

pub(crate) fn find_esm(
&self,
specifier: &str,
) -> Option<&ExtensionFileSource> {
self
.get_esm_sources()?
.iter()
.find(|s| s.specifier == specifier)
}
}

// Provides a convenient builder pattern to declare Extensions
Expand Down
1 change: 0 additions & 1 deletion core/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ pub use crate::module_specifier::resolve_url;
pub use crate::module_specifier::resolve_url_or_path;
pub use crate::module_specifier::ModuleResolutionError;
pub use crate::module_specifier::ModuleSpecifier;
pub use crate::modules::ExtModuleLoader;
pub use crate::modules::ExtModuleLoaderCb;
pub use crate::modules::FsModuleLoader;
pub use crate::modules::ModuleCode;
Expand Down
Loading