please (optionally) rebuild minified javascript

[yoe]

There's an interpretation of the DFSG, the Debian Free Software Guidelines that define what can and cannot be allowed into Debian's "main" repository, which claims that minified Javascript is object code rather than source code, and that therefore shipping it as-is in a package, without its corresponding source code and without rebuilding from that source code at package build time, makes the package not comply with the DFSG's requirement of having Debian main ship all its own source code.

While I disagree with parts of that interpretation, it's an argument where I seem to be on the losing side. In that context, a while back someone filed a bug against the ola package in Debian, requesting that the package build all javascript at package build time, including things like jquery etc that are really just libraries you're using.

While I could do so myself, it would be easier if the ola build system itself did that; that way, I don't have to ensure I catch everything every time someone updates the javascript or adds some new file or some such. Also, this doesn't have to be enabled by default; e.g., you could well create a --with-closure-compiler= and/or a --enable-javascript-build configure-argument that would toggle the build of the javascript sources.

Thanks,

[daveol]

well there is two things we need to check for then:

• is node (and npm) available? ("new" ui)
• is the closure-compiler available? (or where is it?) ("old" ui)

[daveol]

maybe we can implement a way for the rdm-test server to (optionally) use python-XStatic since the server already uses python.

[peternewman]

The closure based stuff is built as per the instructions here: https://github.com/OpenLightingProject/ola/blob/master/javascript/README

The new UI is according to the instructions here: https://github.com/OpenLightingProject/ola/blob/master/javascript/new-src/README.md

[kecramer]

@peternewman, following those instructions today with the current closure being pulled from git and compiler jar from google results in a failed build process with ADVANCED_OPTIMIZATIONS turned on, and a broken js output when lower levels of optimizations are used.

Do you know what version of closure/closure compiler is being used in the build process?

