Playwright: Simplify page.evaluate function

Created on 28 Jan 2020  路  3Comments  路  Source: microsoft/playwright

page.evaluate is awesome. You pass 'window.innerWidth' and it will return the width, but it gets even better when you just type a javascript function there and its executed on the browser, crazy world...

evaluate was not so simple to implement in .NET in the past. Mostly because we don't write javascript code, of course. So both an "expression" and a "function" are string arguments.

I didn't want to parse the string to evaluate if the what we get is a function or an expressi贸n (in fact, some times it is even hard to you to know what you are getting).
You could say, well... evaluate parenthesis, but the difference between this () => (34 + 23) * (23) and this (34 + 23) * (23) is hard to tell.

So I ended up with two methods EvaluateExpressionAsync and EvaluateFunctionAsync. You need to know what function use depending on what you want to send to the browser.

I also found a super common error on pptr (Node.JS) users. It's not that clear for them that the function passed to the evaluate function is being executed in the Browser context. They might want to use variables from the Node context inside the evaluate. The javascript compiler will think that the javascript is valid because it's technically valid, but the code will fail.

So, my thoughts are:

  • Are "expressions" a feature people are using? Can't we go to only functions?
  • Do we buy having two functions?
  • Is the evaluate name clear enough?

I would also like to hear from you guys

  • Does the method split make sense in .NET for you?
  • Do you think there is a nice bullet-proof Regex I can use to determine if the string I'm getting it's an expressi贸n or a function?

Most helpful comment

My turn for "how hard" @JoelEinbinder. How hard would be moving most of the logic from evaluate to the browser? (don't kill me).
Something like: This is my expresion || function.toString(), these are the arguments, you decide whether to call Runtime.evaluate or Runtime.callFunctionOn.

Oh! Easy!

You can do it today by doing what playwright does it node, and just run it on the browser side.

    try {
      new Function('(' + functionText + ')');
    } catch (e1) {
      // This means we might have a function shorthand. Try another
      // time prefixing 'function '.
      if (functionText.startsWith('async '))
        functionText = 'async function ' + functionText.substring('async '.length);
      else
        functionText = 'function ' + functionText;
      try {
        new Function('(' + functionText  + ')');
      } catch (e2) {
        // We tried hard to serialize, but there's a weird beast here.
        throw new Error('Passed function is not well-serializable!');
      }
    }

Alternatively Runtime.compileScript exists in the DevTools protocol to do this for you. We can add it to other browsers as well.

This does an extra IPC hop, over just calling Runtime.evaluate or Runtime.callFunctionOn. If you do the new Function stuff, sometimes CSP restrictions will block you.

EDIT: Proofreading

All 3 comments

How hard would it be to get a really JavaScript parser in .NET? That's what we are doing in NodeJS with new Function.

The fact that we stringify functions is definitely a source of confusion for new users. But it provides too much value to remove. Without it, syntax highlighting, autocomplete, and typechecking wouldn't work.

I don't like two methods, because it ends up infecting every place in the api where we evaluate javascript, which is a lot of places.

You can implement EvaluateExpressionAsync via EvaluateFunctionAsync, and vice versa. Maybe it would make sense to have only one, and an extra orthogonal method like FunctionToExpression to ExpressionToFunction that you can wrap your string with. Maybe it can also bind arguments?

How hard would it be to get a really JavaScript parser in .NET? That's what we are doing in NodeJS with new Function.

That could be one option, but I don't want to add a full beast javascript engine as a dependency if I can avoid that.

The fact that we stringify functions is definitely a source of confusion for new users. But it provides too much value to remove. Without it, syntax highlighting, autocomplete, and typechecking wouldn't work.

My thought here was more about the name evaluate. But that was just a thought.

I don't like two methods, because it ends up infecting every place in the api where we evaluate javascript, which is a lot of places.

馃憤

You can implement EvaluateExpressionAsync via EvaluateFunctionAsync, and vice versa. Maybe it would make sense to have only one, and an extra orthogonal method like FunctionToExpression to ExpressionToFunction that you can wrap your string with. Maybe it can also bind arguments?

But that would mean exposing to functions. If Playwright will stick to one function I should try to do the same.
Maybe I could implement some fallbacks?

  • No parens? => expresion
  • Begins with function => function
  • Some regex for (foo, bar) => => function
  • If not I could send some function to the browser and rely on that javascript engine...

My turn for "how hard" @JoelEinbinder. How hard would be moving most of the logic from evaluate to the browser? (don't kill me).
Something like: This is my expresion || function.toString(), these are the arguments, you decide whether to call Runtime.evaluate or Runtime.callFunctionOn.

Thank you man.

My turn for "how hard" @JoelEinbinder. How hard would be moving most of the logic from evaluate to the browser? (don't kill me).
Something like: This is my expresion || function.toString(), these are the arguments, you decide whether to call Runtime.evaluate or Runtime.callFunctionOn.

Oh! Easy!

You can do it today by doing what playwright does it node, and just run it on the browser side.

    try {
      new Function('(' + functionText + ')');
    } catch (e1) {
      // This means we might have a function shorthand. Try another
      // time prefixing 'function '.
      if (functionText.startsWith('async '))
        functionText = 'async function ' + functionText.substring('async '.length);
      else
        functionText = 'function ' + functionText;
      try {
        new Function('(' + functionText  + ')');
      } catch (e2) {
        // We tried hard to serialize, but there's a weird beast here.
        throw new Error('Passed function is not well-serializable!');
      }
    }

Alternatively Runtime.compileScript exists in the DevTools protocol to do this for you. We can add it to other browsers as well.

This does an extra IPC hop, over just calling Runtime.evaluate or Runtime.callFunctionOn. If you do the new Function stuff, sometimes CSP restrictions will block you.

EDIT: Proofreading

Was this page helpful?
0 / 5 - 0 ratings

Related issues

osmenia picture osmenia  路  4Comments

TrySound picture TrySound  路  4Comments

Sue9445 picture Sue9445  路  4Comments

etnbrd picture etnbrd  路  4Comments

andyricchuiti picture andyricchuiti  路  4Comments