You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tapestry.apache.org by hl...@apache.org on 2013/11/26 02:30:01 UTC

[2/2] git commit: Refactor and optimize RequireJS configuration

Refactor and optimize RequireJS configuration

Switched to an alternate pattern that works correctly with both jQuery as the infrastructure compiler, and when
the JavaScript on the page has been aggregated. Without thing change, RequireJS can sometimes attempt to load
a module before it has been configured.


Project: http://git-wip-us.apache.org/repos/asf/tapestry-5/repo
Commit: http://git-wip-us.apache.org/repos/asf/tapestry-5/commit/2dc46950
Tree: http://git-wip-us.apache.org/repos/asf/tapestry-5/tree/2dc46950
Diff: http://git-wip-us.apache.org/repos/asf/tapestry-5/diff/2dc46950

Branch: refs/heads/master
Commit: 2dc469507438308866744b04787a2d1ad427a0b9
Parents: b04bca4
Author: Howard M. Lewis Ship <hl...@apache.org>
Authored: Mon Nov 25 17:29:51 2013 -0800
Committer: Howard M. Lewis Ship <hl...@apache.org>
Committed: Mon Nov 25 17:29:51 2013 -0800

----------------------------------------------------------------------
 .../internal/services/DocumentLinkerImpl.java   | 10 +++---
 .../services/javascript/ModuleManagerImpl.java  | 21 ++++++-----
 .../javascript/ModuleConfigurationCallback.java | 22 +++++++-----
 .../services/javascript/ModuleManager.java      | 38 +++++++++++---------
 .../services/DocumentLinkerImplTest.groovy      | 36 +++++++++++--------
 5 files changed, 74 insertions(+), 53 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/2dc46950/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinkerImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinkerImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinkerImpl.java
index a7cfb78..6cff969 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinkerImpl.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/DocumentLinkerImpl.java
@@ -210,6 +210,10 @@ public class DocumentLinkerImpl implements DocumentLinker
      */
     protected void addScriptsToEndOfBody(Element body)
     {
+        moduleManager.writeConfiguration(body, moduleConfigurationCallbacks);
+
+        // Write the core libraries, which includes RequireJS:
+
         for (String url : coreLibraryURLs)
         {
             body.element("script",
@@ -217,11 +221,9 @@ public class DocumentLinkerImpl implements DocumentLinker
                     "src", url);
         }
 
-        // In prior releases of Tapestry, we've vacillated about where the <script> tags go
-        // (in <head> or at bottom of <body>). Switching to a module approach gives us a new chance to fix this.
-        // Eventually, (nearly) everything will be loaded as modules.
+        // Write the initialization at this point.
 
-        moduleManager.writeInitialization(body, libraryURLs, initsManager.getSortedInits(), moduleConfigurationCallbacks);
+        moduleManager.writeInitialization(body, libraryURLs, initsManager.getSortedInits());
     }
 
     private static Element createTemporaryContainer(Element headElement, String existingElementName, String otherExistingElement, String newElementName)

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/2dc46950/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleManagerImpl.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleManagerImpl.java b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleManagerImpl.java
index fb66c6d..8e0f3df 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleManagerImpl.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/internal/services/javascript/ModuleManagerImpl.java
@@ -86,8 +86,8 @@ public class ModuleManagerImpl implements ModuleManager
     private String buildRequireJSConfig(List<ModuleConfigurationCallback> callbacks)
     {
         // This is the part that can vary from one request to another, based on the capabilities of the client.
-        JSONObject config =  baseConfig.copy().put("baseUrl", getBaseURL());
-        
+        JSONObject config = baseConfig.copy().put("baseUrl", getBaseURL());
+
         // TAP5-2196: allow changes to the configuration in a per-request basis.
         for (ModuleConfigurationCallback callback : callbacks)
         {
@@ -95,7 +95,8 @@ public class ModuleManagerImpl implements ModuleManager
             assert config != null;
         }
 
-        return String.format("requirejs.config(%s);\n", config.toString(compactJSON));
+        // This part gets written out before any libraries are loaded (including RequireJS).
+        return String.format("var require = %s;\n", config.toString(compactJSON));
     }
 
     private JSONObject buildBaseConfig(Map<String, JavaScriptModuleConfiguration> configuration, boolean devMode)
@@ -174,8 +175,8 @@ public class ModuleManagerImpl implements ModuleManager
         tracker.clearOnInvalidation(cache);
     }
 
