Skip to content

make sloppy imports configurable#149

Open
p-hoffmann wants to merge 1 commit intomainfrom
p-hoffmann/simports
Open

make sloppy imports configurable#149
p-hoffmann wants to merge 1 commit intomainfrom
p-hoffmann/simports

Conversation

@p-hoffmann
Copy link
Copy Markdown
Member

No description provided.

Copilot AI review requested due to automatic review settings March 28, 2026 05:32
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

Makes “sloppy imports” (extension/index resolution) behavior configurable for standalone/eszip module loading, instead of always enabling it.

Changes:

  • Adds a sloppy_imports flag to WorkspaceEszip and gates extension/index fallback lookups behind it.
  • Plumbs a new sloppy_imports parameter through create_module_loader_for_eszip / create_module_loader_for_standalone_from_eszip_kind.
  • Reads unstableSloppyImports from the runtime context and passes it into the standalone module loader/resolver configuration.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
crates/deno_facade/module_loader/standalone.rs Adds a sloppy_imports toggle to control extension/index fallback in WorkspaceEszip::get_module, and threads the flag into workspace resolver creation.
crates/base/src/runtime/mod.rs Extracts unstableSloppyImports from runtime context and forwards it into standalone module loader creation.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

let sloppy_imports = context
.get("unstableSloppyImports")
.and_then(|v| v.as_bool())
.unwrap_or(false);
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

sloppy_imports is derived only from context["unstableSloppyImports"] and defaults to false, but DenoOptions::unstable_sloppy_imports() falls back to deno.json / workspace unstable flag (has_unstable("sloppy-imports")). This makes runtime module loading inconsistent with the options used to build/resolve the workspace and can disable sloppy imports even when the workspace config enables them. Consider deriving the value from the computed DenoOptions (or otherwise preserving the tri-state) instead of defaulting to false when the context key is absent.

Suggested change
.unwrap_or(false);
.unwrap_or_else(|| flags.has_unstable("sloppy-imports"));

Copilot uses AI. Check for mistakes.
Comment on lines 740 to +754
@@ -746,6 +751,7 @@ where
}),
Some(base_dir_path.to_string_lossy().as_ref()),
should_block_fs || flags.restrict_host_fs,
sloppy_imports,
Copy link

Copilot AI Mar 28, 2026

Choose a reason for hiding this comment

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

The new unstableSloppyImports plumbing changes runtime resolution behavior but doesn’t appear to be covered by the existing tests in this module. Adding a test that verifies imports resolve with/without extensions depending on the context flag (and/or workspace unstable setting) would help prevent regressions.

Copilot uses AI. Check for mistakes.
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.

2 participants