Opened 9 years ago
Closed 8 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:
- 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.
Attachments (5)
Change History (29)
Changed 9 years ago by
Attachment: | patch.diff added |
---|
comment:1 Changed 9 years ago by
Milestone: | tbd → 1.8 |
---|---|
Priority: | undecided → low |
comment:2 Changed 9 years ago by
Status: | new → assigned |
---|
comment:3 Changed 9 years ago by
comment:4 Changed 9 years ago by
Jan,
Nope, I was planning to update along with these changes if we agree on the enhancement.
comment:5 Changed 9 years ago by
Milestone: | 1.8 → 2.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 9 years ago by
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 9 years ago by
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 9 years ago by
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 9 years ago by
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 8 years ago by
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 8 years ago by
Attachment: | source_maps.diff added |
---|
comment:11 Changed 8 years ago by
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 8 years ago by
Milestone: | 2.0 → 1.9 |
---|---|
Priority: | low → blocker |
Now that there is a 1.9 milestone, this needs to be there.
comment:13 Changed 8 years ago by
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 8 years ago by
So why does it matter that it would not work with the current compiler.jar if it is going to be updated?
comment:15 follow-up: 20 Changed 8 years ago by
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:18 Changed 8 years ago by
Summary: | Auto-generate source maps for optimised layer files → Auto-generate source maps for closure-optimized files |
---|
comment:19 Changed 8 years ago by
@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 follow-up: 21 Changed 8 years ago by
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 follow-up: 22 Changed 8 years ago by
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 Changed 8 years ago by
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 8 years ago by
Resolution: | fixed |
---|---|
Status: | closed → reopened |
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 8 years ago by
Attachment: | 15232-addtag.patch added |
---|
Appends the generated tag into the written file
Changed 6 years ago by
Attachment: | Bob Foster.jpg added |
---|
Information about uptown kitchens you may find by click this link.
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.