-
Notifications
You must be signed in to change notification settings - Fork 1
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
Redirect module path to exact mod.ts/js index.ts/js file #10
Comments
Continuing roadmap discussion, it is important to understand the reasoning for this change. Also, is this going to be a particular feature of /x/, or a general recommendation for Deno-related CDNs? EDIT My bad, I've just seen that this a dotland repo. A think that it's quite clear what the answer for the last question is. If I'm wrong, please, take a note on that. |
It's not necessarily a recommendation, just an added feature of the deno.land/x registry so people can specify more terse specifiers. Personally, I don't think we shouldn't do index.ts/index.js redirects and instead only have mod.ts/mod.js ones for consistency. |
It seems like a fairly odd choice to provide this sort of magic "index file" functionality, especially when it has an order of precedence and even peeks inside essentially "random" folders like
EDIT: Adding a |
I don't think we should do this at all. What's the point of this? Saving like 7 keystrokes (which are auto-completed anyway)? I do not think that being ambiguous or needlessly implicit is a goal. Magically finding the right file in a directory is as good as dropping file extensions. I wonder what @ry thinks about this? |
I came up with this. It's important to note that this is a deno.land/x feature, not a CLI module system change. It's to save keystrokes. |
I don't think this would be considered magic since |
A plain redirect wouldn't itself be particularly magical but the resolving and precedence of it kind of is. Do you prefer The redirect might not even work: import foo from "https://deno.land/x/foo";
import bar from "https://deno.land/x/bar"; Does the above Or perhaps you choose to resolve in order to one of (leaving out TS/JS changes) Furthermore, this automatic redirection especially if paired with the Deno is also aiming to support loading of NPM modules / Node.js code from local file system. Those will resolve their imports through // In Node..js compat code:
import "./folder"; // resolves to "./folder/index.js" or "./folder.js"
import "library"; // resolves to NPM "library", or to package.json-relative "library.js" or "library/index.js"
// In Deno code
import "https://deno.land/x/lib/entry.ts";
import "https://deno.land/x/lib"; // resolves to "lib/mod.ts" or "lib/index.ts" or "lib/src/index.ts", or fails if none of those exist
import "deno:lib"; // Same as above
import "./folder"; // Likely an error; Node.js compat mode imports cannot be copied over.
import "./folder/index.js"; // Proper import of above.
import "./vendor/lib"; // Above `deno:lib` vendored locally: This now doesn't work, the imports aren't "translatable" from remote to local. NPM of course does this sort of automatic "redirection". I expect it originally simply used If I had to pick and choose one way to go about this, I would introduce some |
I'm curious if you're trying to redirect only the |
Currently, the LSP doesn't like redirects. It spits out a warning and tells us to directly use the correct file to import, rather than relying on the remote logic to do so. Would this change then receive an exception? The LSP somehow needs to know about the module resolution logic of /x and then avoid suggesting to use the target of the redirect. I have an alternative suggestion. If we want to save keystrokes, why not leave the registry the way it is and add better auto-complete logic to the LSP? It could easily check for the existence of the remote files |
Below are my personal opinions.
That's ok. It's up to a module author whether they would like to take advantage of this endpoint or not. It's true though that they might accidentally take advantage of this when they don't want to, so that's why I think it should only be for mod.ts/js since that's what's mostly used on the registry already. It would help drive some standardization.
This is similar to
The folder endpoint would only resolve that way for the Deno user agent from my understanding, so people could use a file that's more explicit for browsers.
That's part of node's resolution and not a feature of the registry. This case is fundamentally different because it doesn't change Deno's module resolution.
Most likely just
My personal opinion is the LSP should still offer this quick fix.
This should probably be done anyway. |
In other words, this issue suggests to implement a feature which the LSP in turn discourages to use. |
Was it not Deno's intention to not standardize anything just for Deno? Like not following Node's footsteps. |
This will not be done. |
Extracted from denoland/deno#17475
Take notice that this is in the dotland repo and is not a change to Deno's module resolution. This is a proposal to create a new endpoint on the registry that does a redirect.
/x/module
redirects to whichever of these exist, ordered by precedence:/x/module/mod.ts
/x/module/mod.js
/x/module/index.ts
/x/module/index.js
/x/module/src/mod.ts
/x/module/src/mod.js
This allows people do leave out the path when importing modules (ex.
import { isOdd } from "deno:is_odd"
)The text was updated successfully, but these errors were encountered: