Refactor parse our of page.click

This helps avoid using the goja runtime when it comes to parsing the
options. The options parsing has been moved to the mapping layer and so
has been moved back on to the main goja thread, which will mitigate the
possibility of a panic which could occur when multiple goroutines try
to work with the goja runtime (which is not thread safe).

There is another issue to tackle more of the goja refactoring in

Author: ankur22

browser/mapping.go

@@ -475,11 +475,16 @@ func mapPage(vu moduleVU, p *common.Page) mapping {
 		"addStyleTag":   p.AddStyleTag,
 		"bringToFront":  p.BringToFront,
 		"check":         p.Check,
-		"click": func(selector string, opts goja.Value) *goja.Promise {
+		"click": func(selector string, opts goja.Value) (*goja.Promise, error) {
+			popts, err := parseFrameClickOptions(vu.Context(), opts, p.Timeout())
+			if err != nil {
+				return nil, err
+			}
+
 			return k6ext.Promise(vu.Context(), func() (any, error) {
-				err := p.Click(selector, opts)
+				err := p.Click(selector, popts)
 				return nil, err //nolint:wrapcheck
-			})
+			}), nil
 		},
 		"close": func(opts goja.Value) error {
 			vu.taskQueueRegistry.close(p.TargetID())

common/page.go

@@ -649,15 +649,10 @@ func (p *Page) IsChecked(selector string, opts goja.Value) bool {
 }
 
 // Click clicks an element matching provided selector.
-func (p *Page) Click(selector string, opts goja.Value) error {
+func (p *Page) Click(selector string, opts *FrameClickOptions) error {
 	p.logger.Debugf("Page:Click", "sid:%v selector:%s", p.sessionID(), selector)
 
-	popts := NewFrameClickOptions(p.defaultTimeout())
-	if err := popts.Parse(p.ctx, opts); err != nil {
-		k6ext.Panic(p.ctx, "parsing click options %q: %w", selector, err)
-	}
-
-	return p.MainFrame().Click(selector, popts)
+	return p.MainFrame().Click(selector, opts)
 }
 
 // Close closes the page.

tests/element_handle_test.go

@@ -199,7 +199,7 @@ func TestElementHandleClickConcealedLink(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, wantBefore, clickResult())
 
-	err = p.Click("#concealed", nil)
+	err = p.Click("#concealed", common.NewFrameClickOptions(p.Timeout()))
 	require.NoError(t, err)
 	require.Equal(t, wantAfter, clickResult())
 }
@@ -218,7 +218,7 @@ func TestElementHandleNonClickable(t *testing.T) {
 	require.NotNil(t, resp)
 	require.NoError(t, err)
 
-	err = p.Click("#non-clickable", nil)
+	err = p.Click("#non-clickable", common.NewFrameClickOptions(p.Timeout()))
 	require.Errorf(t, err, "element should not be clickable")
 }
 

tests/frame_manager_test.go

@@ -49,7 +49,7 @@ func TestWaitForFrameNavigationWithinDocument(t *testing.T) {
 				return err
 			}
 			click := func() error {
-				return p.Click(tc.selector, nil)
+				return p.Click(tc.selector, common.NewFrameClickOptions(p.Timeout()))
 			}
 			ctx, cancel := context.WithTimeout(tb.ctx, timeout)
 			defer cancel()
@@ -105,7 +105,7 @@ func TestWaitForFrameNavigation(t *testing.T) {
 		return err
 	}
 	click := func() error {
-		return p.Click(`a`, nil)
+		return p.Click(`a`, common.NewFrameClickOptions(p.Timeout()))
 	}
 	ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
 	defer cancel()

tests/frame_test.go

@@ -9,6 +9,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/grafana/xk6-browser/common"
 	"github.com/grafana/xk6-browser/env"
 )
 
@@ -60,7 +61,7 @@ func TestFrameDismissDialogBox(t *testing.T) {
 			require.NoError(t, err)
 
 			if tt == "beforeunload" {
-				err = p.Click("#clickHere", nil)
+				err = p.Click("#clickHere", common.NewFrameClickOptions(p.Timeout()))
 				require.NoError(t, err)
 			}
 
@@ -136,7 +137,7 @@ func TestFrameNoPanicNavigateAndClickOnPageWithIFrames(t *testing.T) {
 
 	err = tb.run(
 		ctx,
-		func() error { return p.Click(`a[href="/iframeSignIn"]`, nil) },
+		func() error { return p.Click(`a[href="/iframeSignIn"]`, common.NewFrameClickOptions(p.Timeout())) },
 		func() error { _, err := p.WaitForNavigation(nil); return err },
 	)
 	require.NoError(t, err)

tests/launch_options_slowmo_test.go

@@ -31,7 +31,7 @@ func TestBrowserOptionsSlowMo(t *testing.T) {
 			t.Parallel()
 			tb := newTestBrowser(t, withFileServer())
 			testPageSlowMoImpl(t, tb, func(_ *testBrowser, p *common.Page) {
-				err := p.Click("button", nil)
+				err := p.Click("button", common.NewFrameClickOptions(p.Timeout()))
 				assert.NoError(t, err)
 			})
 		})

tests/lifecycle_wait_test.go

@@ -175,7 +175,7 @@ func TestLifecycleWaitForNavigation(t *testing.T) {
 					return err
 				}
 				click := func() error {
-					return p.Click(`a`, nil)
+					return p.Click(`a`, common.NewFrameClickOptions(p.Timeout()))
 				}
 
 				ctx, cancel := context.WithTimeout(tb.ctx, 5*time.Second)

tests/locator_test.go

@@ -474,11 +474,9 @@ func TestLocatorShadowDOM(t *testing.T) {
 
 	tb := newTestBrowser(t, withFileServer())
 	p := tb.NewPage(nil)
-	opts := tb.toGojaValue(jsFrameBaseOpts{Timeout: "1000"})
 
 	_, err := p.Goto(tb.staticURL("shadow_dom_link.html"), nil)
 	require.NoError(t, err)
-
-	err = p.Click("#inner-link", opts)
+	err = p.Click("#inner-link", common.NewFrameClickOptions(time.Second))
 	require.NoError(t, err)
 }

tests/webvital_test.go

@@ -8,6 +8,8 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
+	"github.com/grafana/xk6-browser/common"
+
 	k6metrics "go.k6.io/k6/metrics"
 )
 
@@ -61,7 +63,7 @@ func TestWebVitalMetric(t *testing.T) {
 	// also helps the web vital library to measure CLS.
 	err = browser.run(
 		ctx,
-		func() error { return page.Click("#clickMe", nil) },
+		func() error { return page.Click("#clickMe", common.NewFrameClickOptions(page.Timeout())) },
 		func() error { _, err := page.WaitForNavigation(nil); return err },
 	)
 	require.NoError(t, err)