fix: Fixed "web-ext run" regression on Firefox 60

[chrmod]

closes: #1282

by implementing:
#1282 (comment)

cc: @rpl

[coveralls]

Coverage remained the same at 100.0% when pulling da37f9c on chrmod:firefox-60 into 7e2bd88 on mozilla:master.

[rpl]

@chrmod Thanks a lot for working on a fix for this issue, this is basically the approach that we were already going to follow as a first step (fixing the regression in a minor release by publishing a namespaced version of the package with the fix applied).

Do you mind to also push the changes applied to both these npm packages into the related github forked repos?
(https://github.com/cliqz-oss/node-firefox-connect and https://github.com/cliqz-oss/firefox-client/tree/master)

I'm pretty sure that they are composed by a bump of the package version and the dependency firefox-client on node-firefox-connect, and the fix and bump of the package version on firefox-client but, while we prepare to migrate away from these deprecated dependencies, it would still be better if we could also keep track of the fixes we needed and applied on their forked version.

[rpl]

@kumar303 I just looked into the diff between the old "node-firefox-connect / firefox-client" dependencies and the new "@cliqz-oss/node-firefox-connect / @cliqz-oss/firefox-client" and the following are the actual differences:

• node-firefox-connect => @cliqz-oss/node-firefox-connect

  • package.json: changes related to the package name, the package version and the list of the package mantainers and the @cliqz-oss/firefox-client dependency
  • index.js: changes related to require @cliqz-oss/firefox-client dependency (instead of the previous non-namespaced one)

• firefox-client => @cliqz-oss/firefox-client

  • package.json: changes related to the package name, the package version and the list of the package mantainers
  • lib/client.js: applied the fix suggested in web-ext run broken with Firefox Nightly 60.0a1 (2018-03-07) #1282 (comment)
  • lib/dom.js, lib/domnode.js and lib/stylesheets.js: added for some additional RDP request types (not needed to fix web-ext run on Nightly, but they should not do any harm neither)

The Travis Job is currently failing because of the commit message which doesn't pass the changelog-lint rules, but besides that all the tests are passing as expected.

👍 from me on merging this PR and release a minor bugfix release.

[chrmod]

@rpl I've pushed code to forked repos (forgot to that in the earlier, have just pushed the tags). The commit message is also updated so linter check should pass this time.
Please let me know if there is anything else I can do to help with this one.

[kumar303]

Thanks so much for working on this. I just have a clean-up related change request.

[kumar303]

Can you move this to dependencies and re-enable the lint check by removing /* eslint-disable-line import/no-extraneous-dependencies */ ?

I know it's not your fault 👻 but I just noticed it -- I think that lint rule was skipped by mistake. It only works by chance because of how npm/yarn flattens modules.

[chrmod]

Can do an update, but I don't think it is a mistake. firefox-client is a devDependency as it is only imported for flow to figure out the types. As flow syntax gets removed during build so is the import and the firefox-client is not required by web-ext directly. Does this explanation make sense?

[rpl]

@chrmod yeah, you are right, the historical reasons are definitely that:

• we started to use the flow "import types" feature to have an higher coverage of the flow type checks
• later (in a different pull request) we added the additional eslint rules that checks the imports, eslint started to complain about that import and the 'eslint-disable-line' has been used as a workaround

Anyway, I agree with Kumar, I would probably prefer an additional direct dependency (in any case the dependency is already there, even if it is not currently used directly) and avoid the eslint-disable-line workaround if possible.

Do you mind to also move the new namespaced dependencies on the top of the dependencies?

   "dependencies": {
    "@cliqz-oss/firefox-client": "0.3.1",
    "@cliqz-oss/node-firefox-connect": "1.2.1",
     "adbkit": "2.11.0",
    ...

(so that the next 'npm install --save/--save-dev is not going to have to reorder them in an unrelated change)

Thanks again for your help on quickly fixing this regression.

[chrmod]

@kumar303 @rpl I've have pushed another update.

[kumar303]

Thanks again! I'll publish a release asap