Skip to content

Conversation

dsherret
Copy link
Member

@dsherret dsherret commented Jun 13, 2025

This further improves import.meta.resolve to not error in many more scenarios (better alignment with Node).

  1. Non-existent files in npm packages
  2. Non-existent built-in node modules (ex. node:non-existent)
  3. Many things that were previously errors with byonm.
  4. No longer surfaces some deno_graph resolution errors

Additionally, this defers resolving npm specifiers until loading for dynamic imports in order to have prepare_load properly install them loading. Before it could potentially error when loading the same npm specifier on multiple workers (reason for flaky specs::npm::worker_shutdown_during_npm_import).

Copy link
Contributor

@marvinhagemeister marvinhagemeister left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Comment on lines +639 to +640
// but the repl may also end up here and it won't have
// a referrer so create a referrer for it here
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

6 years and counting since we can't solve this :sigh:

@@ -647,7 +652,9 @@ impl DenoPluginHandler {
json_module.specifier.clone(),
esbuild_client::BuiltinLoader::Json,
),
deno_graph::Module::Wasm(_) => todo!(),
deno_graph::Module::Wasm(_) => {
bail!("Wasm modules are not implemented in deno bundle.")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nathanwhit are you planning to address this before 2.4?

@bartlomieju bartlomieju merged commit c064b6d into denoland:main Jun 16, 2025
33 of 35 checks passed
@bartlomieju bartlomieju deleted the fix_error_less_import_meta_resolve branch June 16, 2025 10:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants