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

Support imports map in package.json #124

Open
jgoux opened this issue Oct 7, 2023 · 1 comment
Open

Support imports map in package.json #124

jgoux opened this issue Oct 7, 2023 · 1 comment
Labels
core enhancement New feature or request

Comments

@jgoux
Copy link

jgoux commented Oct 7, 2023

Summary

The imports map doesn't seem supported, if I do something like import { t } from "#config/trpc.js"; it doesn't appear in the graph.

Details

Now that we have nodenext in TypeScript, using the imports map is the standard way of aliasing paths internally in a package, so I'd love to have it supported in skott.

Standard questions

Please answer these questions to help us investigate your issue more quickly:

Question Answer
Would you consider contributing a PR? Yes

As a side-note, it seems that they are a lot of overlaps between skott and knip. I'm working on a "future-proof" TypeScript monorepo example, and I think skott would be a great addition to it. I'm also already using knip which is already on point to crawl every files and understand dependencies (including imports map), so maybe there could be some kind of collaboration here? 😄

@jgoux jgoux added the enhancement New feature or request label Oct 7, 2023
@antoine-coulon
Copy link
Owner

antoine-coulon commented Oct 8, 2023

Hello @jgoux, thanks for opening that issue!

You are right, "imports" map feature is not supported by skott's module resolution algorithm yet. In my opinion this is something that should definitely be added in skott and I was kind of waiting for it to be requested before implementing it as often people would use TypeScript path aliases in the first place (which is already mostly supported) 😄

Now that we have nodenext in TypeScript, using the imports map is the standard way of aliasing paths internally in a package, so I'd love to have it supported in skott.

Glad to hear that, it's great to see the beginning of a unified way of having path aliases for JS/TS, that are both supported by LSP and the runtime with no configuration overhead or path rewrites required!

In addition to "imports", there is also "exports" map that counter-intuitively allows to reference path aliases through self-referencing. But this could be implemented later as I think "imports" map is or will become the most common use case for these.

As a side-note, it seems that they are a lot of overlaps between skott and knip...

I was completely unaware of that tool, it looks great and already widely adopted, thanks for sharing! Could you please elaborate on what is currently implemented in knip that you wish would be supported by skott? I can see that there is indeed some feature overlap, mostly on overall graph resolution and "unused" dependencies. To be honest, skott's "unused" feature surface is pretty small actually, because the idea I have for skott is to be more like a universal graph primitive (only JS/TS supported yet but could be expanded to whatever lang) on which you could visualize / build tools relying on graph data structures, rather being a specialized tool for "unused" cases as I already consider it as a specialized domain, hard to get it right with all the different cases out there.

Consequently I think that skott should and will delegate some of its features to other tools at some point (started a little bit with depcheck for unused dependencies) and because knip seems already mature and focused on "unused" features for JS/TS, this is something that could entirely benefit to skott as it's hard to implement and I don't want to reinvent the wheel again and again for these specific and hard subjects.

Note also that this kind of feature was initially requested a while ago by @ild0tt0re in #13, and I initially thought of using something like ts-prune to do this work, but given knip is also supporting native JS I would favor it for sure.

My biggest concern is about the integration, because both skott and knip are dealing with their own internal graph, as you mentioned they are both crawling it down, and introducing two sources of truth can be both tedious and tricky if they are not traversing/inspecting projects the same way, it could lead to desynchronization between data collected by one or the other but also performance drawbacks as both would end up building their own graphs.

But this is something we can definitely think about and try out as you are contributing to knip (and now skott :) ), we might be able to achieve great things and make both tools even better!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants