denoland / deno_graph Goto Github PK

I don't believe we should need this kind of code when resolving on a resolved graph:

Resolving once the graph is resolved can be one step to the destination for performance reasons. I don't believe storing all the steps is necessary. If it's infinite, then we should store that in the graph as an error at the redirect.

When validating a graph and reaching a type branch, current behaviour descends into the children imports, treating them as code imports, and therefore erroring when there is a type branch only error, which it shouldn't.

See: denoland/deno#15295

I've had to look at the deno_graph source code or run the code to understand what the key values and tuples parts are. We should add some documentation comments describing what the non-descriptive parts of the Module interface mean (and perhaps in the future we should switch to a more descriptive interface).

There are actually just two properties that could be easily improved with some docs.

It would be great if the graph builder allowed one to slot in a ProgressReporter into the graph builder that would be used to report graph building progress to the caller. How I imagine this working like so:

struct GraphLoadStats {
  /// The number of unique module specifiers that have been seen by the graph,
  /// that are done loading and parsing.
  completed_modules: usize,
  /// The number of unique module specifiers that have been seen by the graph,
  /// including ones where loading is still "in progress".
  seen_modules: usize,
}

trait ProgressReporter {
  /// This function is called after each module is done loading and parsing.
  fn on_load(&self, specifier: &Url, stats: GraphLoadStats);
}

This allows for progress indicators to display the progress of building the graph.

Was talking to @bartlomieju today so this is a quick summary of the discussion. For denoland/deno#12648 we'll need a way to identify if .js/MediaType::JavaScript files are CJS or MJS. This can be determined a number of ways in node (ex. a package.json having say "type": "module") which is information that's known by the node compat resolvers in the CLI. Perhaps it would be useful to introduce a ModuleType enum that has members ModuleType::Cjs & ModuleType::Mjs and the Resolver.resolve method could return that value which would then be stored in the graph.

To unblock denoland/deno#2552, deno_graph needs to be able to represent WASM modules in a module graph. To do this, we need to parse out the import sections in a module, because with the ESM-WASM integration, these can reference other JS / WASM modules (and thus they need to be part of the module graph).

To do the analysis of the WASM modules we can probably use wasmparser. It is a streaming parser, so we don't waste compute cycles and memory getting an entire WASM AST just to extract the imports.

tl/dr minimal reproduction lives here https://github.com/deckchairlabs/deno-loading-unprepared-module-error

I'm receiving the following error error: Loading unprepared module: file:///workspace/deno.json, when trying to do the below:

Note removing the compilerOptions related to jsx, the code runs without issue.

main.ts

import config from "./deno.json" assert { type: "json" };
console.log(config);

deno.json

{
  "compilerOptions": {
    "jsx": "react-jsx",
    "jsxImportSource": "react"
  },
  "importMap": "./importMap.json"
}

importMap.json

{
  "imports": {
    "react": "https://esm.sh/react@18",
    "react/jsx-runtime": "https://esm.sh/react@18/jsx-runtime.js"
  }
}

swc already supports parsing this out (we just simply discard those nodes at the moment), but I think the best way we can identify them in the graph is to add a is_require to the resolved dependency and pass it in the load() method, like we do with is_dynamic.

I will work on a PR for it. Makes me feel a bit 🤢 though 😉.

The graph should allow a loader to represent a load request with an External and BuiltIn load result. These would become the ModuleKind for the module slot and no dependency analysis would be performed. This would be an indictor to implementors that these require special loading at runtime.

Hello!

I'd like to be able to cache the WASM binary locally, yet deno_graph doesn't provide a away to init the module with a custom path/url.

// temp.ts
import "./nonexistent.ts";
await import("./nonexistent.ts");

deno cache temp.ts gives no error, since the last import was dynamic and overrides Dependency::is_dynamic.

Modules returned by createGraph seem to have an undefined local property. Is there a reason it is on the type def if it's known to be unimplemented?

  /** The string path to where the local version of the content is located. For
   * non `file:` URLs, this is the location of the cached content, otherwise it
   * is the absolute path to the local file. */
  local?: string;

As the deno version is upgraded to 1.14.0, an error occurs in the following module, I wonder why this module was installed.

