wisdom-framework / wisdom

A modular and dynamic web framework
http://wisdom-framework.org
Apache License 2.0
88 stars 42 forks source link

Provide a way to generate source map for javascript files #512

Open jbrey opened 9 years ago

jbrey commented 9 years ago

Hi,

It would be nice to provide an option to activate the generation of the source map for minified and/or aggregated files. I saw that the CompilerOptions provides method to enable the generation of these files (http://javadoc.closure-compiler.googlecode.com/git/com/google/javascript/jscomp/CompilerOptions.html#setSourceMapOutputPath(java.lang.String).

Maybe I miss an existing option that would allow me to do that already ?

Cheers JB

barjo commented 9 years ago

Hi @jbrey,

As far as I know the generation of the map is already done by default when using the aggregation and or minification for coffee script. In the coffee script case, the map are created in the default target folder, along with the created js files.

We can add an option that allows for the creation of the source map files when creating the aggregates/minified files

barjo commented 9 years ago

It may be a bit tricky to do it properly.

barjo commented 9 years ago

@jbrey

I just push a new version that create the map file by default. It can be disabled with the configuration property googleClosureMap set to false.

Let me know if it's what you had in mind.

jbrey commented 9 years ago

Thanks. I'll try ASAP!

jbrey commented 9 years ago

I've tried.

The .map are generated with the expected file name, but...:

barjo commented 9 years ago

Thanks for the update,

I will investigate the first point. For the second point, I already append the //# sourceMappingURL= at the end of the created js file. I am not sure if it need a full path thought :-?

jbrey commented 9 years ago

The url should be relative to the minified file to be found by the browser so not a full path.

barjo commented 9 years ago

It's awkward that it doesn't work out for you (it pass in the test). Could you check if you have such comment at the end of the created js file ?

jbrey commented 9 years ago

In fact, I've just figured out that I have it in the aggregated file but not inside the aggregated and minified file. I guess the comment is removed by the minifier.

barjo commented 9 years ago

Thanks for your help, I find out why it only work out for the aggregated file and not the minified one as well as your first issue. Basically, I do it only when there is an Aggregation and not when it's just a File :-1: I missed that. I will fix that tomorrow morning! (GMT +7)

jbrey commented 9 years ago

Thank you for the quick answers and implementation :) .

barjo commented 9 years ago

@jbrey @cescoffier

That's a working version. I am doing som refactoring now and have some open questions. When we create the source map files, should the the sources listed in the source map files be a full path? should it be relative to a particular path?

In this example, it is relative to the target folder. It can be a bit tricky to handle this source url, as it could depends if we are handling internals or externals assets.

{
"version":3,
"file":"street-min.js",
"lineCount":1,
"mappings":"AACAA,OAAAC,EAAA,CADUC,6BACV;",
"sources":["target/test-classes/fake-project/target/classes/assets/street.js"],
"names":["console","log","msg"]
}

Should we leave the option to choose the SourceMap output path from the configuration? In that case the sources path will be relatives to it. Or should we give a mapping, and replaces all source relative path by a full path. Or should we handle differently, depending if it's internal,externals or an agregation.

jbrey commented 9 years ago

I think that all paths inside .map files should be relative to the file they refer to, without information on the build environment.

Moreover, when the package (webjar or zip or anything else) is deployed, the absolute path or path relative to target are not valid anymore.

IMHO, the paths to source files should be resolved from the output path of the .map file.

Example: with the following directory structure (a bit strange but for the purpose of the example):

    a
        b
            b1.js
            b2.js
        c
            c1.js
            c2.js
        d
            e
                e1.js
                e2.js
            aggr.min.js
            aggr.min.js.map

I think the .map files should refer the following sources:

../b/b1.js, ../b/b2.js, ../c/c1.js, ../c/c2.js, ./e/e1.js, ./e/e2.js

Does it make sense ?

PS: I didn't really understood the comment on external assets because in order to have a consistent archive the aggregation (and/or minified) file and map file should refer only to the assets it contains, IMO.

Cheers JB

barjo commented 9 years ago

Yes it makes sens.

I was planning to do that, but the way work the Source Map Output path is a bit tricky when you compile a list of file sharing the same options. There is no problem when we have a JavaScript configuration, since the ouput file is specified, we can set the source map output relative to the same path.

The problem is when handling the external and/or internal assets without an aggregation. We will compile a list of file, create the minified version and the map for each of them. So far so good, but they share the same configuration with the same source map output path (everything will be relative to it). I am not sure how I could handle that with the mapping :-?

I could also refactor to call a new compiler for each file with the proper source map output in that case (I was trying to avoid that). @cescoffier wdyt?

barjo commented 9 years ago

Since they share the same compiler, they all have the same source map file too.

jbrey commented 9 years ago

ok, I understand the problem for maps without aggregation. I'm trying to do some resarch about it.

BTW, I think that having it working with the aggregations is already a good first step :)

barjo commented 9 years ago

Ok, so for now I will leave it for the aggregation only, with the expected behavior. And we will see later for the next step ;)

barjo commented 9 years ago

So, by default the SourceMap output will be the folder that contains the minified/aggregated file.

jbrey commented 9 years ago

Fine for me. I'll try to have a look later too.

Thanks

cescoffier commented 9 years ago

Could we close this one ?

jbrey commented 9 years ago

I'm still having a look as a background task, as it generates the full absolute path of the workstation inside the map.