Pin the selenium-webdriver version so that it correctly matches protractor.

[EricHeath-AI]

My team experienced problems running protractor with symptoms as described in angular/protractor#2790. We were able to trace it down to a situation where a bad combination of dependencies (angular, protractor, and benchpress) introduced multiple copies of selenium webdriver into the node_modules tree. This then caused jasminewd2 to resolve the wrong version of selenium webdriver, fail to actually execute anything, and silently return success for all our protractor test cases.

The problem is described in more detail in angular/protractor#2790.

This is the simplest and safest fix I could come up with. Maybe there is another way to achieve the same thing?

[sjelin]

Isn't this a case for peer dependencies?

[EricHeath-AI]

@sjelin Sounds like peer deps might be a good fit. I'm not sure I get the implication though. In my example, I had a project with a dependency graph like:

• benchpress 2.0.0-beta.6 -> selenium-webdriver 2.52.0
• protractor 3.1.1 -> selenium-webdriver 2.48.2.
• protractor 3.1.1 -> jasminewd2 0.0.8
• jasminewd2 0.0.8 -> ... wants selenium-webdriver, but does not specify a version ... it finds whatever it finds.

which resulted in a directory structure like:
myapp/node_modules/protractor/node_modules/selenium_webdriver -> 2.48.2 (for protractor)
myapp/node_modules/selenium_webdriver -> 2.52.0 (for benchpress)
myapp/node_modules/jasminewd2 ...

If I add peerDeps to jasminewd2 0.0.8 -> selenium-webdriver 2.48.2, do I get an error because the module search path reaches selenium-webdriver 2.52.0 ahead of 2.48.2?

An error at npm dependency resolution time would be better than the silent failures in the tests at runtime, but I'm not really sure how I would configure the project so that I can use both benchpress and protractor (which seems like a reasonable thing if I'm building an angular app).

Or am I missing something and peerDeps will actually resolve the correct version in this case?

[sjelin]

Honestly I'm not that sure about the details of peerDependencies. But if jasminewd2 depended on selenium-webdriver, that could cause problems if:

1. Someone is using an old version of node/npm where the dependency tree was more sprawling
2. protractor and jasminewd2 were ever out of sync

Also, are you sure this actually solves the problem you're thinking of? Wouldn't you get:

myapp/node_modules/protractor/node_modules/selenium_webdriver -> 2.48.2 (for protractor)
myapp/node_modules/selenium_webdriver -> 2.52.0 (for benchpress)
myapp/node_modules/jasminewd2/node_modules/selenium_webdriver -> 2.48.2 (for jasminewd2)

And surely benchpress and Protractor have to use the same instance (or at least version) of webdriver? Doesn't benchpress use Protractor and benchpress? @juliemr I could use your help on this one as my benchpress knowledge is extremely limited.

But regardless, you do seem right that peerDependencies are not a solution

[juliemr]

We should avoid peer-deps, they've historically caused problems and are being deprecated.

benchpress does NOT depend on Protractor, it directly depends on selenium-webdriver.

So, with the new flat node_modules directory introduced in the latest npm (default if you are using node v5), you can get into a situation where if you do, e.g. npm install benchpress then npm install protractor your tree will look like:

myapp/node_modules/selenium-webdriver @2.53
myapp/node_modules/jasminewd
myapp/node_modules/protractor/node_modules/selenium-webdriver @2.52

I think @sjelin is right that adding the dep would just cause other problems (two instances of selenium-webdriver at the same version) as listed above.

What we want is to explicitly use the same instance of selenium-webdriver that Protractor is using. I think the only way to ensure this is to have protractor pass it in. Let me make a new PR to do that.

[juliemr]

See, in particular, this comment about why peer dependencies are not a match for what we're trying to do:

npm/npm#6565 (comment)

Also

npm/npm#5080 (comment)

[EricHeath-AI]

@sjelin The node_modules tree you describe is certainly possible. I agree it is messy looking, but I believe it will also result in correct execution of protractor because both protractor and jasminewd2 (which are peers) will resolve 2.48.2 of webdriver from their respective child node_modules directories (before they start searching up higher in the tree). As Julie points out, the real requirement is for jasminewd2 to use the same version of selenium-webdriver as protractor for everything to function correctly.

@juliemr Having protractor pass the selenium-webdriver dep into jasminewd sounds like a good approach. Happy to close this PR in favour of that approach. Feel free to close this one when you're ready.

[heathkit]

This is fixed by #52