feat(evaluate): serialize map and set

[sand4rt]

closes: #24040

[mxschmitt]

I'd do something like this instead which follows our existing serialisation infra, which would also work with circular objects, complex data types inside Map etc. This would be worth adding some tests.

diff --git a/packages/playwright-core/src/protocol/serializers.ts b/packages/playwright-core/src/protocol/serializers.ts
index 3e1f99a41..813ab22cb 100644
--- a/packages/playwright-core/src/protocol/serializers.ts
+++ b/packages/playwright-core/src/protocol/serializers.ts
@@ -76,9 +76,9 @@ function innerParseSerializedValue(value: SerializedValue, handles: any[] | unde
   if (value.r !== undefined)
     return new RegExp(value.r.p, value.r.f);
   if (value.m !== undefined)
-    return new Map(Object.entries(JSON.parse(value.m)));
+    return new Map(Object.entries(innerParseSerializedValue(value.m, handles, refs)));
   if (value.se !== undefined)
-    return new Set(JSON.parse(value.se));
+    return new Set(innerParseSerializedValue(value.se, handles, refs));
 
   if (value.a !== undefined) {
     const result: any[] = [];
@@ -150,9 +150,9 @@ function innerSerializeValue(value: any, handleSerializer: (value: any) => Handl
     return { s: `${error.name}: ${error.message}\n${error.stack}` };
   }
   if (isMap(value))
-    return { m: JSON.stringify(Object.fromEntries(value)) };
+    return { m: innerSerializeValue(Object.fromEntries(value), handleSerializer, visitorInfo) };
   if (isSet(value))
-    return { se: JSON.stringify(Array.from(value)) };
+    return { se: innerSerializeValue(Array.from(value), handleSerializer, visitorInfo) };
   if (isDate(value))
     return { d: value.toJSON() };
   if (isURL(value))
diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts
index 71f1ffbd0..c05a349a5 100644
--- a/packages/playwright-core/src/protocol/validator.ts
+++ b/packages/playwright-core/src/protocol/validator.ts
@@ -58,8 +58,8 @@ scheme.SerializedValue = tObject({
   d: tOptional(tString),
   u: tOptional(tString),
   bi: tOptional(tString),
-  m: tOptional(tString),
-  se: tOptional(tString),
+  m: tOptional(tType('SerializedValue')),
+  se: tOptional(tType('SerializedValue')),
   r: tOptional(tObject({
     p: tString,
     f: tString,
diff --git a/packages/playwright-core/src/server/isomorphic/utilityScriptSerializers.ts b/packages/playwright-core/src/server/isomorphic/utilityScriptSerializers.ts
index 9be8d04a3..90e4b913e 100644
--- a/packages/playwright-core/src/server/isomorphic/utilityScriptSerializers.ts
+++ b/packages/playwright-core/src/server/isomorphic/utilityScriptSerializers.ts
@@ -20,8 +20,8 @@ export type SerializedValue =
     { d: string } |
     { u: string } |
     { bi: string } |
-    { m: string } |
-    { se: string } |
+    { m: SerializedValue } |
+    { se: SerializedValue } |
     { r: { p: string, f: string} } |
     { a: SerializedValue[], id: number } |
     { o: { k: string, v: SerializedValue }[], id: number } |
@@ -105,9 +105,9 @@ export function source() {
       if ('bi' in value)
         return BigInt(value.bi);
       if ('m' in value)
-        return new Map(Object.entries(JSON.parse(value.m)));
+        return new Map(Object.entries(parseEvaluationResultValue(value.m)));
       if ('se' in value)
-        return new Set(JSON.parse(value.se));
+        return new Set(parseEvaluationResultValue(value.se));
       if ('r' in value)
         return new RegExp(value.r.p, value.r.f);
       if ('a' in value) {
@@ -178,9 +178,9 @@ export function source() {
       return { bi: value.toString() };
 
     if (isMap(value))
-      return { m: JSON.stringify(Object.fromEntries(value)) };
+      return { m: serialize(Object.fromEntries(value), handleSerializer, visitorInfo) };
     if (isSet(value))
-      return { se: JSON.stringify(Array.from(value)) };
+      return { se: serialize(Array.from(value), handleSerializer, visitorInfo) };
 
     if (isError(value)) {
       const error = value;
diff --git a/packages/protocol/src/channels.ts b/packages/protocol/src/channels.ts
index c57877321..9e7abcbd1 100644
--- a/packages/protocol/src/channels.ts
+++ b/packages/protocol/src/channels.ts
@@ -180,8 +180,8 @@ export type SerializedValue = {
   d?: string,
   u?: string,
   bi?: string,
-  m?: string,
-  se?: string,
+  m?: SerializedValue,
+  se?: SerializedValue,
   r?: {
     p: string,
     f: string,
diff --git a/packages/protocol/src/protocol.yml b/packages/protocol/src/protocol.yml
index bf7cd020e..72f4bf809 100644
--- a/packages/protocol/src/protocol.yml
+++ b/packages/protocol/src/protocol.yml
@@ -83,9 +83,9 @@ SerializedValue:
     # String representation of BigInt.
     bi: string?
     # String representation of Map.
-    m: string?
+    m: SerializedValue?
     # String representation of Set.
-    se: string?
+    se: SerializedValue?
     # Regular expression pattern and flags.
     r:
       type: object?

[sand4rt]

I'd do something like this instead which follows our existing serialisation infra, which would also work with circular objects, complex data types inside Map etc. This would be worth adding some tests.

Thanks for the pointers. I've updated the PR.

[dgozman]

Looks great, just some nits.

[github-actions]

Test results for "tests 1"

5 flaky
⚠️ [chromium] › library/tracing.spec.ts:239:5 › should not include trace resources from the previous chunks
⚠️ [chromium] › page/page-event-request.spec.ts:130:3 › should report navigation requests and responses handled by service worker with routing
⚠️ [playwright-test] › ui-mode-test-progress.spec.ts:167:5 › should update tracing network live
⚠️ [webkit] › library/browsercontext-reuse.spec.ts:50:1 › should reset serviceworker
⚠️ [playwright-test] › ui-mode-test-progress.spec.ts:22:5 › should update trace live

25555 passed, 595 skipped
✔️✔️✔️

Merge workflow run.