❯ deno run ...blabla
Check file:///...blabla
error: TS2801 [ERROR]: This condition will always return true since this 'Promise<Deno.PermissionStatus>' is always defined.
  if (Deno.permissions.query({ name: "env" })) {
      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    at https://deno.land/x/[email protected]/is_interactive.ts:2:7

TS2773 [ERROR]:     Did you forget to use 'await'?
      if (Deno.permissions.query({ name: "env" })) {
          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
        at https://deno.land/x/[email protected]/is_interactive.ts:2:7

The deno info command didn't give me the info I needed.

❯ deno info https://deno.land/x/[email protected]/is_interactive.ts
local: /Users/me/Library/Caches/deno/deps/https/deno.land/26e835f96821d84b7a10ed84ee8bf125c1bf7bbdbf8f1cfc5cd8b03d96a83ae0
type: TypeScript
emit: /Users/me/Library/Caches/deno/gen/https/deno.land/26e835f96821d84b7a10ed84ee8bf125c1bf7bbdbf8f1cfc5cd8b03d96a83ae0.js
dependencies: 0 unique (total 275B)

https://deno.land/x/[email protected]/is_interactive.ts (275B)

As title.

Might be worth looking into console-error-panic-hook or similar to actually show messages in unreachable!()s

Ref denoland/deno#12805 (comment).

  pending_assert_types: HashMap<ModuleSpecifier, HashSet<Option<String>>>,

https://github.com/denoland/deno/blob/1eb78731eb57b1d0eb7c0ece97b2018c1724989e/cli/lsp/documents.rs#L1025-L1059

This is where it goes wrong. Because the dependencies list is empty, it fails to resolve the imported module.
I think think it might be possible to fix this without making changes to swc.
If you add a regular import at the top of a file, that fixes the jsdoc import:

import {Bar} from "./Bar.js";

/** @type {import("./Bar.js").Bar} */
const foo = null;

Originally posted by @jespertheend in https://github.com/denoland/vscode_deno/issues/588#issuecomment-998330173

Breaking change for me when trying to upgrade from 0.5 to 0.11.1

It is referenced in ModuleGraph (which is exported) toJSON()

When analyzing sources, any "upstream" source maps should be identified and become part of the module graph. This would allow consumers of the graph information to be able to easily identify any upstream source maps and consume them.

This would help enable denoland/deno#4499

Currently the graph cannot represent multiple locations an import specifier might be imported in a file. For example, an import can be like this:

import * as a from "https://example.com/a";
import { default as b } from "https://example.com/a";

In addition, in theory, a module can be statically and dynamically imported:

import * as a from "https://example.com/a";
const b = await import("https://example.com/a";

Or in TypeScript the same specifier can be provided for a type only import:

import { a } from "https://example.com/a";
import type { A } from "https://example.com/a";

Currently we only store the last range for the specifier in the graph. This makes it difficult when trying to rewrite specifiers (like in dnt or deno vendor) as well as means that diagnostics are only issued for the "last" import location, not all import locations for a given specifier.

We should enhance the graph to be able to store this information, which is likely going to be a breaking change, but one that increases the usability of the graph.

I just wonder when I see on forked deno_graph , I can't see create_graph function .
confused sorry for this question .

Another question I have , is , shall we add one element in Module struct or Specifier our absolute remote Url which may change on requesting and when we want to vendor it , it doesn't build with main package name and it could be strange name with path . I have just PR on deno which resolved this issue but wonder if we can create vendored path with main package name in importing . appreciate to help out and permit me to do it.

There exists TS2822 [ERROR]: Import assertions cannot be used with type-only imports or exports., so we shouldn't check assertions for type-only imports. denoland/deno#14913

Something is going on with the publishing on tagging: https://github.com/denoland/deno_graph/runs/7722568935?check_suite_focus=true#step:14:165

Given a module I would like to know all its deno-types references. Is there a way to do this without having to traverse the AST to get the module specifiers? I think this is only currently possible in Wasm since in the module.dependencies it exposes maybe_type and maybe_code for Wasm, but I didn't see a way to do this in Rust.

I want to do this because I would like to know up front all the /// @deno-types="..." that a file has in order to find out the declaration files used for a module (for the purposes of the deno -> node transform).

Ref #32.

Currently deno_graph has strong opinions about how type dependencies are resolved when building a graph, but this ability should be extended.

Specifically, a module has a maybe_types_dependency and this gets determined one of two ways:

• The file is a JavaScript file and it contains a leading triple-slash reference types directive.
• When loading a module, the module contains a header of X-TypeScript-Types

But other resolution algorithms maybe have other semantics, for example:

• A package.json is inspected and there is a "typings" property.
• When looking at files on the disk, there is a desire to resolve a .d.ts file and a .js file for an import.
• It is determined that a @types dependency is available locally or remotely.

I want to do a little bit more thinking about it, so we don't just "hack" it in there, but I think we need to come up with a solution, but I am currently thinking that the Loader trait would have a new optional method:

• fn resolve_types(&self, specifier: &ModuleSpecifier) -> Result<Option<ModuleSpecifier>>

It would only get called if the media type for the module was not TypeScript or TSX and the built-in logic did not identify a types dependency. It would be called with the media specifier returned from the load result, so in the case of a redirect, it would call with the final specifier.

cc/ @bartlomieju and @dsherret

When running e.g. deno info https://deno.land/std/http/mod.ts, I get

Download https://deno.land/std/http/mod.ts
Warning Implicitly using latest version (0.112.0) for https://deno.land/std/http/mod.ts
Download https://deno.land/[email protected]/http/mod.ts
Download https://deno.land/[email protected]/http/cookie.ts
Download https://deno.land/[email protected]/http/http_status.ts
Download https://deno.land/[email protected]/http/server.ts
Download https://deno.land/[email protected]/_util/assert.ts
Download https://deno.land/[email protected]/datetime/mod.ts
Download https://deno.land/[email protected]/async/mod.ts
Download https://deno.land/[email protected]/datetime/formatter.ts
Download https://deno.land/[email protected]/async/deadline.ts
Download https://deno.land/[email protected]/async/debounce.ts
Download https://deno.land/[email protected]/async/deferred.ts
Download https://deno.land/[email protected]/async/delay.ts
Download https://deno.land/[email protected]/async/mux_async_iterator.ts
Download https://deno.land/[email protected]/async/pool.ts
Download https://deno.land/[email protected]/async/tee.ts
Download https://deno.land/[email protected]/datetime/tokenizer.ts
error: an internal error occured

and @lucacasonato seemed to be able to reproduce it as well. The same happens for other std modules with that URL pattern. The CLI returns with a 0 exit code though.

I am on 1.15.2 but cannot say for sure if it was just introduced by that patch or not.

Very hard to understand without digging into the source:

type MemoryLoaderSources<S> =
  Vec<(S, Result<(S, Option<Vec<(S, S)>>, S), Error>)>;

I think lines and columns is an LSP & display concern and we should remove it from deno_graph. I think deno_graph should just return byte indexes or at least have both. It would also be good to standardize any unit types with deno_ast (so probably just use swc's Span for ranges... or maybe we could make our own type that uses usize and easily converts to an swc span)

discussion_r767983595

Ok suppose I've got parseModule/createGraph resolving modules correctly. How can I now execute the module?

const m = await denoGraph.parseModule("http://example.ts", "export default { foo: true }")

const { foo } = // ??
console.log(foo)

This works:

/** @type {import("./Foo.js").Foo?} */
const x = null;

But this doesn't:

/** @type {Set<import("./Foo.js").Foo>} */
const x = new Set();

error: TS2694 [ERROR]: Namespace '__' has no exported member 'Foo'.
/** @type {Set<import("./Foo.js").Foo>} */
                                  ~~~

Reduced test case here: jsdocimport.zip
Simply run deno run mod.js

I suppose this is an issue with deno_graph just like with denoland/deno#13156
ref: #91 #92

Currently, when building a graph, both type dependencies and code dependencies are analysed, because most consumers care about both of these types of dependencies, but when generating documentation (e.g. deno_doc) or just running code (e.g. Deploy/eszip) only parts of the graph are needed, but both kinds will be loaded and followed fully to generate the graph.

When generating documentation on the fly, this causes a lot of wasted CPU cycles, potentially loading runtime code that can't impact the typing of the code, and when creating a "runtime" code graph, wastes cycles on analysing type only dependencies.

We should have an option when building the graph to have All TypeOnly and CodeOnly options and visiting only those modules as appropriate.

Ref #104

I'm not able to import an external package in version 1.15.3. Never finish the downloads and do not show any error.
The package is: https://unpkg.com/[email protected]/src/index.js

I had to downgrade to version 1.12.1 to make it work again.

WASM throws unreachable when giving arguments to ModuleGraph.resolveDependency in the wrong order.

This may be the line causing the error:

It would be useful for deno vendor to warn when there's a dynamic import specifier that deno_graph can't statically analyze.

For example, given the following:

const value = (() => "./logger.ts")();
const { Logger } = await import(value);

This might be and is at the current moment a dynamic import deno_graph doesn't know how to statically analyze for. It would be useful for deno_graph to be able to say that somehow so the CLI can warn about these for deno vendor.

Which generates a (ModuleSpecifier, ModuleKind) tuple.

Right now we only have Version::parse_from_npm, but it would be useful to just have cargo version parsing for stuff like deno upgrade (allows us to drop the semver dependency).

We’re going to be changing the output of deno info --json in 2.0. To help with this change and future changes, we should include a version number in the JSON output so people using this output can quickly tell if they support it without having to version sniff the binary and know which version something changed or have to detect the shape.

Related: #228

This should only be done when releasing deno_graph with the new schema changes.

$ cat test.js
import * as data from "./data.json";
$ cargo run -- run -A test.js
error: An unsupported media type was attempted to be imported as a module.
  Specifier: file:///Users/biwanczuk/dev/deno/data.json
  MediaType: Json
    at file:///Users/biwanczuk/dev/deno/test.js:1:23

The error message should suggest to add assert {type: "json} to the import statement.

For reference Chrome gives following message:

Expected a JavaScript module script but the server responded with a MIME type of "application/json". Strict MIME type checking is enforced for module scripts per HTML spec.

CC @kitsonk

For deno vendor, it might be nice to have a way to represent /// <reference types="..." /> and x-TypeScript-types separately in the graph while still providing a common method for the maybe_types_dependency.

This would also help us remove Range from being necessary for x-typescript-types as the types directive could have a range, but the x-typescript-types doesn't need one:

Alternatively, I can just do a check for the start and end position all being zero, but that seems a little hacky.

When doing something like this:

import a from "./a.js";
import * as b from "./a.js";
const c = await import("./a.js");

The dependencies for the module only include one import site. They should actually be two seperate dependency objects, as they "may" differ in ways (like import assertions or type definitions).

That being said, it still works and adding support for it would make everything related to it fairly complex, as knowing the import location would become essential for accessing information (unless we stored a vec of locations, and associated information, like it being a dynamic import and any import assertions).

Given thoses files :

// file://root/page.ts
function main() {
  import('./script.ts')
}

// file://root/script.ts
function main() {
  console.log('hello world')
}

If i run this :

    const moduleGraph = await graph.createGraph(`file://root/page.ts`)
    
    console.log(JSON.stringify(moduleGraph, null, 4))

I get the following JSON :

{
    "roots": [
        "file:///root/page.ts"
    ],
    "modules": [
        {
            "dependencies": [
                {
                    "specifier": "./script.ts",
                    "code": {
                        "specifier": "file:///root/script.ts",
                        "span": {
                            "start": {
                                "line": 1,
                                "character": 11
                            },
                            "end": {
                                "line": 1,
                                "character": 25
                            }
                        }
                    }
                }
            ],
            "mediaType": "TypeScript",
            "size": 59,
            "specifier": "file:///root/page.ts"
        },
        {
            "dependencies": [],
            "mediaType": "TypeScript",
            "size": 97,
            "specifier": "file:///root/script.ts"
        }
    ],
    "redirects": {}
}

I expected that for the module with specifier "file:///root/page.ts", there would be one dependency with the property isDynamic: true. Instead the property isDynamic is missing.

Edit:
If i understand #32 correctly, the property is_dynamic is not exposed by the underlying rust struct. But given the signature of the load function, i would expect this information to be exposed.

To support CJS (remove top level await) and scenarios where deno graph is imported but not used, we should support lazily creating a deno_graph.

For example:

import { load } from "...";

const denoGraph = await load();

await denoGraph.createGraph(....);

And found a little LSP bug: the preact in // @jsxImportSource preact is highlighted, but this pragma is only supported in a multi line comment (/** @jsxImportSource preact */).

Originally posted by @lucacasonato in denoland/deno#12631 (comment)

It looks like we could remove the concept of "locking" from deno graph and instead keep this functionality completely in the CLI. All we really need is a list of modules and their sources to do this, so this could help us simplify deno graph and we could also do this work more easily in parallel in the CLI.