Opened 8 years ago

Closed 7 years ago

#15232 closed enhancement (fixed)

Auto-generate source maps for closure-optimized files

Reported by: James Thomas Owned by: Rawld Gill
Priority: blocker Milestone: 1.9
Component: BuildSystem Version: 1.7.2
Keywords: 1.9 Cc:
Blocked By: Blocking:

Description

Source maps allow debugging tools to map between minified source files and the original source files. It allows user to step through the code as if it's un-minified, allowing them to debug errors inside the minified build layers we generate. This is huge advantage to developers who need to debug errors that occur in optimised applications without having to switch the app back to an unoptimised version.

There's a full explanation in this article, http://www.html5rocks.com/en/tutorials/developertools/sourcemaps/

Source maps can be be automatically generated by the latest version of the Closure compiler. I've managed to update the layerOptimise transforms in the build system to turn on source map generation and ensure that the necessary mapping files between the optimised and unoptimised versions are created. The attached patch has the modifications needed to turn this feature on.

Rawld, Can you take a look and see if you spot anything I've done wrong?

Issues:

  1. We'd need to update the version of compiler.jar to the latest version to support V3 source maps (the version supported in the debugging tools).
  1. We need to pass the local file name, rather than the full file path, at stages in the generation. I've used path.split("/").pop() to retrieve the local file name from a file path, does the build tool on windows use forward slashes? I wasn't sure if this information was available somewhere else for access?
  1. This only works when using the Node platform to run the build. Source maps rely on having the exact unoptimised file available to translate to. When I added these changes to writeOptimized, the source mapping was incorrect due to differences in the text between the text passed to the compiler and the unoptimised layer file contents (e.g. consoleStripped is enabled, the source is modified in memory).

If people agree this is a useful addition, I'll commit the changes.

Attachments (5)

patch.diff (1.7 KB) - added by James Thomas 8 years ago.
source_maps.diff (2.2 KB) - added by James Thomas 7 years ago.
15232-addtag.patch (685 bytes) - added by Brian Arnold 7 years ago.
Appends the generated tag into the written file
blue1.jpg (65.6 KB) - added by Slavon8 5 years ago.
crooksandliars.com
Bob Foster.jpg (32.0 KB) - added by Slavon8 5 years ago.
Information about uptown kitchens you may find by click this link.

Download all attachments as: .zip

Change History (29)

Changed 8 years ago by James Thomas

Attachment: patch.diff added

comment:1 Changed 8 years ago by James Thomas

Milestone: tbd1.8
Priority: undecidedlow

comment:2 Changed 8 years ago by Rawld Gill

Status: newassigned

comment:3 Changed 8 years ago by eventsiaarhus

Has the compiler.jar been updated to the latest version ? Jan.


Source maps allow debugging tools to map between minified source files and the original source files. It allows user to step through the code as if it's un-minified, allowing them to debug errors inside the minified build layers we generate. This is huge advantage to developers who need to debug errors that occur in optimised applications without having to switch the app back to an unoptimised version.

There's a full explanation in this article, http://www.html5rocks.com/en/tutorials/developertools/sourcemaps/

Source maps can be be automatically generated by the latest version of the Closure compiler. I've managed to update the layerOptimise transforms in the build system to turn on source map generation and ensure that the necessary mapping files between the optimised and unoptimised versions are created. The attached patch has the modifications needed to turn this feature on.

Rawld, Can you take a look and see if you spot anything I've done wrong?

Issues:

We'd need to update the version of compiler.jar to the latest version to support V3 source maps (the version supported in the debugging tools). We need to pass the local file name, rather than the full file path, at stages in the generation. I've used path.split("/").pop() to retrieve the local file name from a file path, does the build tool on windows use forward slashes? I wasn't sure if this information was available somewhere else for access? This only works when using the Node platform to run the build. Source maps rely on having the exact unoptimised file available to translate to. When I added these changes to writeOptimized, the source mapping was incorrect due to differences in the text between the text passed to the compiler and the unoptimised layer file contents (e.g. consoleStripped is enabled, the source is modified in memory). If people agree this is a useful addition, I'll commit the changes.

comment:4 Changed 8 years ago by James Thomas

Jan,

Nope, I was planning to update along with these changes if we agree on the enhancement.

comment:5 Changed 7 years ago by Colin Snover

Milestone: 1.82.0

1.8 is frozen. Move all enhancements to next release. If you need an exemption from the freeze for this ticket, contact me immediately.

comment:6 Changed 7 years ago by Adam Peller

Would be nice to get this in the Dojo 1.x line. Users shouldn't need to wait for / move to 2.0 for this. If not 1.8.0, perhaps 1.8.1?

comment:7 Changed 7 years ago by dylan

