feat(cli): provide Deno.info() API #10758

kitsonk · 2021-05-25T07:51:09Z

Resolves #2096

This is a work in progress PR. There are a couple of items still to address:

There should be a permission check for read permission to the DENO_DIR to access the API.
The API should support import-maps, like Deno.emit(), but no one is happy with the Deno.emit() solution, so we need to debate the best way to allow an import-map to be provided.
It needs tests.

I have implemented the import map in a more straight forward way and my opinion is that we should change Deno.emit() to follow. In addition I added two options that are off by default that suppress some sensitive data from being returned and documented the whole security implications of using the API. Flexing all the features looks something like this:

const info = await Deno.info("./test.ts", {
  checksums: true,
  importMap: "./import_map.json",
  paths: true,
});

console.log(info);

Or with an import object literal, something like this:

const info = await Deno.info("./test.ts", {
  importMap: {
    imports: {
      jquery: "https://cdn.skypack.dev/jquery",
    },
  },
});

console.log(info);

crowlKats · 2021-05-25T07:56:33Z

Shouldn't it also support usage without a specifier, just like the info subcommand?

kitsonk · 2021-05-25T07:58:14Z

Shouldn't it also support usage without a specifier, just like the info subcommand?

I'm not personally convinced of that. What would be a legitimate use case for having that type of information available programatically?

crowlKats · 2021-05-25T08:03:40Z

I'm not personally convinced of that. What would be a legitimate use case for having that type of information available programatically?

One use case is together with #10589, so one could get access to the location of the localstorage file (and other apis that would store data based on the origin)

nayeemrmn · 2021-05-25T10:32:51Z

What's your opinion on #10628, and maybe representing that change in advance with the runtime API? It would solve needing read permission to DENO_DIR when you just want a module graph and no cache info, so this is another justification IMO.

kitsonk · 2021-05-25T11:25:57Z

so one could get access to the location of the localstorage file (and other apis that would store data based on the origin)

Knowing a location isn't a use case... 😄 What problem does that solve having that information available via an API.

What's your opinion on #10628, and maybe representing that change in advance with the runtime API?

Maybe. I get the reasoning for it, but it complicates the API surface and as noted in the issue, it isn't something to be considered until 2.0 and I would then expect the Deno APIs to reflect the subcommands. My opinion is we shouldn't wait until 2.0 to introduce this API (it already has quite a few real world use cases, especially with those using Deno as an on the fly transpiler/bundler).

crowlKats · 2021-05-25T11:32:45Z

Knowing a location isn't a use case... 😄 What problem does that solve having that information available via an API.

Well one example would be that one could make a backup by copying the file (useful for indexeddb)

SyrupThinker · 2021-05-27T12:03:11Z

Shouldn't it also support usage without a specifier, just like the info subcommand?

I'm not personally convinced of that. What would be a legitimate use case for having that type of information available programatically?

I have an example use-case with Docuraptor.
It uses deno info to get the module cache directory to then aggregate a list of all locally cached modules.
This is used to provide a documentation index for said modules.

nayeemrmn · 2021-05-27T12:48:13Z

I think any info provided under deno info [mod.ts] has a use case in meta kind of tooling.

However I wouldn't be surprised if the runtime equivalents for deno info and deno info mod.ts were different functions. There is zero intersection in both arguments and return types. Other than the sheer justification that they are the same on the CLI, or that return type overloading is allowed by TS, I'm not a fan of keeping disjoint functions under the same name. We should consider having both Deno.info() and Deno.moduleInfo().

bartlomieju

Sorry for slow review @kitsonk, I completely missed the request 🤦

I have only minor comments

bartlomieju · 2021-06-30T01:08:54Z

cli/dts/lib.deno.unstable.d.ts

+     * In order to resolve relative modules, import maps need a "base" which is
+     * the import map itself. When an object is passed as a value the current
+     * working directory for the Deno process will be used. */
+    importMap?: string | URL | ImportMap;


In case this is ImportMap type - what is the "base URL" used for that import map?

bartlomieju · 2021-06-30T01:09:38Z

cli/dts/lib.deno.unstable.d.ts

+   * Similar to the command line functionality `deno info --json`, the API
+   * provides information about a module and its dependencies.
+   *
+   * Requires `allow/read` permissions to access local modules and cached


Nitpick: change allow/read to --allow-read to keep consistent with other docs?

bartlomieju · 2021-06-30T01:10:41Z

cli/dts/lib.deno.unstable.d.ts

+   *   checksums: true,
+   *   paths: true,


Why not provide these things by default?

bartlomieju · 2021-06-30T01:16:42Z

cli/ops/runtime_compiler.rs

