Description

Let's see how confident we are about BiDi API. In general I like the applied approach, but with some concerns.

• driver.AsBiDiAsync() extension method should be finally live in AnyDriver class. Thus AnyDriver object is an owner of the BiDi instance, and dispose BiDi as soon as owner is disposed. Not sure, but I think IWebDriver interface is a great candidate to have this new AsBiDiAsync() method.
  It is important. Finally we will have straightforward dependency: Selenium.Webdriver -> Selenium.WebDriver.BiDi.
• Really expose all hidden methods from users, such as AddIntercept(). So just make it public instead of internal, that's it. [🚀 Feature]: [dotnet] [bidi] Expose BiDi to be truly public low-level API #15612
• Revisit InterceptRequestAsync() method (and neighbors: response, auth). This method(s) provides more convenient API to intercept network. Literally saying it should be extensions built on top of already publicly available methods.
  UPD: .NET 10 provides new extensions capabilities, where we can create properties. Amazing, so the decision here would be move all helpers/extensions to dedicated namespace.
  RESOLUTION: Removed them at all to be low-level, extension methods are still in mind.
• EmptyResult should be valid return type for all methods instead of void. [🚀 Feature]: [dotnet] [bidi] Introduce EmptyResult as a type #15562
• Normal types as event args (inheritance topic) [🚀 Feature]: [dotnet] [bidi] Re-think about EventArgs #15546 Solution: introduce non-serializable top-level *EventArgs, and construct it from spec defined type (including IBiDi hydration!) [dotnet] [bidi] Revise event args #17304
• We have more and more strange syntax sugar, like Result class implements IReadOnlyList<T> -

  selenium/dotnet/src/webdriver/BiDi/BrowsingContext/GetTreeCommand.cs

  Line 50 in 55f02a9

  public sealed record GetTreeResult : EmptyResult, IReadOnlyList<BrowsingContextInfo>

  . It is useful definitely, but seems it is overkill. We can create extensions, like .GetTreeAsync().AsContexts(). In any case I propose to remove IReadOnlyList<T>, and more important remove strange json converters for these types. - [dotnet] [bidi] Remove IEnumerable of command results #16219
• Commands should return Result instead of one object of this Result. Example: SetCookieAsync return Task<PartitionKey> instead of Task<SetCookieResult>. Review all commands and return exact type. - [dotnet] [bidi] Specific result type for any command #16405
• Command options should be immutable? Like when I do await bidi.DoSomethingAsync(new DoSomethingOptions(...)) then DoSomethingOptions object should be constructed once and I should not be able to change its state. - Internally we capture command options into immutable object, it is safe that user might change his own options. It is still not safe, we can use records with {get;init;}. - [dotnet] [bidi] Fully immutable commands and events #17077
• Remove stateful json converters (who is dependent on bidi object)! There is no big benefit. - We nicely resolved it technically, still exposing BiDi instance for extensibility, and still perfectly AOT trimming friendly. [dotnet] [bidi] Stateful converters with hydration #16670
• Scoped event handlers: remove or keep: [🚀 Feature]: [bidi] Multiple scoped event listeners/handlers #16604 [dotnet] [bidi] Remove browsing scoped events at core level #16694
• Expose functionality via interfaces? IBiDi/IBrowserModule? What benefits, testability?
• Discriminated unions are coming back (Add commands for screencasting w3c/webdriver-bidi#1069 (comment)). How to implement?.. Supporting netstandard2.0/net462 target.. History: [dotnet] [bidi] Earlier preview feedback gathering  #14530.
  UPD: Seems unions semantically will be without json discriminator, fuh. So, continue using polymorphic types, just handle this special case manually.
• Extensible command/result/event (2 levels of extensibility: root json object and params object)
• IReadOnly vs IEnumerable. Currently we accept IEnumerable as incoming interface, meaning we can fail with Collection was modified during serialization. This is only one interface for commands execution which is not truly safe. Better way is to accept materialized collections. This is still convenient for users, I believe.
• Anything else?..

Have you considered any alternatives or workarounds?

This issue is for tracking possible big changes in the approach. As soon as we resolve all of them, then we are confident with the approach and can move further (like documenting and fine tuning).