cleanup, remove op_fetch_source_file

denoland · Sep 14, 2019 · 1d5e968 · 1d5e968
1 parent 8a46128
commit 1d5e968
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 94 deletions.
diff --git a/cli/ops/compiler.rs b/cli/ops/compiler.rs
@@ -32,42 +32,6 @@ pub fn op_cache(
   Ok(JsonOp::Sync(json!({})))
 }
 
-#[derive(Deserialize)]
-struct FetchSourceFileArgs {
-  specifier: String,
-  referrer: String,
-}
-
-pub fn op_fetch_source_file(
-  state: &ThreadSafeState,
-  args: Value,
-  _zero_copy: Option<PinnedBuf>,
-) -> Result<JsonOp, ErrBox> {
-  let args: FetchSourceFileArgs = serde_json::from_value(args)?;
-
-  // TODO(ry) Maybe a security hole. Only the compiler worker should have access
-  // to this. Need a test to demonstrate the hole.
-  let is_dyn_import = false;
-
-  let resolved_specifier =
-    state.resolve(&args.specifier, &args.referrer, false, is_dyn_import)?;
-
-  let fut = state
-    .file_fetcher
-    .fetch_source_file_async(&resolved_specifier);
-
-  // WARNING: Here we use tokio_util::block_on() which starts a new Tokio
-  // runtime for executing the future. This is so we don't inadvernently run
-  // out of threads in the main runtime.
-  let out = tokio_util::block_on(fut)?;
-  Ok(JsonOp::Sync(json!({
-    "moduleName": out.url.to_string(),
-    "filename": out.filename.to_str().unwrap(),
-    "mediaType": out.media_type as i32,
-    "sourceCode": String::from_utf8(out.source_code).unwrap(),
-  })))
-}
-
 #[derive(Deserialize)]
 struct FetchSourceFilesArgs {
   specifiers: Vec<String>,

diff --git a/cli/ops/mod.rs b/cli/ops/mod.rs
@@ -37,7 +37,7 @@ pub const OP_START: OpId = 10;
 pub const OP_APPLY_SOURCE_MAP: OpId = 11;
 pub const OP_FORMAT_ERROR: OpId = 12;
 pub const OP_CACHE: OpId = 13;
-pub const OP_FETCH_SOURCE_FILE: OpId = 14;
+pub const OP_FETCH_SOURCE_FILES: OpId = 14;
 pub const OP_OPEN: OpId = 15;
 pub const OP_CLOSE: OpId = 16;
 pub const OP_SEEK: OpId = 17;
@@ -81,7 +81,6 @@ pub const OP_TRUNCATE: OpId = 54;
 pub const OP_MAKE_TEMP_DIR: OpId = 55;
 pub const OP_CWD: OpId = 56;
 pub const OP_FETCH_ASSET: OpId = 57;
-pub const OP_FETCH_SOURCE_FILES: OpId = 58;
 
 pub fn dispatch(
   state: &ThreadSafeState,
@@ -134,8 +133,8 @@ pub fn dispatch(
     OP_CACHE => {
       dispatch_json::dispatch(compiler::op_cache, state, control, zero_copy)
     }
-    OP_FETCH_SOURCE_FILE => dispatch_json::dispatch(
-      compiler::op_fetch_source_file,
+    OP_FETCH_SOURCE_FILES => dispatch_json::dispatch(
+      compiler::op_fetch_source_files,
       state,
       control,
       zero_copy,
@@ -301,12 +300,6 @@ pub fn dispatch(
       control,
       zero_copy,
     ),
-    OP_FETCH_SOURCE_FILES => dispatch_json::dispatch(
-      compiler::op_fetch_source_files,
-      state,
-      control,
-      zero_copy,
-    ),
     _ => panic!("bad op_id"),
   };
 

diff --git a/js/compiler.ts b/js/compiler.ts
@@ -136,20 +136,6 @@ function fetchAsset(name: string): string {
   return sendSync(dispatch.OP_FETCH_ASSET, { name });
 }
 
-/** Ops to Rust to resolve and fetch a modules meta data. */
-function fetchSourceFile(specifier: string, referrer: string): SourceFile {
-  util.log("compiler.fetchSourceFile", { specifier, referrer });
-  const res = sendSync(dispatch.OP_FETCH_SOURCE_FILE, {
-    specifier,
-    referrer
-  });
-
-  return {
-    ...res,
-    typeDirectives: parseTypeDirectives(res.sourceCode)
-  };
-}
-
 /** Ops to Rust to resolve and fetch modules meta data. */
 function fetchSourceFiles(
   specifiers: string[],
@@ -240,36 +226,6 @@ class Host implements ts.CompilerHost {
 
   private _sourceFileCache: Record<string, SourceFile> = {};
 
-  private _resolveModule(specifier: string, referrer: string): SourceFile {
-    util.log("host._resolveModule", { specifier, referrer });
-    // Handle built-in assets specially.
-    if (specifier.startsWith(ASSETS)) {
-      const moduleName = specifier.split("/").pop()!;
-      if (moduleName in this._sourceFileCache) {
-        return this._sourceFileCache[moduleName];
-      }
-      const assetName = moduleName.includes(".")
-        ? moduleName
-        : `${moduleName}.d.ts`;
-      const sourceCode = fetchAsset(assetName);
-      const sourceFile = {
-        moduleName,
-        filename: specifier,
-        mediaType: MediaType.TypeScript,
-        sourceCode
-      };
-      this._sourceFileCache[moduleName] = sourceFile;
-      return sourceFile;
-    }
-    const sourceFile = fetchSourceFile(specifier, referrer);
-    assert(sourceFile.moduleName != null);
-    const { moduleName } = sourceFile;
-    if (!(moduleName! in this._sourceFileCache)) {
-      this._sourceFileCache[moduleName!] = sourceFile;
-    }
-    return sourceFile;
-  }
-
   private _getAsset(specifier: string): SourceFile {
     const moduleName = specifier.split("/").pop()!;
     if (moduleName in this._sourceFileCache) {
@@ -289,6 +245,10 @@ class Host implements ts.CompilerHost {
     return sourceFile;
   }
 
+  private _resolveModule(specifier: string, referrer: string): SourceFile {
+    return this._resolveModules([specifier], referrer)[0];
+  }
+
   private _resolveModules(
     specifiers: string[],
     referrer: string
@@ -301,7 +261,7 @@ class Host implements ts.CompilerHost {
         : undefined;
     // First of all built-in assets are handled specially, so they should
     // be removed from array of files will be requesting from Rust.
-    let resolvedModules: (SourceFile | undefined)[] = [];
+    const resolvedModules: Array<SourceFile | undefined> = [];
     const modulesToRequest = [];
 
     for (const specifier of specifiers) {
@@ -315,6 +275,10 @@ class Host implements ts.CompilerHost {
         const assetFile = this._getAsset(mappedModuleName);
         resolvedModules.push(assetFile);
         continue;
+      } else if (mappedModuleName in this._sourceFileCache) {
+        const module = this._sourceFileCache[mappedModuleName];
+        resolvedModules.push(module);
+        continue;
       }
 
       modulesToRequest.push(mappedModuleName);
@@ -324,7 +288,7 @@ class Host implements ts.CompilerHost {
     }
 
     // Now get files from Rust.
-    let sourceFiles = fetchSourceFiles(modulesToRequest, referrer);
+    const sourceFiles = fetchSourceFiles(modulesToRequest, referrer);
 
     for (const sourceFile of sourceFiles) {
       assert(sourceFile.moduleName != null);

diff --git a/js/dispatch.ts b/js/dispatch.ts
@@ -16,7 +16,7 @@ export const OP_START = 10;
 export const OP_APPLY_SOURCE_MAP = 11;
 export const OP_FORMAT_ERROR = 12;
 export const OP_CACHE = 13;
-export const OP_FETCH_SOURCE_FILE = 14;
+export const OP_FETCH_SOURCE_FILES = 14;
 export const OP_OPEN = 15;
 export const OP_CLOSE = 16;
 export const OP_SEEK = 17;
@@ -60,7 +60,6 @@ export const OP_TRUNCATE = 54;
 export const OP_MAKE_TEMP_DIR = 55;
 export const OP_CWD = 56;
 export const OP_FETCH_ASSET = 57;
-export const OP_FETCH_SOURCE_FILES = 58;
 
 export function asyncMsgFromRust(opId: number, ui8: Uint8Array): void {
   switch (opId) {