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

unlock install for runtime #400

Open
udarrr opened this issue Nov 22, 2022 · 12 comments
Open

unlock install for runtime #400

udarrr opened this issue Nov 22, 2022 · 12 comments

Comments

@udarrr
Copy link

udarrr commented Nov 22, 2022

Hello

Is it possible unlock install function for runtime ?

There are could be just some changes

image
image

@giggio
Copy link
Owner

giggio commented Nov 28, 2022

@udarrr what is the use case? This already runs on package installation.

@udarrr
Copy link
Author

udarrr commented Nov 29, 2022

@udarrr what is the use case? This already runs on package installation.

In case when we don't know exact version of chromedriver that we need. For example when we're going to connect to electron app by debugging port. Firstable we determine version of electron and than depends on version of electron do chromedriver.install(electronChromedriverVersion)

@udarrr
Copy link
Author

udarrr commented Nov 29, 2022

Look like people already had been requested it here #256 and here #278, #332 but there were a little bit complex solutions there. I suggest just unlock install and everything are going to be fine :)

@christian-bromann
Copy link

@giggio I would like to second this request. Especially now with Chrome for Testing it doesn't make much sense to have the binary downloaded at install time because you don't know which browser version is installed or desired. IMHO for the user it makes much more sense to not have to care about that Chromedriver and Chrome are the same version. Chromedriver can just handle this for them.

Therefor I would suggest the following:

  • change the default behavior to have Chromedriver downloaded at runtime based on detected version
  • create an environment variable allowing to continue download during the installation if desired
  • stop pinning the NPM package version to the Chromedriver version as it doesn't make sense anymore given new default behavior and also simplifies maintenance

To emphasise the use case: every user just wants to automate Chrome, they don't know which version they are on and honestly it shouldn't matter. Chromedriver as a NPM package should ensure that the right driver binary is installed (bonus points if the driver could also download the right Chrome browser is browserVersion is defined but not available on the system but I guess this should be a different ticket).

I am very interested in this and would be happy to do the leg work here. I am maintaining already the other drivers where this is the default behavior.

What do you think?

@giggio
Copy link
Owner

giggio commented Jun 15, 2023

I will not significantly change how this library works by removing the Chromedriver download which happens during npm install. This would break a lot of projects that assume it works like that.

We could add an option that would not download during install, opt in. That's fine. And also another work to allow it to download on demand (#256).

@giggio
Copy link
Owner

giggio commented Jun 15, 2023

I was now thinking, I would be willing to publish a new npm package that had these set as default. We could use the same source. What do you think?

@christian-bromann
Copy link

christian-bromann commented Jun 16, 2023

Hey @giggio , thanks for replying.

I am wondering how much this change could affect users given that for them no interface will change. Only the first test executed will be minimal slower because the binary is being downloaded then. Could you clarify where you see people projects breaking?

We could add an option that would not download during install, opt in.

My use case here is that I would like to ship a minimal package for Chromedriver with the webdriver NPM module so users aren't bothered to download this dependency even though they don't run Chrome tests. I am not sure if it is possible to add chromedriver as dependency with settings that allows us to have the install happen when the user actually requires to use the driver.

I think there is no real reason to have the NPM package download the binary on install. No one installs the package and then copies the binary out of node_modules. Therefore I believe there won't be any breaking changes.

The reason why I would love to see this land is so I can make this NPM package a dependency of WebdriverIO. If the driver install only happens if the user actually wants to test Chrome, then the download time of the webdriverio package stays low too. I already made this the default behavior of all other NPM browser driver, e.g. geckodriver and edgedriver.

And also another work to allow it to download on demand (#256).

I think this person requesting the same as I did above. Allowing to skip download at install time and make it on demand.

@christian-bromann
Copy link

@giggio what do you think?

@giggio
Copy link
Owner

giggio commented Jul 18, 2023

I am wondering how much this change could affect users given that for them no interface will change. Only the first test executed will be minimal slower because the binary is being downloaded then. Could you clarify where you see people projects breaking?

It would break projects where people assume the binary is available.
The workflow this package serves, and which I do not intend to break, is:

  1. Package is added with npm i (and later the project is cloned by another person, which runs npm i)
  2. At some point chromedriver.start(args) is called.

The main reason this package exists is to make the first step work, and I do not intend to break the projects that depend on this behavior. These projects don't call an install function to make chromedriver available, they assume it is available when it is installed.

But I can make another package that has this behavior set as default, and it could use the same code from this project, we just don't install with npm. It would version much less frequently, as this package versions with the Chromedriver version. And we could add an install function.

@christian-bromann
Copy link

It would break projects where people assume the binary is available.

Can you explain which workflow these people use? I doubt that anyone is installing the NPM library and then copy out the downloaded binary file.

The main reason this package exists is to make the first step work, and I do not intend to break the projects that depend on this behavior.

I still can't see how downloading Chromedriver on runtime breaks the behavior. Again, with the assumption that these NPM users interact with the Node module rather than copying the binary out of node_modules the behavior would stay the same. The only difference is that chromedriver.start(args) would check whether a binary was downloaded and does so if not.

These projects don't call an install function to make chromedriver available, they assume it is available when it is installed.

The new behavior also doesn't require anyone to call and install function. It is all embedded in the start method.

@giggio
Copy link
Owner

giggio commented Jul 19, 2023

Can you explain which workflow these people use? I doubt that anyone is installing the NPM library and then copy out the downloaded binary file.

Like this:

const chromedriver = require('chromedriver');
const chromedriverPath = chromedriver.path;
// use chromedriverPath to setup selenium 

@christian-bromann
Copy link

I see. Yes I have seen this in packages. IMO it is not an efficient way to use the package and I could see the same being achieved with:

const chromedriver = require('chromedriver');
const chromedriverPath = await chromedriver.download();

This could be clearly marked breaking change, with suggestions how to update the code to fit a new interface. I would also argue that the ability to download Chromedriver and choosing the version to download and use at runtime provides more value then sticking with this interface.

Anyway, I respect your decision not wanting to move forward with this, so I will find a different solution that fits my requirements.

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