From d41d3b8e2f6079d806fb4bc4845b9ad681c20f00 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Bartek=20Iwa=C5=84czuk?= <biwanczuk@gmail.com>
Date: Sun, 8 Oct 2023 21:00:41 +0200
Subject: [PATCH] refactor: migrate last tsc op to op2 macro (#20816)

---
 cli/tsc/mod.rs | 72 ++++++++++++++++++++++++++++----------------------
 1 file changed, 40 insertions(+), 32 deletions(-)

diff --git a/cli/tsc/mod.rs b/cli/tsc/mod.rs
index c08d12e280..5f4f7ddf16 100644
--- a/cli/tsc/mod.rs
+++ b/cli/tsc/mod.rs
@@ -14,16 +14,13 @@ use deno_core::anyhow::Context;
 use deno_core::ascii_str;
 use deno_core::error::AnyError;
 use deno_core::located_script_name;
-use deno_core::op;
 use deno_core::op2;
 use deno_core::resolve_url_or_path;
 use deno_core::serde::Deserialize;
 use deno_core::serde::Deserializer;
 use deno_core::serde::Serialize;
 use deno_core::serde::Serializer;
-use deno_core::serde_json;
 use deno_core::serde_json::json;
-use deno_core::serde_json::Value;
 use deno_core::serde_v8;
 use deno_core::JsRuntime;
 use deno_core::ModuleSpecifier;
@@ -444,17 +441,29 @@ pub fn as_ts_script_kind(media_type: MediaType) -> i32 {
   }
 }
 
-// TODO(bartlomieju): `op2` doesn't support `serde_json::Value`
-#[op]
-fn op_load(state: &mut OpState, args: Value) -> Result<Value, AnyError> {
+#[derive(Debug, Serialize)]
+#[serde(rename_all = "camelCase")]
+struct LoadResponse {
+  data: Option<String>,
+  version: Option<String>,
+  script_kind: i32,
+}
+
+#[op2]
+#[serde]
+fn op_load(
+  state: &mut OpState,
+  #[serde] v: LoadArgs,
+) -> Result<LoadResponse, AnyError> {
   let state = state.borrow_mut::<State>();
-  let v: LoadArgs = serde_json::from_value(args)
-    .context("Invalid request from JavaScript for \"op_load\".")?;
+
   let specifier = normalize_specifier(&v.specifier, &state.current_dir)
     .context("Error converting a string module specifier for \"op_load\".")?;
+
   let mut hash: Option<String> = None;
   let mut media_type = MediaType::Unknown;
   let graph = &state.graph;
+
   let data = if &v.specifier == "internal:///.tsbuildinfo" {
     state.maybe_tsbuildinfo.as_deref().map(Cow::Borrowed)
   // in certain situations we return a "blank" module to tsc and we need to
@@ -521,11 +530,11 @@ fn op_load(state: &mut OpState, args: Value) -> Result<Value, AnyError> {
     maybe_source
   };
 
-  Ok(json!({
-    "data": data,
-    "version": hash,
-    "scriptKind": as_ts_script_kind(media_type),
-  }))
+  Ok(LoadResponse {
+    data: data.map(String::from),
+    version: hash,
+    script_kind: as_ts_script_kind(media_type),
+  })
 }
 
 #[derive(Debug, Deserialize, Serialize)]
@@ -879,6 +888,7 @@ mod tests {
   use super::*;
   use crate::args::TsConfig;
   use deno_core::futures::future;
+  use deno_core::serde_json;
   use deno_core::OpState;
   use deno_graph::GraphKind;
   use deno_graph::ModuleGraph;
@@ -1054,11 +1064,13 @@ mod tests {
     .await;
     let actual = op_load::call(
       &mut state,
-      json!({ "specifier": "https://deno.land/x/mod.ts"}),
+      LoadArgs {
+        specifier: "https://deno.land/x/mod.ts".to_string(),
+      },
     )
     .unwrap();
     assert_eq!(
-      actual,
+      serde_json::to_value(actual).unwrap(),
       json!({
         "data": "console.log(\"hello deno\");\n",
         "version": "7821807483407828376",
@@ -1067,14 +1079,6 @@ mod tests {
     );
   }
 
-  #[derive(Debug, Deserialize)]
-  #[serde(rename_all = "camelCase")]
-  struct LoadResponse {
-    data: String,
-    version: Option<String>,
-    script_kind: i64,
-  }
-
   #[tokio::test]
   async fn test_load_asset() {
     let mut state = setup(
@@ -1083,15 +1087,15 @@ mod tests {
       Some("some content".to_string()),
     )
     .await;
-    let value = op_load::call(
+    let actual = op_load::call(
       &mut state,
-      json!({ "specifier": "asset:///lib.dom.d.ts" }),
+      LoadArgs {
+        specifier: "asset:///lib.dom.d.ts".to_string(),
+      },
     )
     .expect("should have invoked op");
-    let actual: LoadResponse =
-      serde_json::from_value(value).expect("failed to deserialize");
     let expected = get_lazily_loaded_asset("lib.dom.d.ts").unwrap();
-    assert_eq!(actual.data, expected);
+    assert_eq!(actual.data.unwrap(), expected);
     assert!(actual.version.is_some());
     assert_eq!(actual.script_kind, 3);
   }
@@ -1106,11 +1110,13 @@ mod tests {
     .await;
     let actual = op_load::call(
       &mut state,
-      json!({ "specifier": "internal:///.tsbuildinfo"}),
+      LoadArgs {
+        specifier: "internal:///.tsbuildinfo".to_string(),
+      },
     )
     .expect("should have invoked op");
     assert_eq!(
-      actual,
+      serde_json::to_value(actual).unwrap(),
       json!({
         "data": "some content",
         "version": null,
@@ -1124,11 +1130,13 @@ mod tests {
     let mut state = setup(None, None, None).await;
     let actual = op_load::call(
       &mut state,
-      json!({ "specifier": "https://deno.land/x/mod.ts"}),
+      LoadArgs {
+        specifier: "https://deno.land/x/mod.ts".to_string(),
+      },
     )
     .expect("should have invoked op");
     assert_eq!(
-      actual,
+      serde_json::to_value(actual).unwrap(),
       json!({
         "data": null,
         "version": null,