playwright-dotnet icon indicating copy to clipboard operation
playwright-dotnet copied to clipboard

[FEATURE] EvaluateAsync deserializes correctly to a class, but not to a record without parameterless constructor

Open WhitWaldo opened this issue 1 year ago • 2 comments

System info

  • Playwright Version: [v1.37.1] (also reproduced in clone of latest main branch)
  • Operating System: Windows 10
  • Browser: Chromium

Source code

The crux of the issue is that Page.EvaluateAsync does not deserialize to a record and throws an exception. Deserializing to a class with the same members works without issue. See tests below:

Working deserialization of EvaluateAsync

This one works - it deserializes to the indicated class.

    private class TestClass
    {
        [JsonPropertyName("name")]
        public string Name { get; set; }

        [JsonPropertyName("count")]
        public int Count { get; set; }
    }

    [TestMethod]
    public async Task DeserializeToClassTest()
    {
        await Page.GotoAsync("https://www.github.com");
        var jsExpr = """
                     var doWork = function() {
                        var val = {"name": "Playwright", "count": 5};
                        return val;
                     }

                     doWork();
                     """;
        var result = await Page.EvaluateAsync<TestClass>(jsExpr);
        Assert.AreEqual("Playwright", result.Name);
        Assert.AreEqual(5, result.Count);
    }

Not working deserialization of EvaluateAsync

This one will fail to work and throws the following exception:

Expecting My.Tests.Selectors.ParserTests+TestRecord, got Object at Microsoft.Playwright.Transport.Converters.EvaluateArgumentValueConverter.ToExpectedType(Object parsed, Type t, IDictionary2 visited) in /_/src/Playwright/Transport/Converters/EvaluateArgumentValueConverter.cs:line 246 at Microsoft.Playwright.Transport.Converters.EvaluateArgumentValueConverter.Deserialize(JsonElement result, Type t) in /_/src/Playwright/Transport/Converters/EvaluateArgumentValueConverter.cs:line 211 at Microsoft.Playwright.Core.ScriptsHelper.ParseEvaluateResult[T](Nullable1 resultOrNull) in //src/Playwright/Core/ScriptsHelper.cs:line 61 at Microsoft.Playwright.Core.Frame.EvaluateAsync[T](String script, Object arg) in //src/Playwright/Core/Frame.cs:line 555 at Scraper.Tests.Selectors.ParserTests.DeserializeToRecordTest() in G:\My.Tests\Selectors\ParserTests.cs:line 130 at Microsoft.VisualStudio.TestPlatform.MSTestAdapter.PlatformServices.ThreadOperations.ExecuteWithAbortSafety(Action action)

    private record TestRecord([property: JsonPropertyName("name")] string Name,
        [property: JsonPropertyName("count")] int Count);

    [TestMethod]
    public async Task DeserializeToRecordTest()
    {
        await Page.GotoAsync("https://www.github.com");
        var jsExpr = """
                     var doWork = function() {
                        var val = {"name": "Playwright", "count": 5};
                        return val;
                     }

                     doWork();
                     """;
        var result = await Page.EvaluateAsync<TestRecord>(jsExpr);
        Assert.AreEqual("Playwright", result.Name);
        Assert.AreEqual(5, result.Count);
    }

** Steps **

  • Paste the above snippets into a test class
  • Run the test and observe DeserializeToClassTest pass and DeserializeToRecordTest fail.

Expected

Both tests should pass and produce their respective class and record types with the deserialized values.

Actual

The class deserialization test works. The record deserialization test does not work and throws an exception.

Having played around with it a little bit, the issue appears to be the use of the init setters on the record. If one creates the TestRecord with the following signature instead:

    private record TestRecord
    {
        [JsonPropertyName("name")]
        public string Name { get; set; }

        [JsonPropertyName("count")]
        public int Count { get; set; }
    }

Deserialization will now work precisely as intended. However, given that a lot of record usage promotes using the implicit properties via the record constructor, this is an issue that should be remedied as I expect most developers would expect classes and records to be largely interchangeable and that they wouldn't have to change setter types to make deserialization work (as that's not typically something you have to do with System.Text.Json deserialization anyway).

WhitWaldo avatar Sep 18 '23 11:09 WhitWaldo

Playwright currently only supports types which have a parameterless contstructor. But you can workaround it by going through JsonElement instead.

private record TestRecord([property: JsonPropertyName("name")] string Name, [property: JsonPropertyName("count")] int Count);

JsonElement? json = await Page.EvaluateAsync("() => ({ "name": "Playwright", "count": 5 })");
var result = json.Value.Deserialize<TestRecord>();

campersau avatar Sep 18 '23 15:09 campersau

You're right - it's the lack of parameterless constructor that causes the issue. I was able to get your workaround to work locally. Should I re-file this issue as a feature request to support parameterless constructors? I don't see any open issues for that yet.

Especially with primary constructors coming up in the next C# version, I bet more people start hitting this unexpectedly.

WhitWaldo avatar Sep 18 '23 17:09 WhitWaldo