Note: The README in the javascript directory needs updated as the subversion link no longer works (I'm willing to help with this update as I get my build working)

[peternewman]

Hmm, good question @kecramer . Did it generate any errors/warnings? We had similar issues with https://github.com/openlightingproject/rdm-app when that was attempted to be recompiled recently.

So I downloaded closure_linter-2.3.12 around the same time I suspect (2013-10-29), and had run that over the code. My compiler.jar is from 2013-04-05 12:01, likewise my trunk svn is from 2013-04-05 11:57.

Svn revision info:

Last Changed Rev: 2519
Last Changed Date: 2013-02-08 02:23:52 +0000 (Fri, 08 Feb 2013)

[peternewman]

Also as per the discussion in https://github.com/OpenLightingProject/ola/pull/1053, even @nomis52 and I had differences back in 2016, so I suspect he's running a different version to me.

When it's working, it's probably something else we should add to Travis too, so we know as soon as it breaks with new releases.

Most of this PR is fixing up the rdm-app's JS, which would have been of a similar vintage: https://github.com/OpenLightingProject/rdm-app/pull/128

[kecramer]

Got it, I'll take a look at those links.

What I'm getting with modern closure libs (from the past week) is the following:

closure-library/closure/bin/build/closurebuilder.py --root=closure-library/ --root=ola --namespace="ola.Setup" --output_mode=compiled --compiler_jar=closure-library/compiler.jar --compiler_flags="--compilation_level=ADVANCED_OPTIMIZATIONS" > ../olad/www/ola.js

closure-library/closure/bin/build/closurebuilder.py: 1652 sources scanned.
closure-library/closure/bin/build/closurebuilder.py: Building dependency tree..
closure-library/closure/bin/build/closurebuilder.py: Closure Compiler now natively understands and orders Closure dependencies and
is prefererred over using this script for performing JavaScript compilation.

Please migrate your codebase.

See:
https://github.com/google/closure-compiler/wiki/Managing-Dependencies

closure-library/closure/bin/build/closurebuilder.py: Compiling with the following command: java -client -jar closure-library/compiler.jar --flagfile /var/folders/vx/rcc3pgk148z_js0ygsllx4580000gn/T/tmpfHGp9O
ola/common/server.js:70: WARNING - Provided symbol declared with type Object. This is rarely useful. For more information see https://github.com/google/closure-compiler/wiki/A-word-about-the-type-Object
ola.common.Server.EventType = {
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

ola/common/server.js:536: WARNING - Bad type annotation. type not recognized due to syntax error. See https://github.com/google/closure-compiler/wiki/Bad-Type-Annotation for more information.
 * @param {Array.<{{id: string, mode: string, priority: number}}>}
                   ^

ola/common/server.js:536: WARNING - Bad type annotation. expected closing } See https://github.com/google/closure-compiler/wiki/Bad-Type-Annotation for more information.
 * @param {Array.<{{id: string, mode: string, priority: number}}>}
                    ^

ola/full/dmx_console.js:89: WARNING - Redeclared variable: data_length
  var data_length = this.data.length;
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

ola/full/dmx_console.js:281: WARNING - Redeclared variable: i
  for (var i = 0; i < this.sliders.length; ++i) {
           ^^^^^

ola/full/rdm_patcher.js:283: WARNING - Redeclared variable: device
        var device = devices.pop();
            ^^^^^^^^^^^^^^^^^^^^^^

ola/full/rdm_patcher.js:714: WARNING - Redeclared variable: tr
    var tr = goog.dom.createElement('tr');
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

ola/full/rdm_patcher.js:715: WARNING - Redeclared variable: td
    var td = goog.dom.createElement('td');
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

ola/full/universe_settings_tab.js:187: WARNING - Redeclared variable: count
  var count = this.output_table.getChildCount();
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

ola/full/universe_settings_tab.js:244: WARNING - Redeclared variable: dialog
  var dialog = ola.Dialog.getInstance();
      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

ola/port_table.js:92: WARNING - Redeclared variable: td
      var td = goog.dom.createElement('td');
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

ola/common/server.js:553: ERROR - variable modified_port_ids is undeclared
  modified_port_ids = new Array();
  ^^^^^^^^^^^^^^^^^

ola/common/sorted_list.js:156: ERROR - variable f is undeclared
      delete f;
             ^

ola/common/sorted_list.js:163: ERROR - variables, functions, and arguments cannot be deleted in strict mode
    delete n;
    ^^^^^^^^

ola/full/rdm_attributes_panel.js:232: ERROR - variable obj is undeclared
    var section = obj[i];
                  ^^^

ola/full/rdm_patcher.js:827: ERROR - variable personalities is undeclared
    personalities = response['personalities'];
    ^^^^^^^^^^^^^

5 error(s), 11 warning(s)
Traceback (most recent call last):
  File "closure-library/closure/bin/build/closurebuilder.py", line 293, in <module>
    main()
  File "closure-library/closure/bin/build/closurebuilder.py", line 282, in main
    compiler_flags=options.compiler_flags)
  File "/Users/cramer/rpe/ola/javascript/closure-library/closure/bin/build/jscompiler.py", line 159, in Compile
    raise JsCompilerError('JavaScript compilation failed.')
jscompiler.JsCompilerError: JavaScript compilation failed.

Resolving the errors in the most sane way I can quickly determine results in compiled javascript that fails on initial page load.

[yoe]

If, once you have this working, the relevant stuff is added to configure.ac, that would resolve this issue too as far as I'm concerned ;-)

[peternewman]

And if you don't optimise, you get the same errors? In theory with it unoptimised, you should be able to fix the errors and then get a JS file which shows some locatable bugs in the browser console.

Do you want to push your changes (and then error resolutions in a subsequent commit) up somewhere so we can take a look? One possible short-term option is I could try compiling with my older version, which may at least get us something working for whatever you want.

[kecramer]

Yeah, just doing a WHITESPACE_ONLY compile still results in the errors.

I haven't made any sweeping changes to the javascript yet - only resolved the errors keeping the files from compiling. I'll push those into a branch on my fork.

If you could compile a debug version with your older closure, that would be very helpful as I work through it.

[peternewman]

I think the type errors are because we've used two {{, rather than one. See Record Types here: https://github.com/google/closure-compiler/wiki/Types-in-the-Closure-Type-System#the-javascript-type-language

Instead of delete, there is talk of assigning null to a variable, which will free it without causing the error, it would be worth trying that to see if it works (or at least doesn't throw an error).

What's left in terms of your errors after those fixes and yours?

I'll try and compile a debug one soon.