Opened 13 years ago

Closed 11 years ago

#69 closed defect (fixed)

script errors

Reported by: blq Owned by: madair
Priority: major Milestone: 1.1.0
Component: core Version: trunk
Keywords: Cc:

Description

Hi, I tried to compile Proj4js (current trunk) using the Google Closure JavaScript compiler, http://code.google.com/closure/compiler/. Using the VERBOSE mode it found a bunch of errors, mostly related to use of undefined variables.

Applying the attached patch the most obvious errors are fixed and you get a "successful" compilation. But, I'd say it's better if you do the compilation yourselves and inspect the output step by step to ensure the correctness. There's still a couple of most-likely bugs left although they appear only as warnings. For example use of a "this.phi0", "this.at1" and "c" that doesn't seem to be declared, "this.gam" vs "this.gama", qsfnz() is called with three args but only take two etc.

Here's the command line I used: "java -jar closure.jar --js Proj4js-combined.js --warning_level VERBOSE --js_output_file compiled_proj4s-combined.js"

If you manage to navigate around the massive amounts of "dangerous use of the global this object" warnings you find the more serious bugs!

As a nice side-effect the output from Closure is a 15kb smaller file.

Regards / Fredrik Blomqvist

Attachments (2)

proj4js.patch (1.8 KB ) - added by blq 13 years ago.
patch for proj4js-combined.js that fixes errors found using Closure compiler.
proj4js_closure.patch (17.1 KB ) - added by blq 13 years ago.
adds Closure VERBOSE mode compilation.

Download all attachments as: .zip

Change History (16)

by blq, 13 years ago

Attachment: proj4js.patch added

patch for proj4js-combined.js that fixes errors found using Closure compiler.

comment:1 by blq, 13 years ago

Noticed that you can add "--jscomp_off globalThis" to the command line to silence the huge number of that, in this case, non-critical warning.

comment:2 by blq, 13 years ago

..bump. Any updates?

comment:3 by madair, 13 years ago

Status: newassigned

I will include this with a new release around a license change in the near future

comment:4 by blq, 13 years ago

Great, thanks.

comment:5 by madair, 13 years ago

Resolution: fixed
Status: assignedclosed

modified patch applied at rev 2203

comment:6 by madair, 13 years ago

Milestone: 1.0.4

Milestone 1.0.4 deleted

comment:7 by madair, 13 years ago

Milestone: 1.1.0

comment:8 by blq, 13 years ago

I can see you updated the package now, thanks! But please see if you can investigate the remaining handful of warnings when set to compile using VERBOSE mode (VERBOSE finds more errors, it's not just how much is printed to the log).

For example, in the inverse polyconic projection there's a 'c' variable that is used in the calculations despite never being set. (add --jscomp_off globalThis to filter out the unimportant warning).

by blq, 13 years ago

Attachment: proj4js_closure.patch added

adds Closure VERBOSE mode compilation.

comment:9 by blq, 13 years ago

Attached a new patch: proj4js_closure.patch (relative the new 1.1.0 trunk). It adds verbose compilation (and couple of trivial edits to remove a handful of non-critical warnings to reduce log noise). The warnings that are output now I'd say needs some further investigation.

Regards / Fredrik Blomqvist

comment:10 by blq, 12 years ago

Resolution: fixed
Status: closedreopened

comment:11 by madair, 12 years ago

seen the light on benefits of using the google closure compiler.

will be working using use plovr for building this library similar to what is being used for ol3

comment:12 by blq, 12 years ago

Nice. Please note that you should strive to use the "--warning_level VERBOSE" setting to catch "all" warnings (see my patch above).

comment:13 by madair, 12 years ago

I have created a branch for this work here http://svn.osgeo.org/metacrs/proj4js/branches/closure Note I've only brought over the lcc.js projection class at this point. Once the correct pattern is defined, we can apply to all other projection classes.

It mostly works, but not in ADVANCED compilation mode because some symbols are not properly exported. Help with getting the annotations correctly defined is needed

comment:14 by madair, 11 years ago

Resolution: fixed
Status: reopenedclosed

this work is now being carried out on github, https://github.com/proj4js/proj4js

new issues can be opened on github

Note: See TracTickets for help on using tickets.