-    public void writeInitialization(Element body, List<String> libraryURLs, List<?> inits,
-            List<ModuleConfigurationCallback> callbacks)
+    public void writeConfiguration(Element body,
+                                   List<ModuleConfigurationCallback> callbacks)
     {
         Element element = body.element("script", "type", "text/javascript");
 
@@ -183,14 +184,16 @@ public class ModuleManagerImpl implements ModuleManager
         // (in development mode) URLs for some referenced assets could change (due to URLs
         // containing a checksum on the resource content).
         element.raw(buildRequireJSConfig(callbacks));
+    }
+
+    public void writeInitialization(Element body, List<String> libraryURLs, List<?> inits)
+    {
 
-        StringBuilder content = new StringBuilder(1000);
+        Element element = body.element("script", "type", "text/javascript");
 
-        content.append(globalMessages.format("core-page-initialization-template",
+        element.raw(globalMessages.format("core-page-initialization-template",
                 convert(libraryURLs),
                 convert(inits)));
-
-        element.raw(content.toString());
     }
 
     private String convert(List<?> input)

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/2dc46950/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/ModuleConfigurationCallback.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/ModuleConfigurationCallback.java b/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/ModuleConfigurationCallback.java
index aef4ad7..5bc60cb 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/ModuleConfigurationCallback.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/ModuleConfigurationCallback.java
@@ -17,24 +17,30 @@ package org.apache.tapestry5.services.javascript;
 import org.apache.tapestry5.json.JSONObject;
 
 /**
- * Used to change the configuration object which will be passed to 
- * <a href="http://requirejs.org/docs/api.html#config">require.config()</a> after it was created
+ * Used to change the configuration object which will be used to
+ * <a href="http://requirejs.org/docs/api.html#config">configure RequireJS</a>; callbacks can modify
+ * and override the configuration after it was created
  * by the {@link ModuleManager} service based on contributed {@link JavaScriptModuleConfiguration}s.
  * This allows components, pages, mixins and services to configure Require.JS dynamically in a
- * per-request basis by using the 
+ * per-request basis by using the
  * {@link JavaScriptSupport#addModuleConfigurationCallback(ModuleConfigurationCallback)} method.
- *  
+ * <p/>
+ * Note that RequireJS is only configured during a full page render; on Ajax requests, RequireJS
+ * will already be loaded and configured.
+ * <p/>
+ *
  * @see JavaScriptSupport#addModuleConfigurationCallback(ModuleConfigurationCallback)
  * @since 5.4
  */
 public interface ModuleConfigurationCallback
 {
     /**
-     * Receives the current configuration, changes it it returns it.
-     * 
-     * @param configuration a {@link JSONObject} containing the current configuration.
+     * Receives the current configuration, which can be copied or returned, or (more typically) modified and returned.
+     *
+     * @param configuration
+     *         a {@link JSONObject} containing the current configuration.
      * @return a {@link JSONObject} containing the changed configuration, most probably the same
-     * one received as a parameter. 
+     *         one received as a parameter.
      */
     JSONObject configure(JSONObject configuration);
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/2dc46950/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/ModuleManager.java
----------------------------------------------------------------------
diff --git a/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/ModuleManager.java b/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/ModuleManager.java
index 3469022..f7d5487 100644
--- a/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/ModuleManager.java
+++ b/tapestry-core/src/main/java/org/apache/tapestry5/services/javascript/ModuleManager.java
@@ -1,4 +1,4 @@
-// Copyright 2012 The Apache Software Foundation
+// Copyright 2012-2013 The Apache Software Foundation
 //
 // Licensed under the Apache License, Version 2.0 (the "License");
 // you may not use this file except in compliance with the License.
@@ -28,34 +28,37 @@ import java.util.List;
  * This is primarily used to wrap non-AMD compliant libraries for use with RequireJS (via contributed {@link JavaScriptModuleConfiguration}s).
  *
  * @since 5.4
+ * @see ModuleConfigurationCallback
  */
 @UsesMappedConfiguration(JavaScriptModuleConfiguration.class)
 public interface ModuleManager
 {
     /**
      * Invoked by the internal {@link org.apache.tapestry5.internal.services.DocumentLinker} service to write the configuration
-     * of the module system into the page, including the tag to load the RequireJS library, and the
-     * necessary initialization of the client-side {@code require} object, including
-     * (critically) its baseUrl property. In addition, a call to the client-side function {@code core/pageinit:loadScriptsAndInitialize}
-     * is constructed to load static scripts and perform page initializations.
+     * for the module system into the page.
+     *
+     * @param body
+     *         {@code <body>} element of the page, to which new {@code <script>} element(s) will be added.
+     * @param moduleConfigurationCallbacks
+     *         a list of {@link org.apache.tapestry5.services.javascript.ModuleConfigurationCallback}s, which
+     *         is used to customize the configuration before it is written.
+     */
+    void writeConfiguration(Element body,
+                            List<ModuleConfigurationCallback> moduleConfigurationCallbacks);
+
+    /**
+     * Invoked by the internal {@link org.apache.tapestry5.internal.services.DocumentLinker} service to write the initializations
+     * (as per {@link JavaScriptSupport#require(String)} into the page; this occurs after the module infrastructure
+     * has been written into the page, along with the core libraries.
      *
      * @param body
      *         {@code <body>} element of the page, to which new {@code <script>} element(s) will be added.
      * @param libraryURLs
-     *         list of additional static JavaScript library URLs that must be loaded on the page, after the
-     *         coreLibraryURLs, and before an initializations
+     *         additional libraries that should be dynamically loaded before evaluating the inits
      * @param inits
-     *         initializations for the page, in the desired execution order. Each element consists of a
-     *         qualified function name, followed by parameters to pass to the function. A qualified function name
-     *         is either a module name, or a module name suffixed with the name of a function property exported by the module
-     *         (separated by a ':', e.g. "myapp/mymodule:myfunc").
-     *         When there are no arguments, the qualified function name may be used; where there are arguments, the
-     *         init must be a JSONArray.
-     * @param moduleConfigurationCallbacks a list of {@link ModuleConfigurationCallback}s. It
-     *         cannot be null. 
+     *         specify initialization on the page, based on loading modules, extacting functions from modules, and invoking those functions
      */
-    void writeInitialization(Element body, List<String> libraryURLs, List<?> inits,
-            List<ModuleConfigurationCallback> moduleConfigurationCallbacks);
+    void writeInitialization(Element body, List<String> libraryURLs, List<?> inits);
 
     /**
      * Given a module name (which may be a path of names separated by slashes), locates the corresponding {@link Resource}.
@@ -68,4 +71,5 @@ public interface ModuleManager
      * @return corresponding resource, or null if not found
      */
     Resource findResourceForModule(String moduleName);
+
 }

http://git-wip-us.apache.org/repos/asf/tapestry-5/blob/2dc46950/tapestry-core/src/test/groovy/org/apache/tapestry5/internal/services/DocumentLinkerImplTest.groovy
----------------------------------------------------------------------
diff --git a/tapestry-core/src/test/groovy/org/apache/tapestry5/internal/services/DocumentLinkerImplTest.groovy b/tapestry-core/src/test/groovy/org/apache/tapestry5/internal/services/DocumentLinkerImplTest.groovy
index a6ff67f..4535403 100644
--- a/tapestry-core/src/test/groovy/org/apache/tapestry5/internal/services/DocumentLinkerImplTest.groovy
+++ b/tapestry-core/src/test/groovy/org/apache/tapestry5/internal/services/DocumentLinkerImplTest.groovy
@@ -1,6 +1,5 @@
 package org.apache.tapestry5.internal.services;
 
-
 import org.apache.tapestry5.dom.Document
 import org.apache.tapestry5.dom.Element
 import org.apache.tapestry5.dom.XMLMarkupModel
@@ -93,7 +92,7 @@ class DocumentLinkerImplTest extends InternalBaseTestCase {
 
         document.newRootElement("html").element("body").element("p").text("Ready to be updated with scripts.")
 
-        def manager = mockModuleManager(["core.js", "foo.js", "bar/baz.js"], [new JSONArray("t5/core/pageinit:evalJavaScript", "pageInitialization();")])
+        def manager = mockModuleManager(["core.js", "foo.js", "bar/baz.js"], [new JSONArray("t5/core/pageinit:evalJavaScript", "pageINIT();")])
 
         DocumentLinkerImpl linker = new DocumentLinkerImpl(manager, true, "1.2.3")
 
@@ -102,13 +101,13 @@ class DocumentLinkerImplTest extends InternalBaseTestCase {
         linker.addLibrary("core.js")
         linker.addLibrary("foo.js")
         linker.addLibrary("bar/baz.js")
-        linker.addScript(InitializationPriority.NORMAL, "pageInitialization();")
+        linker.addScript(InitializationPriority.NORMAL, "pageINIT();")
 
         linker.updateDocument(document)
 
         check document, '''
 <?xml version="1.0"?>
-<html><body data-page-initialized="false"><p>Ready to be updated with scripts.</p><!--MODULE-MANAGER-INITIALIZATION--></body></html>
+<html><body data-page-initialized="false"><p>Ready to be updated with scripts.</p><!--MM-CONFIG--><!--MM-INIT--></body></html>
 '''
 
         verify()
@@ -208,7 +207,7 @@ class DocumentLinkerImplTest extends InternalBaseTestCase {
         linker.updateDocument(document)
 
         check document, '''
-<html><body data-page-initialized="false"><p>Ready to be updated with scripts.</p><!--MODULE-MANAGER-INITIALIZATION--></body></html>
+<html><body data-page-initialized="false"><p>Ready to be updated with scripts.</p><!--MM-CONFIG--><!--MM-INIT--></body></html>
 '''
 
         verify()
@@ -235,7 +234,7 @@ class DocumentLinkerImplTest extends InternalBaseTestCase {
 
         check document, '''
 <?xml version="1.0"?>
-<html><notbody><p>Ready to be updated with scripts.</p></notbody><body data-page-initialized="false"><!--MODULE-MANAGER-INITIALIZATION--></body></html>
+<html><notbody><p>Ready to be updated with scripts.</p></notbody><body data-page-initialized="false"><!--MM-CONFIG--><!--MM-INIT--></body></html>
 '''
 
         verify()
@@ -261,7 +260,7 @@ class DocumentLinkerImplTest extends InternalBaseTestCase {
         linker.updateDocument(document)
 
         check document, '''
-<html><head><meta/><script></script></head><body data-page-initialized="false"><!--MODULE-MANAGER-INITIALIZATION--></body></html>
+<html><head><meta/><script></script></head><body data-page-initialized="false"><!--MM-CONFIG--><!--MM-INIT--></body></html>
 '''
 
         verify()
@@ -309,7 +308,7 @@ class DocumentLinkerImplTest extends InternalBaseTestCase {
     }
 
     @Test
-    void module_based_initialization() throws Exception {
+    void module_based_INIT() throws Exception {
         Document document = new Document()
 
         Element head = document.newRootElement("html").element("head")
@@ -331,14 +330,14 @@ class DocumentLinkerImplTest extends InternalBaseTestCase {
         linker.updateDocument(document)
 
         check document, '''
-<html><head><meta/></head><body data-page-initialized="false"><!--MODULE-MANAGER-INITIALIZATION--></body></html>
+<html><head><meta/></head><body data-page-initialized="false"><!--MM-CONFIG--><!--MM-INIT--></body></html>
 '''
 
         verify()
     }
 
     @Test
-    void module_initialization_with_no_parameters_coalesce() throws Exception {
+    void module_INIT_with_no_parameters_coalesce() throws Exception {
         Document document = new Document()
 
         Element head = document.newRootElement("html").element("head")
@@ -359,7 +358,7 @@ class DocumentLinkerImplTest extends InternalBaseTestCase {
         linker.updateDocument(document)
 
         check document, '''
-<html><head><meta/></head><body data-page-initialized="false"><!--MODULE-MANAGER-INITIALIZATION--></body></html>
+<html><head><meta/></head><body data-page-initialized="false"><!--MM-CONFIG--><!--MM-INIT--></body></html>
 '''
 
         verify()
@@ -369,15 +368,22 @@ class DocumentLinkerImplTest extends InternalBaseTestCase {
 
         ModuleManager mock = newMock(ModuleManager);
 
-        expect(mock.writeInitialization(isA(Element),
-            eq(libraryURLs),
-            eq(inits),
+        expect(mock.writeConfiguration(isA(Element),
             eq([]))).andAnswer({
             def body = EasyMock.currentArguments[0]
 
-            body.comment("MODULE-MANAGER-INITIALIZATION")
+            body.comment("MM-CONFIG")
         } as IAnswer).once()
 
+        expect(mock.writeInitialization(isA(Element),
+            eq(libraryURLs),
+            eq(inits))).andAnswer({
+            def body = EasyMock.currentArguments[0];
+
+            body.comment("MM-INIT");
+        } as IAnswer).once()
+
+
         return mock;
     }
 }