diff --git a/packages/playwright-core/src/client/jsHandle.ts b/packages/playwright-core/src/client/jsHandle.ts index 13acb7da19..5975649ff5 100644 --- a/packages/playwright-core/src/client/jsHandle.ts +++ b/packages/playwright-core/src/client/jsHandle.ts @@ -84,7 +84,7 @@ export function serializeArgument(arg: any): channels.SerializedArgument { if (value instanceof JSHandle) return { h: pushHandle(value._channel) }; return { fallThrough: value }; - }, new Set()); + }); return { value, handles }; } diff --git a/packages/playwright-core/src/protocol/channels.ts b/packages/playwright-core/src/protocol/channels.ts index 0e019c352e..3171e1e462 100644 --- a/packages/playwright-core/src/protocol/channels.ts +++ b/packages/playwright-core/src/protocol/channels.ts @@ -173,6 +173,8 @@ export type SerializedValue = { v: SerializedValue, }[], h?: number, + id?: number, + ref?: number, }; export type SerializedArgument = { diff --git a/packages/playwright-core/src/protocol/protocol.yml b/packages/playwright-core/src/protocol/protocol.yml index 046de94b06..18a445f0d1 100644 --- a/packages/playwright-core/src/protocol/protocol.yml +++ b/packages/playwright-core/src/protocol/protocol.yml @@ -85,6 +85,10 @@ SerializedValue: v: SerializedValue # An index in the handles array from SerializedArgument. h: number? + # Index of the object in value-type for circular reference resolution. + id: number? + # Ref to the object in value-type for circular reference resolution. + ref: number? # Represents a value with handle references. diff --git a/packages/playwright-core/src/protocol/serializers.ts b/packages/playwright-core/src/protocol/serializers.ts index 50342c470e..aa28d24f8b 100644 --- a/packages/playwright-core/src/protocol/serializers.ts +++ b/packages/playwright-core/src/protocol/serializers.ts @@ -20,7 +20,7 @@ import type { SerializedError, SerializedValue } from './channels'; export function serializeError(e: any): SerializedError { if (isError(e)) return { error: { message: e.message, stack: e.stack, name: e.name } }; - return { value: serializeValue(e, value => ({ fallThrough: value }), new Set()) }; + return { value: serializeValue(e, value => ({ fallThrough: value })) }; } export function parseError(error: SerializedError): Error { @@ -41,6 +41,12 @@ export function parseError(error: SerializedError): Error { } export function parseSerializedValue(value: SerializedValue, handles: any[] | undefined): any { + return innerParseSerializedValue(value, handles, new Map()); +} + +function innerParseSerializedValue(value: SerializedValue, handles: any[] | undefined, refs: Map): any { + if (value.ref !== undefined) + return refs.get(value.ref); if (value.n !== undefined) return value.n; if (value.s !== undefined) @@ -65,12 +71,19 @@ export function parseSerializedValue(value: SerializedValue, handles: any[] | un return new Date(value.d); if (value.r !== undefined) return new RegExp(value.r.p, value.r.f); - if (value.a !== undefined) - return value.a.map((a: any) => parseSerializedValue(a, handles)); + + if (value.a !== undefined) { + const result: any[] = []; + refs.set(value.id!, result); + for (const v of value.a) + result.push(innerParseSerializedValue(v, handles, refs)); + return result; + } if (value.o !== undefined) { const result: any = {}; + refs.set(value.id!, result); for (const { k, v } of value.o) - result[k] = parseSerializedValue(v, handles); + result[k] = innerParseSerializedValue(v, handles, refs); return result; } if (value.h !== undefined) { @@ -82,15 +95,22 @@ export function parseSerializedValue(value: SerializedValue, handles: any[] | un } export type HandleOrValue = { h: number } | { fallThrough: any }; -export function serializeValue(value: any, handleSerializer: (value: any) => HandleOrValue, visited: Set): SerializedValue { +type VisitorInfo = { + visited: Map; + lastId: number; +}; + +export function serializeValue(value: any, handleSerializer: (value: any) => HandleOrValue): SerializedValue { + return innerSerializeValue(value, handleSerializer, { lastId: 0, visited: new Map() }); +} + +function innerSerializeValue(value: any, handleSerializer: (value: any) => HandleOrValue, visitorInfo: VisitorInfo): SerializedValue { const handle = handleSerializer(value); if ('fallThrough' in handle) value = handle.fallThrough; else return handle; - if (visited.has(value)) - throw new Error('Argument is a circular structure'); if (typeof value === 'symbol') return { v: 'undefined' }; if (Object.is(value, undefined)) @@ -123,21 +143,26 @@ export function serializeValue(value: any, handleSerializer: (value: any) => Han return { d: value.toJSON() }; if (isRegExp(value)) return { r: { p: value.source, f: value.flags } }; + + const id = visitorInfo.visited.get(value); + if (id) + return { ref: id }; + if (Array.isArray(value)) { const a = []; - visited.add(value); + const id = ++visitorInfo.lastId; + visitorInfo.visited.set(value, id); for (let i = 0; i < value.length; ++i) - a.push(serializeValue(value[i], handleSerializer, visited)); - visited.delete(value); - return { a }; + a.push(innerSerializeValue(value[i], handleSerializer, visitorInfo)); + return { a, id }; } if (typeof value === 'object') { const o: { k: string, v: SerializedValue }[] = []; - visited.add(value); + const id = ++visitorInfo.lastId; + visitorInfo.visited.set(value, id); for (const name of Object.keys(value)) - o.push({ k: name, v: serializeValue(value[name], handleSerializer, visited) }); - visited.delete(value); - return { o }; + o.push({ k: name, v: innerSerializeValue(value[name], handleSerializer, visitorInfo) }); + return { o, id }; } throw new Error('Unexpected value'); } diff --git a/packages/playwright-core/src/protocol/validator.ts b/packages/playwright-core/src/protocol/validator.ts index 0e4576fae4..da82ca1815 100644 --- a/packages/playwright-core/src/protocol/validator.ts +++ b/packages/playwright-core/src/protocol/validator.ts @@ -72,6 +72,8 @@ export function createScheme(tChannel: (name: string) => Validator): Scheme { v: tType('SerializedValue'), }))), h: tOptional(tNumber), + id: tOptional(tNumber), + ref: tOptional(tNumber), }); scheme.SerializedArgument = tObject({ value: tType('SerializedValue'), diff --git a/packages/playwright-core/src/server/dispatchers/jsHandleDispatcher.ts b/packages/playwright-core/src/server/dispatchers/jsHandleDispatcher.ts index 5364b71e8a..0d9e2c0114 100644 --- a/packages/playwright-core/src/server/dispatchers/jsHandleDispatcher.ts +++ b/packages/playwright-core/src/server/dispatchers/jsHandleDispatcher.ts @@ -74,5 +74,5 @@ export function parseValue(v: channels.SerializedValue): any { } export function serializeResult(arg: any): channels.SerializedValue { - return serializeValue(arg, value => ({ fallThrough: value }), new Set()); + return serializeValue(arg, value => ({ fallThrough: value })); } diff --git a/packages/playwright-core/src/server/isomorphic/utilityScriptSerializers.ts b/packages/playwright-core/src/server/isomorphic/utilityScriptSerializers.ts index 62fd973b95..b10a6dc879 100644 --- a/packages/playwright-core/src/server/isomorphic/utilityScriptSerializers.ts +++ b/packages/playwright-core/src/server/isomorphic/utilityScriptSerializers.ts @@ -19,13 +19,19 @@ export type SerializedValue = { v: 'null' | 'undefined' | 'NaN' | 'Infinity' | '-Infinity' | '-0' } | { d: string } | { r: { p: string, f: string} } | - { a: SerializedValue[] } | - { o: { k: string, v: SerializedValue }[] } | + { a: SerializedValue[], id: number } | + { o: { k: string, v: SerializedValue }[], id: number } | + { ref: number } | { h: number }; export type HandleOrValue = { h: number } | { fallThrough: any }; -export function source(aliasComplexAndCircularObjects: boolean = false) { +type VisitorInfo = { + visited: Map; + lastId: number; +}; + +export function source() { function isRegExp(obj: any): obj is RegExp { return obj instanceof RegExp || Object.prototype.toString.call(obj) === '[object RegExp]'; @@ -39,10 +45,12 @@ export function source(aliasComplexAndCircularObjects: boolean = false) { return obj instanceof Error || (obj && obj.__proto__ && obj.__proto__.name === 'Error'); } - function parseEvaluationResultValue(value: SerializedValue, handles: any[] = []): any { + function parseEvaluationResultValue(value: SerializedValue, handles: any[] = [], refs: Map = new Map()): any { if (Object.is(value, undefined)) return undefined; if (typeof value === 'object' && value) { + if ('ref' in value) + return refs.get(value.ref); if ('v' in value) { if (value.v === 'undefined') return undefined; @@ -62,12 +70,18 @@ export function source(aliasComplexAndCircularObjects: boolean = false) { return new Date(value.d); if ('r' in value) return new RegExp(value.r.p, value.r.f); - if ('a' in value) - return value.a.map((a: any) => parseEvaluationResultValue(a, handles)); + if ('a' in value) { + const result: any[] = []; + refs.set(value.id, result); + for (const a of value.a) + result.push(parseEvaluationResultValue(a, handles, refs)); + return result; + } if ('o' in value) { const result: any = {}; + refs.set(value.id, result); for (const { k, v } of value.o) - result[k] = parseEvaluationResultValue(v, handles); + result[k] = parseEvaluationResultValue(v, handles, refs); return result; } if ('h' in value) @@ -77,21 +91,10 @@ export function source(aliasComplexAndCircularObjects: boolean = false) { } function serializeAsCallArgument(value: any, handleSerializer: (value: any) => HandleOrValue): SerializedValue { - return serialize(value, handleSerializer, new Set()); + return serialize(value, handleSerializer, { visited: new Map(), lastId: 0 }); } - function serialize(value: any, handleSerializer: (value: any) => HandleOrValue, visited: Set): SerializedValue { - if (!aliasComplexAndCircularObjects) - return innerSerialize(value, handleSerializer, visited); - try { - const alias = serializeComplexObjectAsAlias(value); - return alias || innerSerialize(value, handleSerializer, visited); - } catch (error) { - return error.stack; - } - } - - function serializeComplexObjectAsAlias(value: any): string | undefined { + function serialize(value: any, handleSerializer: (value: any) => HandleOrValue, visitorInfo: VisitorInfo): SerializedValue { if (value && typeof value === 'object') { if (globalThis.Window && value instanceof globalThis.Window) return 'ref: '; @@ -100,22 +103,16 @@ export function source(aliasComplexAndCircularObjects: boolean = false) { if (globalThis.Node && value instanceof globalThis.Node) return 'ref: '; } + return innerSerialize(value, handleSerializer, visitorInfo); } - function innerSerialize(value: any, handleSerializer: (value: any) => HandleOrValue, visited: Set): SerializedValue { + function innerSerialize(value: any, handleSerializer: (value: any) => HandleOrValue, visitorInfo: VisitorInfo): SerializedValue { const result = handleSerializer(value); if ('fallThrough' in result) value = result.fallThrough; else return result; - if (visited.has(value)) { - if (aliasComplexAndCircularObjects) { - const alias = serializeComplexObjectAsAlias(value); - return alias || '[Circular Ref]'; - } - throw new Error('Argument is a circular structure'); - } if (typeof value === 'symbol') return { v: 'undefined' }; if (Object.is(value, undefined)) @@ -151,18 +148,23 @@ export function source(aliasComplexAndCircularObjects: boolean = false) { if (isRegExp(value)) return { r: { p: value.source, f: value.flags } }; + const id = visitorInfo.visited.get(value); + if (id) + return { ref: id }; + if (Array.isArray(value)) { const a = []; - visited.add(value); + const id = ++visitorInfo.lastId; + visitorInfo.visited.set(value, id); for (let i = 0; i < value.length; ++i) - a.push(serialize(value[i], handleSerializer, visited)); - visited.delete(value); - return { a }; + a.push(serialize(value[i], handleSerializer, visitorInfo)); + return { a, id }; } if (typeof value === 'object') { const o: { k: string, v: SerializedValue }[] = []; - visited.add(value); + const id = ++visitorInfo.lastId; + visitorInfo.visited.set(value, id); for (const name of Object.keys(value)) { let item; try { @@ -171,12 +173,11 @@ export function source(aliasComplexAndCircularObjects: boolean = false) { continue; // native bindings will throw sometimes } if (name === 'toJSON' && typeof item === 'function') - o.push({ k: name, v: { o: [] } }); + o.push({ k: name, v: { o: [], id: 0 } }); else - o.push({ k: name, v: serialize(item, handleSerializer, visited) }); + o.push({ k: name, v: serialize(item, handleSerializer, visitorInfo) }); } - visited.delete(value); - return { o }; + return { o, id }; } } diff --git a/packages/playwright-core/src/server/page.ts b/packages/playwright-core/src/server/page.ts index 24c051ebab..7e8b62d4ec 100644 --- a/packages/playwright-core/src/server/page.ts +++ b/packages/playwright-core/src/server/page.ts @@ -729,7 +729,7 @@ export class PageBinding { constructor(name: string, playwrightFunction: frames.FunctionWithSource, needsHandle: boolean) { this.name = name; this.playwrightFunction = playwrightFunction; - this.source = `(${addPageBinding.toString()})(${JSON.stringify(name)}, ${needsHandle}, (${source})(true))`; + this.source = `(${addPageBinding.toString()})(${JSON.stringify(name)}, ${needsHandle}, (${source})())`; this.needsHandle = needsHandle; } @@ -743,7 +743,7 @@ export class PageBinding { const handle = await context.evaluateHandle(takeHandle, { name, seq }).catch(e => null); result = await binding.playwrightFunction({ frame: context.frame, page, context: page._browserContext }, handle); } else { - const args = serializedArgs!.map(a => parseEvaluationResultValue(a, [])); + const args = serializedArgs!.map(a => parseEvaluationResultValue(a)); result = await binding.playwrightFunction({ frame: context.frame, page, context: page._browserContext }, ...args); } context.evaluate(deliverResult, { name, seq, result }).catch(e => debugLogger.log('error', e)); diff --git a/tests/page/jshandle-json-value.spec.ts b/tests/page/jshandle-json-value.spec.ts index 84979a628c..53ceb74702 100644 --- a/tests/page/jshandle-json-value.spec.ts +++ b/tests/page/jshandle-json-value.spec.ts @@ -29,9 +29,9 @@ it('should work with dates', async ({ page }) => { expect(date.toJSON()).toBe('2017-09-26T00:00:00.000Z'); }); -it('should throw for circular objects', async ({ page }) => { - const windowHandle = await page.evaluateHandle('window'); - let error = null; - await windowHandle.jsonValue().catch(e => error = e); - expect(error.message).toContain('Argument is a circular structure'); +it('should handle circular objects', async ({ page }) => { + const handle = await page.evaluateHandle('const a = {}; a.b = a; a'); + const a: any = {}; + a.b = a; + expect(await handle.jsonValue()).toEqual(a); }); diff --git a/tests/page/page-evaluate-handle.spec.ts b/tests/page/page-evaluate-handle.spec.ts index fd8857e5fa..26f4cc14a2 100644 --- a/tests/page/page-evaluate-handle.spec.ts +++ b/tests/page/page-evaluate-handle.spec.ts @@ -63,13 +63,6 @@ it('should accept multiple nested handles', async ({ page }) => { }); }); -it('should throw for circular objects', async ({ page }) => { - const a = { x: 1 }; - a['y'] = a; - const error = await page.evaluate(x => x, a).catch(e => e); - expect(error.message).toContain('Argument is a circular structure'); -}); - it('should accept same handle multiple times', async ({ page }) => { const foo = await page.evaluateHandle(() => 1); expect(await page.evaluate(x => x, { foo, bar: [foo], baz: { foo } })).toEqual({ foo: 1, bar: [1], baz: { foo: 1 } }); diff --git a/tests/page/page-evaluate.spec.ts b/tests/page/page-evaluate.spec.ts index 21901d4dd0..5e56fe9fec 100644 --- a/tests/page/page-evaluate.spec.ts +++ b/tests/page/page-evaluate.spec.ts @@ -331,17 +331,23 @@ it('should properly serialize null fields', async ({ page }) => { }); it('should return undefined for non-serializable objects', async ({ page }) => { - expect(await page.evaluate(() => window)).toBe(undefined); + expect(await page.evaluate(() => function() {})).toBe(undefined); }); -it('should fail for circular object', async ({ page }) => { +it('should alias Window, Document and Node', async ({ page }) => { + const object = await page.evaluate('[window, document, document.body]'); + expect(object).toEqual(['ref: ', 'ref: ', 'ref: ']); +}); + +it('should work for circular object', async ({ page }) => { const result = await page.evaluate(() => { const a = {} as any; - const b = { a }; - a.b = b; + a.b = a; return a; }); - expect(result).toBe(undefined); + const a = {} as any; + a.b = a; + expect(result).toEqual(a); }); it('should be able to throw a tricky error', async ({ page }) => { diff --git a/tests/page/page-expose-function.spec.ts b/tests/page/page-expose-function.spec.ts index d622ef89e1..88bcd4f823 100644 --- a/tests/page/page-expose-function.spec.ts +++ b/tests/page/page-expose-function.spec.ts @@ -292,11 +292,11 @@ it('should alias Window, Document and Node', async ({ page }) => { expect(object).toEqual(['ref: ', 'ref: ', 'ref: ']); }); -it('should trim cycles', async ({ page }) => { +it('should serialize cycles', async ({ page }) => { let object: any; await page.exposeBinding('log', (source, obj) => object = obj); - await page.evaluate('const a = { a: 1 }; a.a = a; window.log(a)'); - expect(object).toEqual({ - a: '[Circular Ref]', - }); + await page.evaluate('const a = {}; a.b = a; window.log(a)'); + const a: any = {}; + a.b = a; + expect(object).toEqual(a); }); diff --git a/utils/build/build.js b/utils/build/build.js index 6adcfa93c1..fa64cc871d 100644 --- a/utils/build/build.js +++ b/utils/build/build.js @@ -239,6 +239,7 @@ onChanges.push({ committed: false, inputs: [ 'packages/playwright-core/src/server/injected/**', + 'packages/playwright-core/src/server/isomorphic/**', 'utils/generate_injected.js', ], script: 'utils/generate_injected.js',