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

discussion: Rewrite deno info command to not execute module #3069

Closed
bartlomieju opened this issue Oct 5, 2019 · 5 comments
Closed

discussion: Rewrite deno info command to not execute module #3069

bartlomieju opened this issue Oct 5, 2019 · 5 comments
Assignees

Comments

@bartlomieju
Copy link
Member

bartlomieju commented Oct 5, 2019

IMHO I believe we should rewrite deno info <module> command a bit.

Why? Because this command loads, compiles and executes the module inside the isolate (that way we can use Modules.deps() API to retrieve dependencies of the module).
With recent changes to TS compiler done by @kitsonk (#3043) I deem need to execute the module inside the isolate obsolete. We can handle all the processing without running the code, but instead using TS compiler - using processImports in compiler.ts to retrieve dependency graph.

This refactor would enable us to provide Deno.deps() API.

Related issues:
#2096
#3035

@kitsonk
Copy link
Contributor

kitsonk commented Oct 6, 2019

So a couple things to consider, while preprocess works, it is still very opinionated for the compiler, which means that it supports that pragma to load types files, so you would get back a different answer in some cases than what is the runtime dependency. Also, I am not actually sure what the preprocess would do with only a JavaScript program. We would want to investigate that. Also, for info, it means you will always then take the hit of spinning up the compiler worker, even if it is just JavaScript, or already cached.

@ry mentioned to me recently that V8 also has a sort of module analysis API. We were speculating on if it would be faster than the preprocess in the compiler and potentially figure out a way to feed that to the compiler instead of using preprocess. It might actually make sense to look at this first for deno info, so we are only doing the first pass of the two pass module analysis, only lazy loading the compiler if we need to.

As a side note, I started looking at deno doc which would have to spin up the compiler, but also change the way the requests are sent into the compiler. Right now it is effectively "compile" or "bundle" just having the one flag, but for doc, ast, and potentially this, we need to the compiler to behave differently, so we need to change the message to the worker to support a sort of request type, which I have started adding.

@bartlomieju
Copy link
Member Author

So a couple things to consider, while preprocess works, it is still very opinionated for the compiler, which means that it supports that pragma to load types files, so you would get back a different answer in some cases than what is the runtime dependency.

Yes, I thought about it, I guess we can add second function to get dependency graph, that would ignore all the typing, etc.

Also, I am not actually sure what the preprocess would do with only a JavaScript program. We would want to investigate that.

Didn't think about it, thanks, definitely to be checked.

Also, for info, it means you will always then take the hit of spinning up the compiler worker, even if it is just JavaScript, or already cached.

That's right, but deno info is a one-off command, so I believe this perf hit won't be noticed. If files are already cached then we're left only with "dependency graph" to compute. I think it still might be faster than executing the module on isolate.

@ry mentioned to me recently that V8 also has a sort of module analysis API. We were speculating on if it would be faster than the preprocess in the compiler and potentially figure out a way to feed that to the compiler instead of using preprocess. It might actually make sense to look at this first for deno info, so we are only doing the first pass of the two pass module analysis, only lazy loading the compiler if we need to.

👍 that's worth exploring

As a side note, I started looking at deno doc which would have to spin up the compiler, but also change the way the requests are sent into the compiler. Right now it is effectively "compile" or "bundle" just having the one flag, but for doc, ast, and potentially this, we need to the compiler to behave differently, so we need to change the message to the worker to support a sort of request type, which I have started adding.

Great, I'll hold up with any work on this PR, till you finish.

@hayd
Copy link
Contributor

hayd commented May 10, 2020

deno info doesn't execute the module, or perhaps i am missing something?

@kitsonk
Copy link
Contributor

kitsonk commented May 10, 2020

I believe it does, so ensure it has the full dependency tree. While there are discreet APIs to analyse the dependencies of an ESM module in V8, we weren't doing it that way. The dependency analyser in Rust via swc is the way to go, which is in progress.

@bartlomieju
Copy link
Member Author

Closed in #6786

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants