Remove dependency from jQuery for modules random and util

[95ulisse]

Hi,
This pull request removes the "dependency" from jQuery for modules random and util.
Although jQuery is not an actual dependency, the presence of the unbound identifier jQuery in the code may confuse some bundlers like WebPack to think that Forge depends on jQuery, causing some troubles if Forge is used in a context where jQuery can't be loaded, such as a WebWorker.

I reworked only the modules random and util since they are the only ones referencing jQuery included in the default bundle.

Also, I completely removed the function util.makeLink since it appears to be dead code. All the tests pass, so maybe it's safe to remove.

[dlongley]

This LGTM, @davidlehn are you ok with removing util.makeLink?

[dlongley]

I don't think we can assume util.makeLink isn't being used by anyone -- so if we remove it we'll need to do so in version 0.7.x. That still means we need a replacement implementation for 0.6.x that doesn't rely on jQuery (or a way to avoid having build tools think it's a hard requirement).

[davidlehn]

I did some digging and found the historical initial makeLink code from 2009 that migrated over into forge. We were using it in a now defunct project but I don't think we've used the version in forge.

I do agree that we probably shouldn't just remove it now since it's slightly possible someone else is using it. I've got no issues marking that as a thing to get rid of or move into an extra lib in the next major breakage version. There's probably much more we can also make optional too since these days things like that are of little use to the primary crypto code that people use forge for.

Does using jQuery actually cause trouble with webpack et al and webworkers, or is this just a guess? I thought some of those had special support to handle such globals. It'd be nice if we could hold off until the next major release but maybe we can find a workaround if it's a problem now.

Maybe it's possible to use a variation of that wizardry global code to try and get jQuery vs using it direct. Would that fix bundler issues? Something like:

var _jQuery;
(function(global) { _jQuery = global.jQuery; })((0, eval)('this'));
util.makeLink = function(...) {if(_jQuery) { ... _jQuery.isArray(...) ... }};

I'm unsure of how correct or portable that is, but it might avoid the bundler issues without special configuration?

[davidlehn]

Style issue, remove space after if.

[95ulisse]

Maybe the workaround can be even simpler. If we declare a jQuery variable at the top of the files, the name is bound, and WebPack stops adding the dependency.

var jQuery = (function(g) { return g.jQuery; })((0, eval)('this'));

By the way, using jQuery in Forge can actually cause trouble if the crypto is done in a worker, because jQuery cannot be loaded in the context of a worker. It's a common configuration for WebPack to automatically add a dependency on jQuery when there are the unbound identifiers $ or jQuery, since a lot of libraries have "hidden" dependencies on jQuery.

In the case of Forge, jQuery is just an optional, so there's no reason for it to have such a dependency.

[dlongley]

@davidlehn, did #456 address this in any way?

[davidlehn]

What test can we run to see if it fails? The webpack and browserify testing seems to be working with the new code so maybe this problem resolved itself? That being said, the makeLink function and others in util.js should perhaps be considered for deprecation, removal, or being moved to some extras file.

[jvitela]

Maybe the workaround can be even simpler. If we declare a jQuery variable at the top of the files, the name is bound, and WebPack stops adding the dependency.

var jQuery = (function(g) { return g.jQuery; })((0, eval)('this'));

I ended up here because my website uses a Content Security Policy And I got an error report that one of the libraries in my bundle was trying to make an unauthorized eval.

I also got highly suspicious when I saw the global jQuery references in the code. I think you should at least clarify in the README that jQuery is an optional dependency an that eval is being used.

[davidlehn]

Sorry, this got auto-closed due to branch rename. Reopen if you want. Will address in #937.