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

Import map expansion doesn't work in LSP #22270

Closed
dsherret opened this issue Feb 5, 2024 · 5 comments · Fixed by #22553
Closed

Import map expansion doesn't work in LSP #22270

dsherret opened this issue Feb 5, 2024 · 5 comments · Fixed by #22553
Assignees
Labels
bug Something isn't working correctly lsp related to the language server

Comments

@dsherret
Copy link
Member

dsherret commented Feb 5, 2024

{
  "imports": {
    "preact": "npm:preact@10.5.13"
  }
}
import * as preact from "preact";
import * as hooks from "preact/hooks"; // error only in lsp

console.log(preact, hooks);

Version: Deno 1.40.3

@dsherret dsherret added bug Something isn't working correctly lsp related to the language server labels Feb 5, 2024
@lucacasonato
Copy link
Member

lucacasonato commented Feb 5, 2024

It also doesn't work in deno publish I think unrelated issue

@guybedford
Copy link
Contributor

If the import map is defined only for preact, expansion / inference of preact/hooks seems to violate the import maps spec? Or is the idea to bend the semantics to better support packaging workflows in this way?

@dsherret
Copy link
Member Author

dsherret commented Feb 8, 2024

Sorry, wasn't clear in the issue. This is only for the embedded import map in deno.json (not if referenced via "importMap": "..."), which is an extension to the import map spec. It only occurs for jsr and npm imports because it was an annoyance to have two entries (#22087)

@guybedford
Copy link
Contributor

Thanks for sharing the context, I missed that. I guess this only makes sense because the targets are not so much final URLs as they are intermediate package specifiers which then get further resolution processing applied. If browsers were ever to adopt the same pattern they would effectively rely on /dir serving the main entry point and /dir/module serving submodules, while while very unlikely to ever be viable, doesn't seem entirely unreasonable either, so it seems an interesting extension to explore. I would be careful of extending the spec much further at all though, effectively these are already two extensions which simply won't be supported by Node.js (partial resolution into intermediate specifier targets and automatic subpathing expansion).

@dsherret
Copy link
Member Author

@nayeemrmn I'll take this one on by making this happen at a lower level.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working correctly lsp related to the language server
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants