Skip to content

Conversation

@CertainLach
Copy link

@CertainLach CertainLach commented Sep 18, 2025

My implementation of multiple import maps in deno (denoland/deno#30754) iterates over import maps to find matches. It is, however, unable to detect if ImportMap::resolve has returned an entry, that should be tried from next import map.

This change allows to only return remapped paths.

This PR also implements import map merging as described by spec: https://html.spec.whatwg.org/multipage/webappapis.html#merge-existing-and-new-import-maps
I can split those changes if that would be better for review purposes

Ref: denoland/deno#30754 (comment)

@CLAassistant
Copy link

CLAassistant commented Sep 18, 2025

CLA assistant check
All committers have signed the CLA.

@CertainLach CertainLach force-pushed the feat/only-remapped-resolve branch from 371f084 to 75f8d48 Compare September 18, 2025 14:06
@CertainLach CertainLach changed the title feat: be able to disable default, non-remapping resolution Import map merging Sep 18, 2025
#[derive(Debug, Clone)]
pub struct SpecifierMap {
#[deprecated = "import map base_url doesn't work with merged import maps"]
base_url: Url,
Copy link
Author

Choose a reason for hiding this comment

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

It seems that base_url be better stored by the side of ImportMaps that are not merged on the application side.


#[deprecated = "specifier map base_url doesn't work with merged import maps"]
#[allow(deprecated)]
pub fn append(&mut self, key: String, value: String) -> Result<(), String> {
Copy link
Author

Choose a reason for hiding this comment

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

...In that case, methods like this be better implemented on SpecifierMapBuilder struct


#[deprecated = "specifier map base_url doesn't work with merged import maps"]
#[allow(deprecated)]
pub fn contains(&self, key: &str) -> bool {
Copy link
Author

Choose a reason for hiding this comment

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

Yet I have no idea what's the good place for this nethod, but it doesn't seem to be used anywhere

@CertainLach
Copy link
Author

CertainLach commented Sep 26, 2025

Might as well postpone deprecation of base_url and related methods, It's just feels wrong to have such api, because all of the deprecated methods doesn't seem to be fit for already constructed import map, especially with how merged and synthetic ones behave.

In case of synthetic import maps, deno already had an error, because workspace synthetic import map had inherited import_url from either deno.json, or from importMap declared in deno.json/passed using --import-map. Second case is invalid, because all the relative paths seem to be rewritten incorrectly when I was testing, and now I'm always setting workspace deno.json, which may not exist, but which makes sense for workspace package resolution.

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