From 29b8671d7ef710ad54cbb35faaea35468a5fd081 Mon Sep 17 00:00:00 2001
From: Caleb Eby <calebeby@users.noreply.github.com>
Date: Thu, 24 Aug 2023 09:29:48 -0700
Subject: [PATCH] Allow returning non-serializable values from `waitFor` (#732)

---
 .changeset/proud-spies-taste.md |  5 ++++
 jest.config.cjs                 |  4 ++++
 src/pptr-testing-library.ts     | 42 +++++++++++++--------------------
 tests/wait-for.test.ts          | 27 ++++++++++++++++++++-
 4 files changed, 52 insertions(+), 26 deletions(-)
 create mode 100644 .changeset/proud-spies-taste.md

diff --git a/.changeset/proud-spies-taste.md b/.changeset/proud-spies-taste.md
new file mode 100644
index 00000000..43448212
--- /dev/null
+++ b/.changeset/proud-spies-taste.md
@@ -0,0 +1,5 @@
+---
+'pleasantest': minor
+---
+
+Allow returning non-serializable values from `waitFor`
diff --git a/jest.config.cjs b/jest.config.cjs
index b4d93acf..97f098fc 100644
--- a/jest.config.cjs
+++ b/jest.config.cjs
@@ -1,4 +1,8 @@
 module.exports = {
+  // See https://jestjs.io/docs/configuration/#prettierpath-string -> "Prettier version 3 is not supported!"
+  // Blocked by https://github.com/prettier/prettier-synchronized/issues/4#issuecomment-1649355749
+  // and https://github.com/jestjs/jest/pull/14311#issuecomment-1649358074
+  prettierPath: null,
   testEnvironment: 'node',
   moduleNameMapper: {
     '^pleasantest$': '<rootDir>/dist/cjs/index.cjs',
diff --git a/src/pptr-testing-library.ts b/src/pptr-testing-library.ts
index e399b7bf..8f5ff746 100644
--- a/src/pptr-testing-library.ts
+++ b/src/pptr-testing-library.ts
@@ -247,22 +247,12 @@ export interface WaitForOptions {
   mutationObserverOptions?: MutationObserverInit;
 }
 
-interface WaitFor {
-  <T>(
-    page: Page,
-    asyncHookTracker: AsyncHookTracker,
-    cb: () => T | Promise<T>,
-    { onTimeout, container, ...opts }: WaitForOptions,
-    wrappedFunction: (...args: any) => any,
-  ): Promise<T>;
-}
-
-export const waitFor: WaitFor = async (
-  page,
-  asyncHookTracker,
-  cb,
-  { onTimeout, container, ...opts },
-  wrappedFunction,
+export const waitFor = async <T>(
+  page: Page,
+  asyncHookTracker: AsyncHookTracker,
+  cb: () => T | Promise<T>,
+  { onTimeout, container, ...opts }: WaitForOptions,
+  wrappedFunction: (...args: any) => any,
 ) =>
   asyncHookTracker.addHook(async () => {
     const { port } = await createClientRuntimeServer();
@@ -272,7 +262,10 @@ export const waitFor: WaitFor = async (
     // So we need a unique name for each variable
     const browserFuncName = `pleasantest_waitFor_${waitForCounter}`;
 
-    await page.exposeFunction(browserFuncName, cb);
+    let returnValue: T | undefined;
+    await page.exposeFunction(browserFuncName, async () => {
+      returnValue = await cb();
+    });
 
     const evalResult = await page.evaluateHandle(
       // Using new Function to avoid babel transpiling the import
@@ -282,8 +275,8 @@ export const waitFor: WaitFor = async (
         `return import("http://localhost:${port}/@pleasantest/dom-testing-library")
           .then(async ({ waitFor }) => {
             try {
-              const result = await waitFor(${browserFuncName}, { ...opts, container })
-              return { success: true, result }
+              await waitFor(${browserFuncName}, { ...opts, container })
+              return { success: true }
             } catch (error) {
               if (/timed out in waitFor/i.test(error.message)) {
                 // Leave out stack trace so the stack trace is given from Node
@@ -298,12 +291,11 @@ export const waitFor: WaitFor = async (
       container,
     );
     const wasSuccessful = await evalResult.evaluate((r) => r.success);
-    const result = await evalResult.evaluate((r) =>
-      r.success
-        ? r.result
-        : { message: r.result.message, stack: r.result.stack },
-    );
-    if (wasSuccessful) return result;
+    if (wasSuccessful) return returnValue as T;
+    const result = await evalResult.evaluate((r) => ({
+      message: r.result.message,
+      stack: r.result.stack,
+    }));
     const err = new Error(result.message);
     if (result.stack) err.stack = result.stack;
     else removeFuncFromStackTrace(err, asyncHookTracker.addHook);
diff --git a/tests/wait-for.test.ts b/tests/wait-for.test.ts
index 64f5cd4f..6a9fe31c 100644
--- a/tests/wait-for.test.ts
+++ b/tests/wait-for.test.ts
@@ -25,6 +25,29 @@ test(
   }),
 );
 
+test(
+  'Returned non-serializable values',
+  withBrowser(async ({ utils, page, waitFor }) => {
+    await utils.runJS(`
+      setTimeout(() => {
+        document.title = 'hallo'
+      }, 100)
+    `);
+    // At first the title won't be set to hallo
+    // Because it waits 100ms before it sets it
+    expect(await page.title()).not.toEqual('hallo');
+    const imNotSerializable = Symbol('test');
+    const waitForCallback = jest.fn(async () => {
+      expect(await page.title()).toEqual('hallo');
+      return imNotSerializable;
+    });
+    const returnedValue = await waitFor(waitForCallback);
+    expect(returnedValue).toBe(imNotSerializable);
+    expect(await page.title()).toEqual('hallo');
+    expect(waitForCallback).toHaveBeenCalled();
+  }),
+);
+
 test(
   'Throws error with timeout',
   withBrowser(async ({ waitFor }) => {
@@ -40,7 +63,9 @@ test(
       tests/wait-for.test.ts
 
               throw new Error('something bad happened');
-                    ^"
+                    ^
+      -------------------------------------------------------
+      dist/cjs/index.cjs"
     `);
 
     // If the callback function never resolves (or takes too long to resolve),