Added ability in watch mode to copy source maps to output folder for … #250
Conversation
…only changed projects.
| @@ -171,6 +171,12 @@ public class BuildMojo extends AbstractBuildMojo { | |||
| @Parameter(defaultValue = "false") | |||
| protected boolean enableSourcemaps; | |||
|
|
|||
| /** | |||
| * True to copy only changed source files to the output directory. This is a performance optimization. | |||
There was a problem hiding this comment.
| * True to copy only changed source files to the output directory. This is a performance optimization. | |
| * True to copy only changed source files to the output directory. This is a performance optimization, but could result in incorrect output if generated files are altered outside of this plugin. |
There was a problem hiding this comment.
...or if output is cached from an earlier run.
Basically you are sacrificing correctness for speed, especially if your "speed" also comes in the form of "i made it faster by using a cache outside of target".
| @@ -246,6 +246,12 @@ public class TestMojo extends AbstractBuildMojo { | |||
| @Parameter(defaultValue = "false") | |||
| protected boolean enableSourcemaps; | |||
|
|
|||
| /** | |||
| * True to copy only changed source files to the output directory. This is a performance optimization. | |||
There was a problem hiding this comment.
| * True to copy only changed source files to the output directory. This is a performance optimization. | |
| * True to copy only changed source files to the output directory. This is a performance optimization, but could result in incorrect output if generated files are altered outside of this plugin. |
| @@ -129,6 +129,12 @@ public class WatchMojo extends AbstractBuildMojo { | |||
| @Parameter(defaultValue = "true") | |||
| protected boolean enableSourcemaps; | |||
|
|
|||
| /** | |||
| * True to copy only changed source files to the output directory. This is a performance optimization. | |||
There was a problem hiding this comment.
| * True to copy only changed source files to the output directory. This is a performance optimization. | |
| * True to copy only changed source files to the output directory. This is a performance optimization, but could result in incorrect output if generated files are altered outside of this plugin. |
| .SOURCES_DIRECTORY_NAME)).collect(Collectors.toSet())) { | ||
| FileUtils.copyDirectory(dir.toFile(), destSourcesDir); | ||
| // Copy sources to the output directory, if sourcemaps are enabled and incremental source maps is not. | ||
| if(sourcemapsEnabled) { |
There was a problem hiding this comment.
| if(sourcemapsEnabled) { | |
| if (sourcemapsEnabled) { |
| @@ -84,6 +93,26 @@ public Task resolve(Project project, Config config) { | |||
| if (!j2cl.transpile(javaSources, nativeSources)) { | |||
| throw new IllegalStateException("Error while running J2CL"); | |||
| } | |||
|
|
|||
| if(sourcemapsEnabled) { | |||
There was a problem hiding this comment.
| if(sourcemapsEnabled) { | |
| if (sourcemapsEnabled) { |
| @@ -84,6 +93,26 @@ public Task resolve(Project project, Config config) { | |||
| if (!j2cl.transpile(javaSources, nativeSources)) { | |||
| throw new IllegalStateException("Error while running J2CL"); | |||
| } | |||
|
|
|||
| if(sourcemapsEnabled) { | |||
| if(sourceMapsFolder.toFile().exists()) { | |||
There was a problem hiding this comment.
| if(sourceMapsFolder.toFile().exists()) { | |
| if (sourceMapsFolder.toFile().exists()) { |
| }; | ||
| } | ||
|
|
||
| private Path prepareSourcesFolder(Config config, Project project) { |
There was a problem hiding this comment.
Won't doing this here prevent projects that don't get j2cl run on them (bootstrap, jre, any JS-only dependency) from getting sourcemaps? It seems like this might belong in ClosureBundleTask instead (copy from "here's where my input declared its sourcemaps, skip ahead and copy straight to the output dir).
That would also remove the need to have a "is compilationLevel==BUNDLE_JAR", which technically this should have as written - if you have this property on when doing a prod build, you're going to write a bunch of extra stuff to prod that you probably don't want...
There was a problem hiding this comment.
Won't doing this here prevent projects that don't get j2cl run on them (bootstrap, jre, any JS-only dependency) from getting sourcemaps? It seems like this might belong in ClosureBundleTask instead (copy from "here's where my input declared its sourcemaps, skip ahead and copy straight to the output dir).
That would also remove the need to have a "is compilationLevel==BUNDLE_JAR", which technically this should have as written - if you have this property on when doing a prod build, you're going to write a bunch of extra stuff to prod that you probably don't want...
Definitely, what's good is that the projects you mentioned are those that won't change during the j2cl:watch process, which means their source maps need to be copied only once. ClosureBundleTask is definitely the best place for this.
I was thinking about how to copy the source maps of reactor modules in ClosureBundleTask, rather than in J2clTask, but I couldn't figure out how to let ClosureBundleTask know which projects have changed and which have not. Maybe you have some ideas?
treblereel
force-pushed
the
source_maps_refactoring
branch
from
7bab1ee to
bc88cc0
Compare
treblereel
force-pushed
the
source_maps_refactoring
branch
from
bc88cc0 to
36d3c3a
Compare
|
@niloc132 I took another look at my previous solution and realized how to improve it. There is no need to copy the source maps into directories named after Maven modules or dependencies if we can simply overwrite the existing source maps. Yes, this might create a number of source maps related to deleted files, but I don't think that would be a major issue, as it does not affect the app development process. I moved the copying of source maps to the ClosureBundleTask, so now all source maps, including jre.js, are being copied. Therefore, the set of source maps in enableIncrementalSourcemaps now matches the default set of source maps. PS: It seems that in j2cl:watch mode, gwt3BuildCacheDir is ignored. |
…only changed projects.