Merge pull request #16 from link-foundation/issue-15-85e23cad40a2

fix: PlaywrightAdapter.evaluateOnPage() spread multiple arguments correctly

Author: konard

js/.changeset/fix-playwright-evaluate-multiple-args.md

@@ -0,0 +1,9 @@
+---
+'browser-commander': patch
+---
+
+Fix PlaywrightAdapter.evaluateOnPage() to spread multiple arguments correctly
+
+When using `evaluateOnPage()` with multiple arguments, the arguments are now properly spread to the function in the browser context, matching Puppeteer's behavior.
+
+Previously, the function would receive the entire array as its first parameter instead of spread arguments, causing issues like invalid selectors when passing selector + array combinations.

js/src/core/engine-adapter.js

@@ -304,13 +304,25 @@ export class PlaywrightAdapter extends EngineAdapter {
 
   async evaluateOnPage(fn, args = []) {
     // Playwright only accepts a single argument (can be array/object)
+    // To match Puppeteer's behavior where args are spread, we wrap the function
+    // and pass all args as a single array, then apply them in the browser context
     if (args.length === 0) {
       return await this.page.evaluate(fn);
     } else if (args.length === 1) {
       return await this.page.evaluate(fn, args[0]);
     } else {
-      // Multiple args - pass as array
-      return await this.page.evaluate(fn, args);
+      // Multiple args - wrap function to accept array and spread them
+      // This makes Playwright behave like Puppeteer's spread behavior
+      // We pass the function string and args array, then reconstruct and call in browser
+      const fnString = fn.toString();
+      return await this.page.evaluate(
+        ({ fnStr, argsArray }) => {
+          // Reconstruct the function in browser context and call with spread args
+          const reconstructedFn = new Function(`return (${fnStr})`)();
+          return reconstructedFn(...argsArray);
+        },
+        { fnStr: fnString, argsArray: args }
+      );
     }
   }
 

js/tests/helpers/mocks.js

@@ -105,28 +105,15 @@ export function createMockPlaywrightPage(options = {}) {
       return locs.map((l) => l.evaluate(fn, ...args));
     },
     locator: locatorMock,
-    evaluate: async (fn, ...args) => {
+    // Playwright's page.evaluate() only accepts a single argument (not spread)
+    // This is the key difference from Puppeteer
+    evaluate: async (fn, arg) => {
       if (evaluateResult !== null) {
         return evaluateResult;
       }
-      // Create mock window/document context
-      const mockContext = {
-        innerHeight: 800,
-        innerWidth: 1200,
-        sessionStorage: {
-          _data: {},
-          getItem: (key) => mockContext.sessionStorage._data[key] || null,
-          setItem: (key, val) => {
-            mockContext.sessionStorage._data[key] = val;
-          },
-          removeItem: (key) => {
-            delete mockContext.sessionStorage._data[key];
-          },
-        },
-        querySelectorAll: () => [],
-      };
       try {
-        return fn(...args);
+        // Playwright passes exactly one argument (can be object/array)
+        return fn(arg);
       } catch {
         return fn;
       }

js/tests/unit/core/engine-adapter.test.js

@@ -78,6 +78,53 @@ describe('engine-adapter', () => {
       const count = await adapter.count('button');
       assert.strictEqual(count, 5);
     });
+
+    describe('evaluateOnPage', () => {
+      it('should handle zero arguments', async () => {
+        page = createMockPlaywrightPage();
+        adapter = new PlaywrightAdapter(page);
+        const result = await adapter.evaluateOnPage(() => 42);
+        assert.strictEqual(result, 42);
+      });
+
+      it('should handle single argument', async () => {
+        page = createMockPlaywrightPage();
+        adapter = new PlaywrightAdapter(page);
+        const result = await adapter.evaluateOnPage((x) => x * 2, [5]);
+        assert.strictEqual(result, 10);
+      });
+
+      it('should handle multiple arguments by spreading them', async () => {
+        page = createMockPlaywrightPage();
+        adapter = new PlaywrightAdapter(page);
+        // This is the bug case - multiple args should be spread, not passed as array
+        const result = await adapter.evaluateOnPage((a, b) => a + b, [3, 7]);
+        assert.strictEqual(result, 10);
+      });
+
+      it('should handle selector + array arguments (real-world bug case)', async () => {
+        page = createMockPlaywrightPage();
+        adapter = new PlaywrightAdapter(page);
+        // This reproduces the original bug: selector and processedIds
+        const result = await adapter.evaluateOnPage(
+          (selector, processedIds) =>
+            `Selector: ${selector}, Count: ${processedIds.length}`,
+          ['[data-qa="test"]', ['id1', 'id2']]
+        );
+        assert.ok(result.includes('Selector: [data-qa="test"]'));
+        assert.ok(result.includes('Count: 2'));
+      });
+
+      it('should handle multiple arguments of different types', async () => {
+        page = createMockPlaywrightPage();
+        adapter = new PlaywrightAdapter(page);
+        const result = await adapter.evaluateOnPage(
+          (str, num, obj) => `${str}-${num}-${obj.key}`,
+          ['hello', 42, { key: 'world' }]
+        );
+        assert.strictEqual(result, 'hello-42-world');
+      });
+    });
   });
 
   describe('PuppeteerAdapter', () => {
@@ -130,6 +177,42 @@ describe('engine-adapter', () => {
       const count = await adapter.count('button');
       assert.strictEqual(count, 5);
     });
+
+    describe('evaluateOnPage', () => {
+      it('should handle zero arguments', async () => {
+        page = createMockPuppeteerPage();
+        adapter = new PuppeteerAdapter(page);
+        const result = await adapter.evaluateOnPage(() => 42);
+        assert.strictEqual(result, 42);
+      });
+
+      it('should handle single argument', async () => {
+        page = createMockPuppeteerPage();
+        adapter = new PuppeteerAdapter(page);
+        const result = await adapter.evaluateOnPage((x) => x * 2, [5]);
+        assert.strictEqual(result, 10);
+      });
+
+      it('should handle multiple arguments by spreading them', async () => {
+        page = createMockPuppeteerPage();
+        adapter = new PuppeteerAdapter(page);
+        // Puppeteer natively spreads args - ensure same behavior as Playwright fix
+        const result = await adapter.evaluateOnPage((a, b) => a + b, [3, 7]);
+        assert.strictEqual(result, 10);
+      });
+
+      it('should handle selector + array arguments (parity with PlaywrightAdapter)', async () => {
+        page = createMockPuppeteerPage();
+        adapter = new PuppeteerAdapter(page);
+        const result = await adapter.evaluateOnPage(
+          (selector, processedIds) =>
+            `Selector: ${selector}, Count: ${processedIds.length}`,
+          ['[data-qa="test"]', ['id1', 'id2']]
+        );
+        assert.ok(result.includes('Selector: [data-qa="test"]'));
+        assert.ok(result.includes('Count: 2'));
+      });
+    });
   });
 
   describe('createEngineAdapter', () => {