Issue 4552 - Drop jshydra dependency (adblockpluschrome)

Issue 4552 - Drop jshydra dependency (adblockpluschrome)

[kzar, 2016-11-18 17:22:41]

Patch Set 1

https://codereview.adblockplus.org/29363560/diff/29363561/dependencies
File dependencies (right):

https://codereview.adblockplus.org/29363560/diff/29363561/dependencies#newcode3
dependencies:3: buildtools = buildtools git:4552-drop-jshydra
Of course this change depends on the buildtools changes and I'll update the
proper revision here once they are pushed.

https://codereview.adblockplus.org/29363560/diff/29363561/lib/compat.js
File lib/compat.js (right):

https://codereview.adblockplus.org/29363560/diff/29363561/lib/compat.js#newco...
lib/compat.js:82: import: function()
For some reason I needed to add this, I have no idea why!

https://codereview.adblockplus.org/29363560/diff/29363561/lib/io.js
File lib/io.js (right):

https://codereview.adblockplus.org/29363560/diff/29363561/lib/io.js#newcode45
lib/io.js:45: content: Array.from(data),
The data was a generator and was not being saved correctly without this change.

[kzar, 2016-11-21 11:15:01]

Patch Set 2 : Remove used convert_js args

[Wladimir Palant, 2016-11-30 11:47:27]

https://codereview.adblockplus.org/29363560/diff/29363622/lib/compat.js
File lib/compat.js (right):

https://codereview.adblockplus.org/29363560/diff/29363622/lib/compat.js#newco...
lib/compat.js:28: require);
As indicated in another review, passing in require currently makes no sense
because all modules should call the same global require function.

https://codereview.adblockplus.org/29363560/diff/29363622/metadata.common
File metadata.common (right):

https://codereview.adblockplus.org/29363560/diff/29363622/metadata.common#new...
metadata.common:94: ext/background.js = ext/background.js
No point running ext/common.js and ext/background.js through the conversion,
they should come out unchanged.

[kzar, 2016-11-30 14:36:33]

Patch Set 3 : Remove require parameter

https://codereview.adblockplus.org/29363560/diff/29363622/lib/compat.js
File lib/compat.js (right):

https://codereview.adblockplus.org/29363560/diff/29363622/lib/compat.js#newco...
lib/compat.js:28: require);
On 2016/11/30 11:47:27, Wladimir Palant wrote:
> As indicated in another review, passing in require currently makes no sense
> because all modules should call the same global require function.

Done.

https://codereview.adblockplus.org/29363560/diff/29363622/metadata.common
File metadata.common (right):

https://codereview.adblockplus.org/29363560/diff/29363622/metadata.common#new...
metadata.common:94: ext/background.js = ext/background.js
On 2016/11/30 11:47:27, Wladimir Palant wrote:
> No point running ext/common.js and ext/background.js through the 
> conversion, they should come out unchanged.

I thought this initially as well, but then I realised that metadata.chrome
appends chrome/ext/common.js and chrome/ext/background.js to etx/common.js and
ext/background.js respectively. I think this is helpful to ensure the scripts
are ordered correctly.

[Wladimir Palant, 2016-11-30 14:57:54]

LGTM once the dependency can be set properly. Note that the new require()
implementation will throw for missing modules, before it would return undefined.
I consider that a nice side-effect but it's something to keep in mind because it
could break existing code.