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

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

Open
WhitWaldo opened this issue Sep 18, 2023 · 2 comments

Comments

@WhitWaldo
Copy link

WhitWaldo commented Sep 18, 2023

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 WhitWaldo changed the title [BUG] EvaluateAsync deserializes correctly to a class, but not to a record with the same shape [BUG] EvaluateAsync deserializes correctly to a class, but not to a record with the same shape but using init setter Sep 18, 2023
@campersau
Copy link
Contributor

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>();

@WhitWaldo WhitWaldo changed the title [BUG] EvaluateAsync deserializes correctly to a class, but not to a record with the same shape but using init setter [BUG] EvaluateAsync deserializes correctly to a class, but not to a record without parameterless constructor Sep 18, 2023
@WhitWaldo
Copy link
Author

WhitWaldo commented Sep 18, 2023

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.

@mxschmitt mxschmitt changed the title [BUG] EvaluateAsync deserializes correctly to a class, but not to a record without parameterless constructor [FEATURE] EvaluateAsync deserializes correctly to a class, but not to a record without parameterless constructor Sep 18, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants