refactor: factor out SourceFileFetcher to separate module #2683
Conversation
cli/deno_dir.rs
Outdated
|
|
||
| use_disk_cache: bool, | ||
| no_remote_fetch: bool, | ||
| cache_map: Arc<Mutex<HashMap<String, DiskCache>>>, |
There was a problem hiding this comment.
Can you add a comment here describing the typical keys here? I guess it's "deps" and "gen" ?
Will it ever be anything else other than "deps" and "gen"? If not, what's the benefit of having this generalized HashMap structure?
There was a problem hiding this comment.
Can you add a comment here describing the typical keys here? I guess it's "deps" and "gen" ?
Sure
Will it ever be anything else other than "deps" and "gen"? If not, what's the benefit of having this generalized HashMap structure?
It will if we introduce other compilers - then each compiler registers cache for itself.
import { foo } from "./foo.rs is still on my roadmap and I should be able to implement it once #2686 lands.
There was a problem hiding this comment.
I see, so 'gen' and 'deps' belong to the typescript compiler, but there may be others in the future...
What about using an enum instead of String for the key?
There was a problem hiding this comment.
Actually:
genbelongs toTsCompilerand is used and store compiler outputs and source mapsdepsbelongs toFileFetcherand is used to store remote modules locally
Yeah it definitely makes sense to use an enum. Will update 👍
There was a problem hiding this comment.
I'm gonna bring back DenoDir.deps_cache and DenoDir.gen_cache. Keeping them in hash map is an overkill now...
|
@bartlomieju Can you draft a commit message please? |
|
@ry sure: |
| /// Structure representing local or remote file. | ||
| /// | ||
| /// In case of remote file `url` might be different than originally requested URL, if so | ||
| /// `redirect_source_url` will contain original URL and `url` will be equal to final location. |
There was a problem hiding this comment.
These lines are not wrapped at 80 columns. I guess we have a bug in rustfmt...
I've added an issue for it #2699
| // trait and should be handled by "compiler pipeline" | ||
| pub fn js_source(&self) -> String { | ||
| if self.media_type == msg::MediaType::TypeScript { | ||
| panic!("TypeScript module has no JS source, did you forget to run it through compiler?"); |
Fixes #2659
It's a prototype to further decouple responsibilities of
DenoDir, namely logic for obtainingSourceFileis moved to//cli/file_fetcher.rs::FileFetcherstructure.I'll iterate on it before it's ready to land.
For now I just want to see if there's any benefit with this refactor.