+  #[serde(default)]
+  checksums: bool,
+  import_map: Option<StringOrImportMap>,
+  #[serde(default)]


Consider adding Default to derive instead?

bartlomieju · 2021-06-30T01:17:45Z

cli/ops/runtime_compiler.rs

+  let custom_root = std::env::var("DENO_DIR").map(String::into).ok();
+  let deno_dir = DenoDir::new(custom_root)?;


Skip reading env variable and use dir field from ProgramState instead?

bartlomieju · 2021-06-30T01:19:13Z

cli/ops/runtime_compiler.rs

+    match &options.import_map {
+      Some(StringOrImportMap::ImportMap(value)) => {
+        let cwd = std::env::current_dir()?;
+        let base_url = resolve_path(cwd.to_string_lossy().as_ref())?;


To answer myself to previous question - in case of passing "object" import map it uses CWD as default URL, could you please add it to the JSDoc on the API?

timreichen · 2021-07-23T05:27:28Z

Just as an input: instead of a isDynamic in ModuleGraphDependency, maybe a dependencyType would be better.

enum DependencyType {
  Import,
  Export,
  DynamicImport
}

I ran into problems having just a dynamic flag when creating bundler. If one wants to add "Fetch" or some other type in the future, DependencyType can easily be extended.

kitsonk · 2021-08-24T10:27:51Z

Closing due to https://deno.land/x/deno_graph/ being available.

nayeemrmn mentioned this pull request May 25, 2021

Deno.emit() without net permission silently excludes remote dependencies #10761

Closed

kitsonk force-pushed the feat-deno-info branch from 0835d85 to 0aad9cc Compare May 26, 2021 04:49

kitsonk marked this pull request as draft May 26, 2021 06:27

kitsonk added 4 commits June 22, 2021 15:27

feat(cli): provide Deno.info() API

a7caad8

check deno cache read access

a930dad

add import map and options

fe6feae

add tests

38ffb5a

kitsonk force-pushed the feat-deno-info branch from 0aad9cc to 38ffb5a Compare June 22, 2021 05:27

kitsonk changed the title ~~[WIP] feat(cli): provide Deno.info() API~~ feat(cli): provide Deno.info() API Jun 22, 2021

kitsonk requested a review from bartlomieju June 22, 2021 05:28

kitsonk marked this pull request as ready for review June 22, 2021 05:28

fix doc example

151ad5a

kitsonk requested a review from ry June 22, 2021 11:52

bartlomieju reviewed Jun 30, 2021

View reviewed changes

kitsonk closed this Aug 24, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cli): provide Deno.info() API #10758

feat(cli): provide Deno.info() API #10758

kitsonk commented May 25, 2021 •

edited

crowlKats commented May 25, 2021

kitsonk commented May 25, 2021

crowlKats commented May 25, 2021

nayeemrmn commented May 25, 2021

kitsonk commented May 25, 2021

crowlKats commented May 25, 2021

SyrupThinker commented May 27, 2021

nayeemrmn commented May 27, 2021

bartlomieju left a comment

bartlomieju Jun 30, 2021

bartlomieju Jun 30, 2021

bartlomieju Jun 30, 2021

bartlomieju Jun 30, 2021

bartlomieju Jun 30, 2021

bartlomieju Jun 30, 2021

timreichen commented Jul 23, 2021 •

edited

kitsonk commented Aug 24, 2021

		let custom_root = std::env::var("DENO_DIR").map(String::into).ok();
		let deno_dir = DenoDir::new(custom_root)?;

feat(cli): provide Deno.info() API #10758

feat(cli): provide Deno.info() API #10758

Conversation

kitsonk commented May 25, 2021 • edited

crowlKats commented May 25, 2021

kitsonk commented May 25, 2021

crowlKats commented May 25, 2021

nayeemrmn commented May 25, 2021

kitsonk commented May 25, 2021

crowlKats commented May 25, 2021

SyrupThinker commented May 27, 2021

nayeemrmn commented May 27, 2021

bartlomieju left a comment

Choose a reason for hiding this comment

bartlomieju Jun 30, 2021

Choose a reason for hiding this comment

bartlomieju Jun 30, 2021

Choose a reason for hiding this comment

bartlomieju Jun 30, 2021

Choose a reason for hiding this comment

bartlomieju Jun 30, 2021

Choose a reason for hiding this comment

bartlomieju Jun 30, 2021

Choose a reason for hiding this comment

bartlomieju Jun 30, 2021

Choose a reason for hiding this comment

timreichen commented Jul 23, 2021 • edited

kitsonk commented Aug 24, 2021

kitsonk commented May 25, 2021 •

edited

timreichen commented Jul 23, 2021 •

edited