Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move Async API Handling to Mapping Layer #683

Closed
8 tasks done
Tracked by #428
inancgumus opened this issue Jan 3, 2023 · 1 comment
Closed
8 tasks done
Tracked by #428

Move Async API Handling to Mapping Layer #683

inancgumus opened this issue Jan 3, 2023 · 1 comment
Assignees
Labels
async supports async (promises) compatibility k6 core compatibility dx developer experience epic mapping k6 browser to Goja mapping related. next Might be eligible for the next planning (not guaranteed!) refactor
Milestone

Comments

@inancgumus
Copy link
Member

inancgumus commented Jan 3, 2023

This issue concerns an architectural redesign for the upcoming Async API and improving the k6 browser Async API to Goja mapping.

The problems this issue solves

  • Provide an easy switch to the k6 browser Async API.
  • Increase developer productivity by clearly separating the k6 browser code and Goja (updates #271).
  • Unlock merging of k6 browser to k6 core: Mapping Async APIs.

POC

Here's a POC for this issue.

Sub issues


Mapping Async APIs

The current problem is that when a function returns a value in a promise, we can't map the returned value in the mapping layer.

So the missing piece of the puzzle is:

  • Mapping the returned value
  • Putting the value in a promise
  • Returning it to Goja in the mapping layer.

These all should happen in the mapping layer to correctly map the Async methods.

Example

But we can't map the returned response.frame() method in the mapping layer because the response object is returned in a promise. We can't get the response object in the mapping layer until the promise is resolved (happening on the Goja side).

To understand it better, let's take a look at the following.

For example, the following code works: We use the query method to get the element.

page.goto('https://test.k6.io/', { waitUntil: 'networkidle' }).then(response => {
  ...
  const f = response.frame();
  const el = f.query('a[href="/my_messages.php"]')
  console.log(el.textContent());
  ...
});

However, the following code does not work:

page.goto('https://test.k6.io/', { waitUntil: 'networkidle' }).then(response => {
  ...
  const f = response.frame();
  const el = f.$('a[href="/my_messages.php"]') // <- notice the wildcard: $
  console.log(el.textContent());
  ...
});

Without maping the $ to the query method, the above code won't work. Notice that the above code uses $ to get the element.

Currently, we couldn't map the response.frame() in the mapping layer. So why we can't map the response.frame().$ to the response.frame().query? It's because the response.frame() function returns a promise, and we can't get the response object in the mapping layer until the promise is resolved.

// `page.Goto(...) *goja.Promise` returns a promise, and we can't get the
// `response` object from the mapping layer until the promise is resolved.
func mapPage(rt *goja.Runtime, p api.Page) mapping {
  maps := mapping{
    ...
    // p.Goto(...) *goja.Promise
    // There is no way to extract the value from the
    // promise here without being messy.
    "goto":         p.Goto,
    ...
  }
  ...
}

Solution

Instead of page.goto returning a promise, we can directly make it return a response object. And then, we can map the response.frame() method in the mapping layer.

That means we must move the promise returning/handling functionality to the mapping layer. This will be critical when we fully switch to the Async API. Since we also need it now, we can start moving the promise returning/handling functionality to the mapping layer.

Doing so will also help us separate the Goja logic from our core logic. The code and tests will be easier to maintain and work with. And we'll be ready for the incoming Async/Promise support (#428). I'll send a POC soon.

Related: #661

@inancgumus inancgumus added async supports async (promises) compatibility k6 core compatibility labels Jan 3, 2023
@inancgumus inancgumus added this to the v0.8.0 milestone Jan 3, 2023
@inancgumus inancgumus self-assigned this Jan 3, 2023
@inancgumus inancgumus added the next Might be eligible for the next planning (not guaranteed!) label Jan 3, 2023
@inancgumus inancgumus changed the title Map Async APIs to Goja Abstract Async API handling from the extension core Jan 10, 2023
@inancgumus inancgumus changed the title Abstract Async API handling from the extension core Separate Async/Promise handling from the extension core Jan 10, 2023
inancgumus added a commit that referenced this issue Jan 11, 2023
Exposing VU is preliminary for using promises in the mapping layer.

Updates: #683
inancgumus added a commit that referenced this issue Jan 11, 2023
Exposing VU is preliminary for using promises in the mapping layer.

Updates: #683
inancgumus added a commit that referenced this issue Jan 12, 2023
Exposing VU is preliminary for using promises in the mapping layer.

Updates: #683
@inancgumus inancgumus added mapping k6 browser to Goja mapping related. refactor dx developer experience labels Jan 13, 2023
@inancgumus inancgumus changed the title Separate Async/Promise handling from the extension core Pre-merge: k6 browser Async API to Goja mapping Jan 18, 2023
@inancgumus
Copy link
Member Author

Done done done 🥳

@inancgumus inancgumus changed the title Pre-merge: k6 browser Async API to Goja mapping Pre-merge: Map k6-browser Async API to Goja Jan 23, 2023
inancgumus added a commit that referenced this issue Jan 24, 2023
Related: #683
Closes: #725
inancgumus added a commit that referenced this issue Jan 24, 2023
Related: #683
inancgumus added a commit that referenced this issue Jan 24, 2023
Related: #683
Closes: #725
inancgumus added a commit that referenced this issue Jan 24, 2023
Related: #683
inancgumus added a commit that referenced this issue Jan 24, 2023
Related: #683
Closes: #725
inancgumus added a commit that referenced this issue Jan 24, 2023
Related: #683
inancgumus added a commit that referenced this issue Jan 24, 2023
Related: #683
Closes: #725
inancgumus added a commit that referenced this issue Jan 24, 2023
Related: #683
inancgumus added a commit that referenced this issue Feb 2, 2023
This is a continuation of the efforts set in #683.
Handling the Async API handling in the mapping
layer.
inancgumus added a commit that referenced this issue Feb 2, 2023
This is a continuation of the efforts set in #683.
Handling the Async API handling in the mapping
layer.
@inancgumus inancgumus changed the title Pre-merge: Map k6-browser Async API to Goja Move Async API Handling to Mapping Layer Feb 2, 2023
inancgumus added a commit that referenced this issue Feb 2, 2023
This is a continuation of the efforts set in #683.
Handling the Async API handling in the mapping
layer.
@inancgumus inancgumus mentioned this issue Aug 25, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
async supports async (promises) compatibility k6 core compatibility dx developer experience epic mapping k6 browser to Goja mapping related. next Might be eligible for the next planning (not guaranteed!) refactor
Projects
None yet
Development

No branches or pull requests

1 participant