Refactor Sobek out of locator business logic

[ankur22]

What?

This moves the use of Sobek into the mapping layer for locator and away from the business logic.

Why?

Separating the two will help make things clearer and might also prevent race conditions with Sobek.

Checklist

• I have performed a self-review of my code.
• I have commented on my code, particularly in hard-to-understand areas.
• I have added tests for my changes.
• I have run linter and tests locally (make check) and all pass.

Checklist: Documentation (only for k6 maintainers and if relevant)

Please do not merge this PR until the following items are filled out.

• I have added the correct milestone and labels to the PR.
• I have updated the release notes: link
• I have updated or added an issue to the k6-documentation: grafana/k6-docs#NUMBER if applicable
• I have updated or added an issue to the TypeScript definitions: grafana/k6-DefinitelyTyped#NUMBER if applicable

Related PR(s)/Issue(s)

Closes: #4223

[inancgumus]

LGTM 👍

Since we don't need/want Sobek in the business logic, it'd be great if you could move the option parsing to the mapping layer as well, like this. Maybe in a separate PR?

[ankur22]

Since we don't need/want Sobek in the business logic, it'd be great if you could move the option parsing to the mapping layer as well, like this. Maybe in a separate PR?

Sure, in another PR 👍

[ankur22]

@inancgumus i explored moving the parse logic, but it's a lot of changes, and I'm not sure this strategy is quite the right one we should be doing. Instead of just moving the parse logic to the browser package, I think we should move the types and the parse logic to a new package, one that both common and browser can reference without cyclic dependency issues. It feels odd to have types in common, parse in browser.

Part of what i explored also brought out the issue of having to refactor frame, page and elementHandle too so that I could remove the parse logic completely from the browser package, and not leave duplicates around. Again, a lot of work.

I propose we think about this properly and set out a plan to enable us to refactor with a bit more ease and that everyone agrees on.

For now, i've abandoned the plan to move the parse logic out into common.

[inancgumus]

@ankur22

The common package should not care about how options are parsed (Sobek), but it needs options (Go types). We've been moving the parsing functionality to the browser package for quite a while. Since only the mapping layer requires parsing Sobek options, I feel like it's the right strategy to move that logic into where it's done (browser).

We can also move parsing to another package if it becomes unwieldy. I can give another look at this myself. Maybe we're not talking about the same thing.