deps(robots-parser): patch robots-parser to work in both node and browser env

[kdzwinel]

I've tried creating a browserify transform that replaces request('url') calls with request('path/to/url-shim.js'). It worked, but uncovered another issue - robots-parser is using url.path that doesn't exists on the web and is deprecated in node. So I decied to go with https://github.com/ds300/patch-package as suggested by @patrickhulce and @wardpeet.

Fixes #4794, but nothing will show up in the details table due to #4818

[kdzwinel]

a diff of a diff 😵

[paulirish]

Presumably this also needs some post install script to apply the patch?

[kdzwinel]

@paulirish I also thought so, but …

Patches created by patch-package are automatically and gracefully applied when you use npm(>=5) or yarn.

🕺

[EDIT] oh whoops, I guess I was too optimistic thinking that it's a build-in npm/yarn functionality - you are right, post install script is needed 😢
[EDIT] post install script added, tested by rm -rf node_modules && yarn and looking into node_modules

[patrickhulce]

LGTM!

[wardpeet]

Yeah patch-package is awesome!

[brendankenny]

Is there a long term path for a real fix?

[kdzwinel]

Yeah, I agree it's not perfect. I thought about opening a PR with this patch against the original repo. We can also pull that library into LH or fork it.

[patrickhulce]

I thought about opening a PR with this patch against the original repo

PR to have robots-parser use real URL class and switch to have us swap require('url') with our shim sounds like most reasonable long term solution to me

[paulirish]

+1 to giving @samclarke a PR.
Hi Sam, big fan. :)

[kdzwinel]

Perfect, I'll open a PR👍 should I create an issue to track this?

[brendankenny]

PR to have robots-parser use real URL class and switch to have us swap require('url') with our shim sounds like most reasonable long term solution to me

SGreatTM

[brendankenny]

should I create an issue to track this?

that SG too

[kdzwinel]

Issue created (#4825) 👌