-
-
Notifications
You must be signed in to change notification settings - Fork 443
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
We don't need no dynamics #823
Changes from 4 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
using Newtonsoft.Json.Linq; | ||
|
||
namespace PuppeteerSharp.Messaging | ||
{ | ||
internal class EvaluateHandleResponse | ||
{ | ||
public JToken ExceptionDetails { get; set; } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. shouldn't this be a class? |
||
public JToken Result { get; set; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
using Newtonsoft.Json; | ||
using PuppeteerSharp.Media; | ||
|
||
namespace PuppeteerSharp.Messaging | ||
{ | ||
internal class PageCaptureScreenshotRequest | ||
{ | ||
[JsonProperty(NullValueHandling = NullValueHandling.Ignore)] | ||
public string Format { get; set; } | ||
[JsonProperty(NullValueHandling = NullValueHandling.Ignore)] | ||
public int Quality { get; set; } | ||
[JsonProperty(NullValueHandling = NullValueHandling.Ignore)] | ||
public Clip Clip { get; set; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,11 @@ | ||
namespace PuppeteerSharp.Messaging | ||
using Newtonsoft.Json.Linq; | ||
|
||
namespace PuppeteerSharp.Messaging | ||
{ | ||
internal class PageConsoleResponse | ||
{ | ||
public ConsoleType Type { get; set; } | ||
public dynamic[] Args { get; set; } | ||
public JToken[] Args { get; set; } | ||
public int ExecutionContextId { get; set; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
using Newtonsoft.Json.Linq; | ||
|
||
namespace PuppeteerSharp.Messaging | ||
{ | ||
internal class RuntimeQueryObjectsResponse | ||
{ | ||
public JToken Objects { get; set; } | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,7 +1,6 @@ | ||
using System; | ||
using System.Collections.Generic; | ||
using System.Diagnostics; | ||
using System.Dynamic; | ||
using System.Globalization; | ||
using System.IO; | ||
using System.Linq; | ||
|
@@ -1679,18 +1678,19 @@ private async Task<string> PerformScreenshot(ScreenshotType type, ScreenshotOpti | |
} | ||
} | ||
|
||
dynamic screenMessage = new ExpandoObject(); | ||
|
||
screenMessage.format = type.ToString().ToLower(); | ||
var screenMessage = new PageCaptureScreenshotRequest | ||
{ | ||
Format = type.ToString().ToLower() | ||
}; | ||
|
||
if (options.Quality.HasValue) | ||
{ | ||
screenMessage.quality = options.Quality.Value; | ||
screenMessage.Quality = options.Quality.Value; | ||
} | ||
|
||
if (clip != null) | ||
{ | ||
screenMessage.clip = clip; | ||
screenMessage.Clip = clip; | ||
} | ||
|
||
var result = await Client.SendAsync<PageCaptureScreenshotResponse>("Page.captureScreenshot", screenMessage).ConfigureAwait(false); | ||
|
@@ -1797,7 +1797,7 @@ private async void Client_MessageReceived(object sender, MessageEventArgs e) | |
OnDetachedFromTarget(e); | ||
break; | ||
case "Log.entryAdded": | ||
OnLogEntryAdded(e.MessageData.ToObject<LogEntryAddedResponse>(true)); | ||
await OnLogEntryAddedAsync(e.MessageData.ToObject<LogEntryAddedResponse>(true)).ConfigureAwait(false); | ||
break; | ||
case "Runtime.bindingCalled": | ||
await OnBindingCalled(e.MessageData.ToObject<BindingCalledResponse>(true)).ConfigureAwait(false); | ||
|
@@ -1851,6 +1851,7 @@ private async Task OnBindingCalled(BindingCalledResponse e) | |
|
||
private async Task<object> ExecuteBinding(BindingCalledResponse e) | ||
{ | ||
const string taskResultPropertyName = "Result"; | ||
object result; | ||
var binding = _pageBindings[e.BindingPayload.Name]; | ||
var methodParams = binding.Method.GetParameters().Select(parameter => parameter.ParameterType).ToArray(); | ||
|
@@ -1865,7 +1866,7 @@ private async Task<object> ExecuteBinding(BindingCalledResponse e) | |
if (taskResult.GetType().IsGenericType) | ||
{ | ||
// the task is already awaited and therefore the call to property Result will not deadlock | ||
result = ((dynamic)taskResult).Result; | ||
result = taskResult.GetType().GetProperty(taskResultPropertyName).GetValue(taskResult); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. the old way was weird but this also feels a bit odd, is there a clean way to do this? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I open to new ideas 😂 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Created an issue for this #824 |
||
} | ||
} | ||
|
||
|
@@ -1904,13 +1905,13 @@ private async Task OnAttachedToTarget(MessageEventArgs e) | |
WorkerCreated?.Invoke(this, new WorkerEventArgs(worker)); | ||
} | ||
|
||
private void OnLogEntryAdded(LogEntryAddedResponse e) | ||
private async Task OnLogEntryAddedAsync(LogEntryAddedResponse e) | ||
{ | ||
if (e.Entry.Args != null) | ||
{ | ||
foreach (var arg in e.Entry?.Args) | ||
{ | ||
RemoteObjectHelper.ReleaseObject(Client, arg, _logger); | ||
await RemoteObjectHelper.ReleaseObjectAsync(Client, arg, _logger); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this missing |
||
} | ||
} | ||
if (e.Entry.Source != TargetType.Worker) | ||
|
@@ -1982,7 +1983,7 @@ private void OnDialog(PageJavascriptDialogOpeningResponse message) | |
private Task OnConsoleAPI(PageConsoleResponse message) | ||
{ | ||
var ctx = _frameManager.ExecutionContextById(message.ExecutionContextId); | ||
var values = message.Args.Select<dynamic, JSHandle>(i => ctx.CreateJSHandle(i)).ToArray(); | ||
var values = message.Args.Select<JToken, JSHandle>(i => ctx.CreateJSHandle(i)).ToArray(); | ||
return AddConsoleMessage(message.Type, values); | ||
} | ||
|
||
|
@@ -1992,7 +1993,7 @@ private async Task AddConsoleMessage(ConsoleType type, JSHandle[] values) | |
{ | ||
foreach (var arg in values) | ||
{ | ||
await RemoteObjectHelper.ReleaseObject(Client, arg.RemoteObject, _logger).ConfigureAwait(false); | ||
await RemoteObjectHelper.ReleaseObjectAsync(Client, arg.RemoteObject, _logger).ConfigureAwait(false); | ||
} | ||
|
||
return; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we want to create a class for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the shape of the Token (adding this for reference)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
fair enough
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will implement a class @Meir017 the shape will be a composition of that ;)