Skip to content

[🚀 Feature]: [dotnet] Reevaluate Add* methods in DriverOptions #15700

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

Open
nvborisenko opened this issue May 5, 2025 · 4 comments
Open

[🚀 Feature]: [dotnet] Reevaluate Add* methods in DriverOptions #15700

nvborisenko opened this issue May 5, 2025 · 4 comments
Labels
A-needs-triaging A Selenium member will evaluate this soon! C-dotnet .NET Bindings I-enhancement Something could be better

Comments

@nvborisenko
Copy link
Member

nvborisenko commented May 5, 2025

Description

To initiate Options object we have to write:

var options = new ChromeOptions { UseWebSocketUrl = true };

options.AddArgument("remote-debugging-port=9222");

At the same time we still have an access to:

options.Arguments // ReadOnlyCollection

In general it looks like

Image

Have you considered any alternatives or workarounds?

What if we remove all Add* methods and allow collections to be natively mutable? It will significantly simplify usage:

var options = new ChromeOptions
{
  UseWebSocketUrl = true.
  Arguments = { "remote-debugging-port=9222" }
};

And finally even to be focused on driver initialization without any intermediate local variables:

var driver = new ChromeDriver(new ChromeOptions
{
  UseWebSocketUrl = true.
  Arguments = { "remote-debugging-port=9222" }
});
@nvborisenko nvborisenko added A-needs-triaging A Selenium member will evaluate this soon! I-enhancement Something could be better labels May 5, 2025
@nvborisenko
Copy link
Member Author

@RenderMichael @YevgeniyShunevych what do you think? Any hidden stones I missed?

It is a breaking change. If we see benefits, then we can consider it as part of major v5.

@github-actions github-actions bot added the C-dotnet .NET Bindings label May 5, 2025
@selenium-ci
Copy link
Member

@nvborisenko, thank you for creating this issue. We will troubleshoot it as soon as we can.

Selenium Triage Team: remember to follow the Triage Guide

@RenderMichael
Copy link
Contributor

I agree that this is a good idea. we can turn Arguments into an IList<string> and depreciate the AddArguments method.

However, this would likely be a pretty big change for users. Every user that sets an argument will be using this method.

I suggest we make the change for Arguments, then leave the AddArguments method as well, at least for a while.

@YevgeniyShunevych
Copy link
Contributor

I generally like the idea. Just to mention that there is a plenty of such lists and methods related to them. And for consistency it would be good to change either all of the lists or none.

Image

Image

But there are also private dictionaries with similar Add* methods. For dictionaries, there are no read-only properties, only private fields and public Add* methods. Do you want to do something with dictionaries too, or skip? Among dictionary-related methods there is the most problematic AddAdditionalChromiumOption method, which has a tricky capability name validation.

Image

Simple moving to public dictionary property will lose the validation, so need to think about the way to handle that. As an option, the validation can be moved to the later phase, for example to ToCapabilities method.

For lists there is also a problematic and strange Extensions property:

Image

AddExtensions method also contains tricky file validation.

Summing up, I think it will be a good change. But it needs a careful effort. As validation (including nulls, empty strings, files, etc.) will vanish during options initialization, it should be added (and reviewed) to ToCapabilities method.

And yes, it would good, as usually, to leave old methods marked with ObsoleteAttribute for a few minor versions.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-needs-triaging A Selenium member will evaluate this soon! C-dotnet .NET Bindings I-enhancement Something could be better
Projects
None yet
Development

No branches or pull requests

4 participants