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

Remove processing of files from TS compiler #5018

Closed
bartlomieju opened this issue Apr 30, 2020 · 4 comments
Closed

Remove processing of files from TS compiler #5018

bartlomieju opened this issue Apr 30, 2020 · 4 comments

Comments

@bartlomieju
Copy link
Member

In an effort to fix permission escapes we're currently experiencing (#4744, #4743, #4383) TS compiler and how it obtains source files should be refactored (details are outlined in #4781.).

First step towards that goal is removing processing of imports (processImports) and type directives (parseTypeDirectives) from cli/js/compiler.ts. With #5015 landed it will be easy perform the same analysis in Rust (and hopefully much faster).

I would like to arrive at the situation where we know upfront about all source files needed for compilation/transpilation/bundling process before compiler worker is ever spawned; so the only IO that is done by compiler worker is with $DENO_DIR.

Ideally, when compiler worker is created, all of the sources would be sent in a single "request" message - just like in case of Deno.compile() with map of sources.

Similarly, instead of doing multiple op_cache calls, all compiled sources and source maps should be collected and sent in a single message when compilation is done.

This is a very complex issue, touching multiple concepts and structures from cli/ and might require numerous smaller PRs to achieve.

CC @kitsonk

@ry
Copy link
Member

ry commented Apr 30, 2020

I don't have a general solution to the problem at this point, but I'm definitely in favor of being able to analyze the dependency tree in Rust and having facilities to ensure the dependency tree has been cached. Maybe once this is complete things will be easier to untangle.

@kitsonk
Copy link
Contributor

kitsonk commented Apr 30, 2020

I don't think it will be as difficult as it seems, and I would be glad to work on it. We implemented preprocess to make the resolution of the cache files async, if we do the dependency analysis in Rust and go off and fetch all the sources before we even spin up the compiler, we can just let the compiler be async again, requesting from Rust the files it needs. The original problem was that TS is eager in its fetching of remote resources to be able to do static analysis on them.

@bartlomieju
Copy link
Member Author

we can just let the compiler be async again, requesting from Rust the files it needs.

I think you meant "sync" here? Anyway I agree; I'm already WiP with module graph builder that will discover and download all dependencies. I should have a PR ready tonight; if you could integrate it with the JS side that'd be awesome.

@bartlomieju
Copy link
Member Author

Done in #5029

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