deno icon indicating copy to clipboard operation
deno copied to clipboard

[lsp] `deno.jsonc` option to disable `import-map-remap`

Open loynoir opened this issue 3 years ago • 11 comments

Related https://github.com/denoland/deno/issues/15266

I tried // deno-lint-ignore import-map-remap, seems not working.

loynoir avatar Feb 22 '23 06:02 loynoir

// deno-lint-ignore comment applies to deno_lint rules. https://lint.deno.land/

petamoriken avatar Feb 22 '23 09:02 petamoriken

@petamoriken Yes, seesm there is no option to let the misbehave import-map-remap gone.

loynoir avatar Feb 22 '23 09:02 loynoir

This is a deno diagnostic and not a deno lint diagnostic. I don't think we should have an option for this, but rather fix the underlying issue with the diagnostic or remove it.

dsherret avatar Feb 22 '23 16:02 dsherret

but rather fix the underlying issue with the diagnostic or remove it.

I could revive #15267 which covers a lot of undesired cases, but I think it would be better to remove it. With certain types of import maps (relative-to-relative) it's impossible to know if the mapped or unmapped variant is preferred. And the 'reverse lookup' functionality it depends on is non-spec, potentially impossible to define correctly and would always be error prone. I don't think people are looking for this feature either. It was specifically implemented as a hint for the first reason, but it's been shown that people still treat that as a nagging error.

Same with https://github.com/denoland/import_map/pull/40, rather than fixing it we should remove the non-standard and error prone ImportMap::lookup() from such an otherwise spec-guided crate.

nayeemrmn avatar Mar 12 '23 19:03 nayeemrmn

@nayeemrmn We should fix this feature, not remove it. If I am inside of a Fresh project, and I want to import useState, I want to import it from preact/hooks, not https://esm.sh/~/preact/2187312i3jbnjh237dh172y3h217sdj1238jkd/hooks/index.js?foo=bar32. I consider this is the #1 issue with the LSP at this moment.

The reverse lookup functionality in ImportMap should be improved, not removed.

lucacasonato avatar Mar 15 '23 11:03 lucacasonato

I guess I'm struggling to understand the workflow surrounding it. The diagnostic and code action come into effect if you input that URL in the first place, I speculated it wasn't useful because users would just start with preact/hooks if they configured it. Is the URL the result of an autocomplete?

nayeemrmn avatar Mar 15 '23 13:03 nayeemrmn

Is the URL the result of an autocomplete?

Yes, if you have an import map and you select suggested autocomplete the LSP will put the full URL instead of the import mapped specifier.

I believe this is covered by https://github.com/denoland/deno/issues/15330

bartlomieju avatar Mar 15 '23 13:03 bartlomieju

I have "~/": "./" in my importmap.json and the LSP thinks I want to change ./fresh.gen.ts to ~/fresh.gen.ts and ./Button.tsx to ~/src/components/Button.tsx. So if you use project root for absolute imports as suggested by the official manual, it’s basically impossible to do any relative imports without triggering this problem.

alex-kinokon avatar Apr 23 '23 22:04 alex-kinokon

#20539 doesn't seem to fix this issue. example:

image

i would love 🙏 if we could disable the squiggle.

spence avatar Oct 24 '23 18:10 spence

apparently, the squiggle issue doesn't seem to appear if the import is in a type expression. so if you want to remove it, you can write this (very messed up) workaround expression

await import("../value.ts" as string) as typeof import("../value.ts")

this removes the squiggle from the actual import expression by turning it into a dynamic import, then asserts the type to "put the type back"

bradthomasbrown avatar Aug 15 '24 19:08 bradthomasbrown

Adding to the ~/ use case, I would prefer relative imports if both source and target file shares the same common path, e.g. ~/lib/foo.ts importing ~/lib/bar.ts.

Relative imports are generally desired when a sub-directory is meant to be copypasted to other projects, so technically Deno may suggest monorepo to shave this use case from the issue.

vicary avatar Oct 24 '24 04:10 vicary

I'm going to be frank, the whole concept of hints inherited from eslint is broken.

  1. When you have something yelling at you, you will be inclined to "fix" it to remove distractions that keep you from understanding if your code is actually written correctly.

  2. eslint disables applying fixes form the command line for hints. For this not in a supported IDE means there is no way to work with hints in some meaningful way. My warning in the bigger picture: if you want to constrain adoption of deno to vscode users this is a good path to follow with eslint.

  3. deno is implying that relative paths are not valid in native javascript es modules. This will catch those in the worst way who are uneducated and new.

I personally have converted my project to the mapped imports to remove the warnings, killing a lot of the maintainability. There is just no other way to work around this.

gmzacharydovel avatar Mar 09 '25 15:03 gmzacharydovel

There is a lot of passive aggressive behavior on this from the coc-deno maintainer. The behavior seems to be that this is obvious, but when you search for what they are talking about nothing pops up in any documentation anywhere. They'll also tell you to use a non-existing ':h' command. It's all really weird.

gmzacharydovel avatar Mar 13 '25 13:03 gmzacharydovel

If you're using an editor where you rely on the "problems pane", these local imports basically make using local imports unusable. Please have a look at the screenshot below.

Image

gmzacharydovel avatar Mar 20 '25 11:03 gmzacharydovel

I would describe the current situation with Deno as a sort of absolute import hell. While still better than the relative one, it isn't great. I'm trying to think of how I can restructure my project to reduce this, but my screenshot below shows how it really makes programming unreadable.

Image

gmzacharydovel avatar Mar 20 '25 13:03 gmzacharydovel

From my searches on the internet, there is a wide selection of groups that do not want responsibility for this. Deno doesn't want a way to disable this. The people doing vim plugins don't want responsibility.

This is really really bad. I literally can't see actually programming errors because of this in VIM in my problems tray. I already has this conversation in the coc github, but engineers that don't use the problems panel tend to not output quality work. It isn't appropriate for professional work.

Can I do a PR on this? This is important enough.

gmzacharydovel avatar Mar 20 '25 13:03 gmzacharydovel

I'll resurrect https://github.com/denoland/deno/pull/15267

dsherret avatar Mar 20 '25 13:03 dsherret

To make the context clear, this is the output of the problems pane in VIM versus vscode.

I can poke the maintainers of vim plugins that integrate with the Deno language server. But they all seem to be including this in the problems pane, it it would mean talking to about 5 or more projects with no way to turn this off. Their take in general is that warnings should to not be silence-able so it really reduces options in general for dealing with this.

Image Image

gmzacharydovel avatar Mar 20 '25 14:03 gmzacharydovel

I would prefer relative imports if both source and target file shares the same common path, e.g. ~/lib/foo.ts importing ~/lib/bar.ts.

This was done in #28560. If you run deno upgrade canary in about 30 minutes the fix should be available (you can verify the version after upgrading with deno --version to see which commit it's for).

dsherret avatar Mar 20 '25 15:03 dsherret