You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@shindig.apache.org by jo...@apache.org on 2011/03/29 08:35:11 UTC
svn commit: r1086495 - in /shindig/trunk/java/gadgets/src:
main/java/org/apache/shindig/gadgets/js/
main/java16/org/apache/shindig/gadgets/rewrite/js/
test/java/org/apache/shindig/gadgets/js/
test/java16/org/apache/shindig/gadgets/rewrite/js/
Author: johnh
Date: Tue Mar 29 06:35:10 2011
New Revision: 1086495
URL: http://svn.apache.org/viewvc?rev=1086495&view=rev
Log:
Closure compiler cleanups, suggested by Michael Hermanto.
Also moved AnonFuncWrappingProcessor back to the end of the serving pipeline, as Closure Compiler
doesn't like it when a function def is split between sources.
Modified:
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AnonFuncWrappingProcessor.java
shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsServingPipelineModule.java
shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java
shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AnonFuncWrappingProcessorTest.java
shindig/trunk/java/gadgets/src/test/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompilerTest.java
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AnonFuncWrappingProcessor.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AnonFuncWrappingProcessor.java?rev=1086495&r1=1086494&r2=1086495&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AnonFuncWrappingProcessor.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/AnonFuncWrappingProcessor.java Tue Mar 29 06:35:10 2011
@@ -23,8 +23,8 @@ public class AnonFuncWrappingProcessor i
public boolean process(JsRequest jsRequest, JsResponseBuilder builder)
throws JsException {
if (jsRequest.getJsUri().getCompileMode() != JsCompileMode.BUILD_TIME) {
- builder.prependJs("(function() {\n", "[js-anon-wrapper]");
- builder.appendJs("\n})();", "[js-anon-wrapper]");
+ builder.prependJs("(function(){", "[js-anon-wrapper]");
+ builder.appendJs("})();", "[js-anon-wrapper]");
}
return true;
}
Modified: shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsServingPipelineModule.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsServingPipelineModule.java?rev=1086495&r1=1086494&r2=1086495&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsServingPipelineModule.java (original)
+++ shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/js/JsServingPipelineModule.java Tue Mar 29 06:35:10 2011
@@ -45,12 +45,12 @@ public class JsServingPipelineModule ext
ConfigInjectionProcessor configInjectionProcessor,
AddOnloadFunctionProcessor addOnloadFunctionProcessor,
AddJsLoadCallbackProcessor addJsLoadCallbackProcessor,
- AnonFuncWrappingProcessor anonFuncProcessor,
- CompilationProcessor compilationProcessor) {
+ CompilationProcessor compilationProcessor,
+ AnonFuncWrappingProcessor anonFuncProcessor) {
return ImmutableList.of(injectJsInfoVariableProcessor, jsLoaderGeneratorProcessor,
ifModifiedSinceProcessor, getJsContentProcessor, configInjectionProcessor,
- addOnloadFunctionProcessor, addJsLoadCallbackProcessor, anonFuncProcessor,
- compilationProcessor);
+ addOnloadFunctionProcessor, addJsLoadCallbackProcessor, compilationProcessor,
+ anonFuncProcessor);
}
}
Modified: shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java?rev=1086495&r1=1086494&r2=1086495&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java (original)
+++ shindig/trunk/java/gadgets/src/main/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompiler.java Tue Mar 29 06:35:10 2011
@@ -66,50 +66,47 @@ public class ClosureJsCompiler implement
+ "var parts=name.split('.'),cur=window,part;"
+ "for(;parts.length&&(part=parts.shift());){if(!parts.length){"
+ "cur[part]=obj;}else{cur=cur[part]||(cur[part]={})}}};", "[goog.exportSymbol]");
-
- private static final JSSourceFile[] JSSOURCE_TYPE = new JSSourceFile[0];
-
+
@VisibleForTesting
static final String CACHE_NAME = "CompiledJs";
private final ExportJsCompiler exportCompiler;
- private final CompilerOptions options;
- private final Cache<String, ClosureResult> cache;
- private ClosureResult lastResult;
+ private final Cache<String, JsResponse> cache;
+ private JsResponse lastResult;
@Inject
public ClosureJsCompiler(CacheProvider cacheProvider, FeatureRegistry registry) {
- this(newCompilerOptions(), cacheProvider, registry);
- }
-
- public ClosureJsCompiler(CompilerOptions options, CacheProvider cacheProvider,
- FeatureRegistry registry) {
- this(options, new ExportJsCompiler(registry), cacheProvider);
+ this(new ExportJsCompiler(registry), cacheProvider);
}
@VisibleForTesting
- ClosureJsCompiler(CompilerOptions options, ExportJsCompiler exportCompiler,
- CacheProvider cacheProvider) {
- // TODO: Consider using Provider<Compiler> here.
- this.options = options;
+ ClosureJsCompiler(ExportJsCompiler exportCompiler, CacheProvider cacheProvider) {
this.cache = cacheProvider.createCache(CACHE_NAME);
this.exportCompiler = exportCompiler;
}
- public static CompilerOptions newCompilerOptions() {
- // Same as google3/javascript/closure/builddefs:CLOSURE_COMPILER_FLAGS_FULL.
- // Flags are used/preferred by Gmail.
+ public static CompilerOptions defaultCompilerOptions() {
CompilerOptions result = new CompilerOptions();
CompilationLevel.ADVANCED_OPTIMIZATIONS.setOptionsForCompilationLevel(result);
result.removeUnusedPrototypePropertiesInExterns = false;
// Avoid multiple declarations of tamings___ variables.
// TODO: Get rid of this deviation from standard flags.
result.checkSymbols = false;
- result.sourceMapOutputPath = "create.out";
- result.sourceMapDetailLevel = SourceMap.DetailLevel.ALL;
return result;
}
+
+ protected CompilerOptions getCompilerOptions() {
+ CompilerOptions options = defaultCompilerOptions();
+
+ if (outputCorrelatedJs()) {
+ options.sourceMapOutputPath = "create.out";
+ options.sourceMapFormat = SourceMap.Format.LEGACY;
+ options.sourceMapDetailLevel = SourceMap.DetailLevel.ALL;
+ }
+
+ return options;
+ }
@VisibleForTesting
Compiler newCompiler() {
@@ -138,16 +135,16 @@ public class ClosureJsCompiler implement
String externStr = toExternString(externs);
String cacheKey = makeCacheKey(exportResponse.toJsString(), externStr, jsUri);
- ClosureResult cachedResult = cache.getElement(cacheKey);
+ JsResponse cachedResult = cache.getElement(cacheKey);
if (cachedResult != null) {
lastResult = cachedResult;
- return cachedResult.response;
+ return cachedResult;
}
JsResponseBuilder builder = new JsResponseBuilder();
- String theExterns = null;
// Only run actual compiler if necessary.
+ CompilerOptions options = getCompilerOptions();
if (!jsUri.isDebug() || options.isExternExportsEnabled()) {
List<JSSourceFile> allExterns = Lists.newArrayList();
allExterns.add(JSSourceFile.fromCode("externs", externStr));
@@ -159,8 +156,8 @@ public class ClosureJsCompiler implement
Compiler actualCompiler = newCompiler();
Result result = actualCompiler.compile(
- allExterns.toArray(JSSOURCE_TYPE),
- convertToJsSource(allContent).toArray(JSSOURCE_TYPE),
+ allExterns,
+ convertToJsSource(allContent),
options);
if (actualCompiler.hasErrors()) {
@@ -170,9 +167,9 @@ public class ClosureJsCompiler implement
}
builder.setStatusCode(HttpResponse.SC_NOT_FOUND)
.addErrors(errors.build()).build();
- ClosureResult errorResult = new ClosureResult(builder.build(), null);
+ JsResponse errorResult = builder.build();
cache.addElement(cacheKey, errorResult);
- return errorResult.response;
+ return errorResult;
}
String compiled = actualCompiler.toSource();
@@ -186,15 +183,15 @@ public class ClosureJsCompiler implement
builder.appendJs(compiled, "[compiled]");
}
- theExterns = result.externExport;
+ builder.setExterns(result.externExport);
} else {
// Otherwise, return original content and null exports.
builder.appendAllJs(content);
}
- lastResult = new ClosureResult(builder.build(), theExterns);
+ lastResult = builder.build();
cache.addElement(cacheKey, lastResult);
- return lastResult.response;
+ return lastResult;
}
// Override this method to return "true" for cases where individual chunks of
@@ -245,6 +242,7 @@ public class ClosureJsCompiler implement
};
List<JsContent> builder = Lists.newLinkedList(exportCompiler.getJsContent(jsUri, bundle));
+ CompilerOptions options = getCompilerOptions();
if (options.isExternExportsEnabled()) {
List<String> exports = Lists.newArrayList(bundle.getApis(ApiDirective.Type.JS, true));
Collections.sort(exports);
@@ -261,7 +259,7 @@ public class ClosureJsCompiler implement
return builder;
}
- public ClosureResult getLastResult() {
+ public JsResponse getLastResult() {
return this.lastResult;
}
@@ -271,25 +269,8 @@ public class ClosureJsCompiler implement
HashUtil.checksum(code.getBytes()),
HashUtil.checksum(externs.getBytes()),
uri.getCompileMode(),
- uri.isDebug());
- }
-
- public static class ClosureResult {
- private final JsResponse response;
- private final String externs;
-
- public ClosureResult(JsResponse response, String externs) {
- this.response = response;
- this.externs = externs;
- }
-
- public String getExterns() {
- return externs;
- }
-
- public JsResponse getResponse() {
- return response;
- }
+ uri.isDebug(),
+ outputCorrelatedJs());
}
/**
@@ -344,12 +325,17 @@ public class ClosureJsCompiler implement
curMapping = nextMapping;
}
}
- JsContent lastSource = orig.get(getRootSrc(mappings[curMapping]));
- compiledOut.add(JsContent.fromFeature(compiled.substring(codeStart, codePos + 1),
- lastSource.getSource(), lastSource.getFeature(), null));
+ appendJsContent(compiledOut, codeStart, codePos + 1, compiled, curMapping);
return compiledOut;
}
+ private void appendJsContent(List<JsContent> out, int startPos, int codePos,
+ String compiled, int mapping) {
+ JsContent sourceJs = orig.get(getRootSrc(mappings[mapping]));
+ out.add(JsContent.fromFeature(compiled.substring(startPos, codePos),
+ sourceJs.getSource(), sourceJs.getFeature(), null));
+ }
+
private static final String BEGIN_COMMENT = "/*";
private static final String END_COMMENT = "*/";
private static SourceMappings parseV1(String sourcemap, List<JsContent> orig)
Modified: shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AnonFuncWrappingProcessorTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AnonFuncWrappingProcessorTest.java?rev=1086495&r1=1086494&r2=1086495&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AnonFuncWrappingProcessorTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/js/AnonFuncWrappingProcessorTest.java Tue Mar 29 06:35:10 2011
@@ -56,7 +56,7 @@ public class AnonFuncWrappingProcessorTe
assertTrue(processor.process(request, builder));
control.verify();
if (wraps) {
- assertEquals("(function() {\nJS_CODE\n})();", builder.build().toJsString());
+ assertEquals("(function(){JS_CODE})();", builder.build().toJsString());
} else {
assertEquals("JS_CODE", builder.build().toJsString());
}
Modified: shindig/trunk/java/gadgets/src/test/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompilerTest.java
URL: http://svn.apache.org/viewvc/shindig/trunk/java/gadgets/src/test/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompilerTest.java?rev=1086495&r1=1086494&r2=1086495&view=diff
==============================================================================
--- shindig/trunk/java/gadgets/src/test/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompilerTest.java (original)
+++ shindig/trunk/java/gadgets/src/test/java16/org/apache/shindig/gadgets/rewrite/js/ClosureJsCompilerTest.java Tue Mar 29 06:35:10 2011
@@ -133,11 +133,16 @@ public class ClosureJsCompilerTest exten
private ClosureJsCompiler newClosureJsCompiler(final Compiler realComp,
CompilerOptions realOptions, ExportJsCompiler exportComp, CacheProvider cache) {
- return new ClosureJsCompiler(realOptionsMock, exportComp, cache) {
+ return new ClosureJsCompiler(exportComp, cache) {
@Override
Compiler newCompiler() {
return realComp;
}
+
+ @Override
+ protected CompilerOptions getCompilerOptions() {
+ return realOptionsMock;
+ }
};
}
@@ -232,7 +237,7 @@ public class ClosureJsCompilerTest exten
private static List<JsContent> newJsContents(String jsCode) {
List<JsContent> result = Lists.newArrayList();
- result.add(new JsContent(jsCode, null));
+ result.add(JsContent.fromText(jsCode, null));
return result;
}
}