projectEndings / staticSearch

A codebase to support a pure JSON search engine requiring no backend for any XHTML5 document collection
https://endings.uvic.ca/staticSearch/docs/index.html
Mozilla Public License 2.0
50 stars 22 forks source link

Look into creating sourcemaps for the compiled JS for quick debugging #236

Closed joeytakeda closed 2 years ago

joeytakeda commented 2 years ago

Per #234:

If the search-debug page is there purely to make debugging in the browser easier (rather than testing whether the Closure compiler works), then we might be better off simply providing the sourcemap, if the Closure Compiler in Java is able to create one properly: https://github.com/google/closure-compiler/wiki/Source-Maps. In my experience, the sourcemaps produced by sass, webpack, etc all work perfectly in Chrome and Firefox, but we would have to test to see if Closure does make useful sourcemaps.

martindholmes commented 2 years ago

Sourcemaps aren't a complete solution, because every change has to be followed by a compile/copy before you can test it. Sometimes it's just way more convenient to hack the uncompiled source live, for me anyway.

joeytakeda commented 2 years ago

True, hacking the ssSearch-debug.js is easier, but any changes made in that file has to be made in the actual source anyway. But fair enough, especially since the debug page is just for internal purposes anyway.

I still think a sourcemap is a nice bonus for people who want to see how the JS works, so I think it's worth looking into. I'll rename this, though, to better reflect that this is purely an additional thing, not a replacement.

I just took a quick look at the closure stuff, since I hadn't really done so before. It seems very difficult to find any information at all about the ant task (I ended up just looking at the source code to find the options) and it's slightly frustrating that the command line options are so different from the ant task. The wiki page above says you should be able to pass a boolean flag, but that's not the case with the ant task; you have to set each option explicitly:

 <jscomp compilationLevel="${ssClosureCompilationLevel}" 
      warning="${ssClosureWarning}" 
      debug="false" languageOut="${ssClosureOutputSpec}" 
      output="${ssBaseDir}/js/ssSearch.js"
      sourceMapFormat="V3"
      sourceMapLocationMapping="js/ssSearch-debug.js|ssSearch-debug.js"
      sourceMapOutputFile="${ssBaseDir}/js/ssSearch.js.map"

>

(The @sourceMapLocationMapping is necessary since the debug JS copied into the same directory as the compiled JS)

The above creates a sourcemap that looks more or less correct, but I can't test it as the ssSearch.js isn't getting the sourceMappingURL comment bit and it's not clear to me whether I'm missing an option or if this is a bug in the ant task or something else? Next time I have a chance, I'm going to see if invoking the jar directly yields different results.

martindholmes commented 2 years ago

We can always call the jar directly from Ant. We could look at other compilers, but I'm not keen on adding new dependencies if we can avoid it.

martindholmes commented 2 years ago

Working on this in branch issue-236-closure.

martindholmes commented 2 years ago

Confirming that compiling the source map works; that the closure compiler fails to add the mapping URL comment; but that if I add that comment manually, the source map functions as expected. So I think the next stage is to figure out whether the compiler can be induced to add the comment, but if not, we should simply append it.

martindholmes commented 2 years ago

Having Ant append the source map link seems to work fine; I'm able to use the debug js for debugging in both Chrom* and Firefox. @joeytakeda Can you think of anything else we should do here, or should I just issue a pull request?

joeytakeda commented 2 years ago

I don't think there's anything else to add (though it seems like a design flaw in closure that the sourcemap link isn't added automatically). I just checked out the branch and it works perfectly for me too (even in Safari!), so feel free to merge the PR straight away.

martindholmes commented 2 years ago

PRed, merged, and closing now.