Keywords: 1.8.1 added

Adding a 1.8.1 keyword, as we may want to consider this improvement prior to 2.0

comment:8 Changed 7 years ago by Colin Snover

Keywords: 1.9 added; 1.8.1 removed

The keyword for pre-2.0 is 1.9. Not sure why you guys are discussing adding enhancements to bugfix releases, this should not happen.

comment:9 Changed 7 years ago by James Thomas

This is really just waiting on an answer to my second issue. It should be a simple addition once the windows path issue has been resolved. Assuming it's too late for 1.8, 1.9 seems like a good place.

comment:10 Changed 7 years ago by Colin Snover

jamesthomas, if you are still interested in having this feature land could you provide a revised patch now that the optimizers have been partially split to files? Thanks.

Changed 7 years ago by James Thomas

Attachment: source_maps.diff added

comment:11 Changed 7 years ago by James Thomas

I've re-factored the original patch now that the optimizers have been split out into separate files. We need to upgrade the closure compiler jar in "utils" to a recent version to support this functionality when we merge this patch.

comment:12 Changed 7 years ago by Colin Snover

Milestone: 2.01.9
Priority: lowblocker

Now that there is a 1.9 milestone, this needs to be there.

comment:13 Changed 7 years ago by cjolif

would it mean that the build tool wouldn't work with the current compiler.jar after that? Or would it just mean that if you run with the old version you won't get the source map support?

(also as this was not part of the 1.9 plans I'm not sure this can be considered a blocker, that said I agree we should do our best to get that in).

comment:14 Changed 7 years ago by Colin Snover

So why does it matter that it would not work with the current compiler.jar if it is going to be updated?

comment:15 Changed 7 years ago by cjolif

Because even if committed in our repo that is not Dojo CLA'd code. So knowing if that is a mandatory or an optional upgrade will help those who care about that to plan accordingly. In general by the way if I can be cced on those kind of changes that impacts non Dojo CLA'd code this allows me to plan ahead.

comment:16 Changed 7 years ago by James Thomas

It won't break, you just won't get source map support.

comment:17 Changed 7 years ago by Rawld Gill

Resolution: fixed
Status: assignedclosed

In [30914]:

enhance builder to auto generate source maps for closure-optimized resources; imported latest closure compiler; fixes #15232; !strict; thanks jamesthomas

comment:18 Changed 7 years ago by Rawld Gill

Summary: Auto-generate source maps for optimised layer filesAuto-generate source maps for closure-optimized files

comment:19 Changed 7 years ago by Rawld Gill

@jamesthomas: Thanks for the patch. I expanded the patch to work on node. Also note that the patch always optimized all closure-compiled resources, not just layers.

I tested on both platforms (node and rhino) and the output looks rational; however, I don't debug with source maps so a second set of eyes on the output would be reassuring.

comment:20 in reply to:  15 ; Changed 7 years ago by Rawld Gill

Replying to cjolif:

Because even if committed in our repo that is not Dojo CLA'd code.

Just giving you an official "head's up" that I did update the closure compiler to the latest. Let me know if this is a problem.

comment:21 in reply to:  20 ; Changed 7 years ago by cjolif

Replying to rcgill:

Replying to cjolif:

Because even if committed in our repo that is not Dojo CLA'd code.

Just giving you an official "head's up" that I did update the closure compiler to the latest. Let me know if this is a problem.

For the record it is the Feb 28 2013 build, right? (marked 0227).

comment:22 in reply to:  21 Changed 7 years ago by Rawld Gill

For the record it is the Feb 28 2013 build, right? (marked 0227).

The compiler reports 2013/02/28 at 16:05. I could not get a version to print from it. It was the featured release on the downloads page on 18 March 2013. Iiuc, this should be the latest stable version.

comment:23 Changed 7 years ago by Brian Arnold

Resolution: fixed
Status: closedreopened

Source maps are almost working, but not quite. The map files were being built, but the final optimized output didn't have the map_tag tacked on at the end. I'm attaching a very simple patch that enables them.

I've tested this patch by doing a simple build with the AMD profile that we ship with, and when I enabled source maps, I was getting pointed to various uncompressed.js files rather than the built file, which is pretty awesome.

Changed 7 years ago by Brian Arnold

Attachment: 15232-addtag.patch added

Appends the generated tag into the written file

comment:24 Changed 7 years ago by Brian Arnold

Resolution: fixed
Status: reopenedclosed

In [31051]:

Appending the map_tag into the built file, fixes #15232

Changed 5 years ago by Slavon8

Attachment: blue1.jpg added

Changed 5 years ago by Slavon8

Attachment: Bob Foster.jpg added

Information about uptown kitchens you may find by click this link.

Note: See TracTickets for help on using tickets.