feat(unstable): optional deno_modules directory

[dsherret]

Overview: #15633 (comment)

This doesn't yet implement the LSP mapping the remote files to the local ones when doing stuff like "go to definition".

Closes #15633

[marvinhagemeister]

Tried it out locally and it works great so far with the deno run command. Couldn't get it to work with deno task or deno test. Both of these commands seem to ignore the "remoteModulesDir": true setting.

There are a bunch of proxy files from esm.sh which don't seem to have an extension. Not sure if it's possible but do they give any hints for which file type that is? That would make the editor experience a bit nicer.

[dsherret]

Tried it out locally and it works great so far with the deno run command. Couldn't get it to work with deno task or deno test. Both of these commands seem to ignore the "remoteModulesDir": true setting.

@marvinhagemeister I suspect that maybe something under the hood is spawning a new Deno process, which then uses the one defined globally. Maybe try replacing your global deno version?

There are a bunch of proxy files from esm.sh which don't seem to have an extension. Not sure if it's possible but do they give any hints for which file type that is? That would make the editor experience a bit nicer.

I updated these to have a file extension on them now.

[nayeemrmn]

We need to handle URLs which normalize to the same path.

Demo

Run this server:

// deno run -A server.ts
Deno.serve((r) => {
  const path = new URL(r.url).pathname;
  console.log(path);
  if (path == "/hello.ts") {
    return new Response(
      'console.log("Hello!");\n',
      { headers: { "Content-Type": "application/typescript" } },
    );
  } else if (path == "//hello.ts") {
    return new Response(
      'console.log("Hello with an extra leading slash!");\n',
      { headers: { "Content-Type": "application/typescript" } },
    );
  }
  return new Response(null, { status: 404 });
});

And then this main module with "remoteModulesDir": true:

// cargo run -- run main.ts
import "http:https://localhost:8000/hello.ts";
import "http:https://localhost:8000//hello.ts";
// Expected output:
// Hello!
// Hello with an extra leading slash!

Currently the first run will look as expected, though the content of //hello.ts overrides the content of /hello.ts in the remote modules dir. The second run will hit a lock file error. With --no-lock you get Hello with an extra leading slash! twice.

Rather than trying to come up with a readable but 1-to-1 path mapping, we could potentially put the manifest to use and just add _{n} or something to duplicates like this.

But what happens if the first import "http:https://localhost:8000/hello.ts" gets removed? We don't want the mapping for the second import to suddenly change to an un-suffixed one. For that reason the mapping function as a first step should check for an existing mapping for the given URL. That will also let us freely make changes to the mapping function in the future.

So something like:

fn url_to_local_sub_path(url: Url, manifest: Manifest): PathBuf {
  if let Some(path) = manifest.get_path(&url) {
    return path;
  }
  let default_path = base_mapping_function(&url);
  let mut path = default_path.clone();
  let mut id = 1;
  while manifest.has_path(&path) {
    path.set_file_name(format!("{}_{}", default_path.file_stem().unwrap(), id));
    if let Some(extension) = default_path.extension() {
      path.set_extension(extension);
    }
    id = id + 1;
  }
  return path;
}

[dsherret]

@nayeemrmn thanks! I fixed that.

[nayeemrmn]

Oh cool, that was the only example I tried so I didn't realise there was already a mechanism

[bartlomieju]

Looks mostly good to me! I have a few questions, but overall super cleanly implemented - the bulk of the PR is boilerplate and changing expected types, which is a great indication that your factory approach to various parts of the CLI is working well 👍

[bartlomieju]

LGTM, great work. I literally have one question left