refactor: TS compiler and module graph #5817
Conversation
cli/state.rs
Outdated
| let module_specifier = module_specifier.clone(); | ||
| let state = self.borrow(); | ||
| let target_lib = state.target_lib.clone(); | ||
| let maybe_import_map = state.import_map.clone(); | ||
| let permissions = if state.is_main && !is_dyn_import { | ||
| Permissions::allow_all() | ||
| } else { | ||
| state.permissions.clone() | ||
| }; | ||
| let global_state = state.global_state.clone(); |
There was a problem hiding this comment.
Implementation of this method requires to solve issue described in #1692 as well.
Currently TS compiler is spun up during fetch_compiled_module() call - if root module was already compiled and hasn't changed then cached code will be used. If one of root's dependencies has changed then TS compiler will be spun up.
Implementing prepare_load() means that we need to manually handle outdated dependencies - because prepare_load() is the only function that can spin up TS compiler. load() will only access already compiled source code in $DENO_DIR, but it won't be able to spin up TS compiler.
There was a problem hiding this comment.
Added basic handling of cache invalidation by saving dependency list in "module graph" meta file.
| ) -> std::io::Result<()> { | ||
| let version_hash = | ||
| crate::checksum::gen(vec![version::DENO.as_bytes(), &self.config.hash]); | ||
| let mut deps = vec![]; |
There was a problem hiding this comment.
This function should discriminate files that were compiled to JS as well as other auxiliary files (like type definitions)
| (the following is a syntax error ^^ ! ) | ||
| ~~~~~~~~~ | ||
| at [WILDCARD]tests/error_syntax.js:3:6 | ||
| error: Expected Comma, got Some(Word(following)) at [WILDCARD]tests/error_syntax.js:3:5 |
There was a problem hiding this comment.
I think we need to improve these over time, as got Some(Word(follow)) is pretty inaccessible to developers. Ultimately we want go grab the source line and highlight the span, and put some sort of match in to humanise the errors.
| @@ -1,2 +1 @@ | |||
| error: Uncaught SyntaxError: Unexpected end of input | |||
| at [WILDCARD]tests/error_syntax_empty_trailing_line.mjs:[WILDCARD] | |||
| error: Unexpected eof at [WILDCARD]tests/error_syntax_empty_trailing_line.mjs:2:21 | |||
There was a problem hiding this comment.
I think the same here... "eof" is sometimes well understood, but "Unexpected end of file" is clearer. No sense in holding up this PR, but maybe we should have a DX issue for error messages for this. Hopefully something the wider community would help out with.
|
Secretly hoping that this might solve my issues regarding #5665 😅 |
@katywings not really, that's a tangential issue with "deno bundle" which is tracked here #4542 |
| fn load( | ||
| &self, | ||
| module_specifier: &ModuleSpecifier, | ||
| maybe_referrer: Option<ModuleSpecifier>, | ||
| is_dyn_import: bool, | ||
| _is_dyn_import: bool, |
There was a problem hiding this comment.
I'm a bit confused about this is_dyn_import argument. What was its purpose before? Why is it not needed anymore?
There was a problem hiding this comment.
It was used for permission checks - it is still used, but in prepare_load() function (for the same purpose).
With this PR calling ModuleLoader.load() results in loading already cached file; all of the permission check, etc. are done during building of module graph
bartlomieju
changed the title
This PR addresses many problems with module graph loading introduced in denoland#5029, as well as many long standing issues. "ModuleGraphLoader" has been wired to "ModuleLoader" implemented on "State" - that means that dependency analysis and fetching is done before spinning up TS compiler worker. Basic dependency tracking for TS compilation has been implemented. Errors caused by import statements are now annotated with import location. Co-authored-by: Ryan Dahl <ry@tinyclouds.org>
This PR addresses many problems with module graph loading
introduced in #5029, as well as many long standing issues.
"ModuleGraphLoader" has been wired to "ModuleLoader" implemented
on "State" - that means that dependency analysis and fetching is done
before spinning up TS compiler worker.
Basic dependency tracking for TS compilation has been implemented.
Errors caused by import statements are now annotated with import
location.
Fixes #1692
Fixes #5080
Fixes #5419
Fixes #5815
Fixes #5900