Issue 5060 - Move require into modules template, make info a module

templates/modules.js.tmpl

 {%- if args["module"] -%}

[Sebastian Noack, 2017/03/31 16:33:11]

It seems we can get rid of the "module" argument now. It was only necessary to
bundle content scripts by concatenating them instead of generating modules.

[kzar, 2017/04/01 01:32:56]

Not quite yet, there are a few scripts that we need to keep bundling for now.
For example ext/common.js, ext/background.js and elemHideEmulation.js. I think
you're right that we could perhaps remove that in the future though.

[Sebastian Noack, 2017/04/01 10:02:56]

It seems elemHideEmulation.js can just be added unchanged as module to the
content scripts bundle. We probably should do that anyway.

As for ext, since Safari is gone, the separation between ext/common.js and
chrome/ext/common.js, and ext/background.js and ext/chrome/background.js doesn't
make any sense anymore. These scripts should have been merged when we removed
safari/ext. But since this didn't happen, we should do it now. It's quite
trivial anyway, and once we did that there is no need to bundle any scripts
without the module system anymore.

[kzar, 2017/04/01 10:50:28]

We agree that at some point we can probably remove the module flag, but I don't
agree about doing it right now. I've had enough of these refactoring jobs for a
while, and hope to focus on the WebRTC work soon. Perhaps this could be a
"goodfirstissue" for Jon or one of your mentees?

[Sebastian Noack, 2017/04/01 11:04:54]

Merging those scripts, while on it, is trivial. If we do it later, however, we
need to update buildtools, update the buildtools dependency in
adblockpluschrome, and adapt adblockpluschrome, again, which is some effort.

Also in regard to that potential no cosmetic changes policy, you voted for, the
idea was to clean up code while changing it anyway, like we'd do here. Not
saying that we will go with that policy, but it seems inconsistent that you
agree to it, but then suggest doing the related clean up separately here.

[kzar, 2017/04/01 12:13:13]

Honestly I've have had enough of these "while at it" refactoring requests for a
while, especially when each one leads to three more. The ESLint stuff was
painful and it left me fairly burnt out there. I plan to work on the WebRTC
wrapper next week. I'm happy to try and finish this, or to let someone else take
over, but I don't want the scope of the changes to continue to grow if I'm the
one stuck making them.

[Sebastian Noack, 2017/04/01 13:09:06]

Well, you were the one pushing for the ESLint configuration and the related
refactoring. Refactoring the content scripts into modules was necessary, in
order to use the no-undef rule there (which wasn't even my idea) in a sane way,
besides we planned to put everything into modules for a long time anyway. Also
you were the one pushing for dropping Safari support, and despite we are talking
about a change that was just forgotten there, you turn down every opportunity to
correct that. While it was good we did these things, and I'm happy you moved
them forward, I think your reaction here isn't fair.

Generally, I think, it is important while we advance our code, to watch out for
things that can be simplified or cleaned up. Otherwise, we end up in an
unmaintainable mess in the long run (if we aren't there yet), whether the code
is linted or not. Anyway, I guess, I will file a separate issue.

+if (typeof require == "undefined")
+{
+  var require = function(module)
+  {
+    if (!(module in require.scopes))
+    {
+      let scope = {exports: {}};
+      require.scopes[module] = require.modules[module](scope, scope.exports);
+    }
+    return require.scopes[module];
+  };
+  require.modules = Object.create(null);
+  require.scopes = Object.create(null);
+}
+
+{% if args["injectinfomodule"] %}
+require.modules["info"] = function(module, exports)
+{
+{% if type == "gecko-webext" %}
+{% include "geckoInfo.js.tmpl" %}
+{% elif type == "chrome" or type == "edge" %}
+{% include "chromeInfo.js.tmpl" %}
+{% endif %}
+return module.exports;
+};
+{% endif %}
+
 {%- for module_name, script in modules -%}
 require.modules[{{ module_name|json }}] = function(module, exports)
 {
 {{ script }}
 return module.exports;
 };
 
 {% endfor -%}
 {%- set modules = dict(modules) -%}
 {%- for module_name in args["autoload"] -%}
 {%- if module_name in modules -%}
 require({{ module_name|json }});
 {% endif %}
 {%- endfor -%}
 {%- else -%}
 {%- for module_name, script in modules -%}
 {{ script }}
 {% endfor -%}
 {%- endif -%}