Split assets environment in two

The web application adds asset links to pages with the help of an assets environment (an instance of `h.assets.Environment`). The environment knows how to map bundle names such as "site_js" to a series of URLs for inclusion -- it does this by reading two files: - `assets.ini`, which maps bundle names to a list of individual asset filenames. - `manifest.json`, which maps individual asset filenames to URL paths (for the purposes of busting browser caches). Up until now both client and site assets have been built in the same step, and have written out a single `manifest.json`. Now, with the client installed with npm, there are two manifest files: one for the client and one for the site assets. This commit splits the rest of the asset handling into two to match this division. Client asset bundles are now defined in h/assets_client.ini, and the client assets have a separate environment instance which reads the manifest from the installed `hypothesis` package in `node_modules/`. We can remove this additional assets environment and configuration once two things have happened: 1. Responsibility for generating `app.html` and `embed.js` have moved into the client build. 2. The remaining site pages which depend on the client CSS (AKA `app_css`) have been moved to site CSS.
hypothesis · Jul 1, 2016 · e06157c · e06157c
1 parent 35e962d
commit e06157c
Show file tree

Hide file tree

Showing 15 changed files with 75 additions and 56 deletions.
diff --git a/MANIFEST.in b/MANIFEST.in
@@ -11,6 +11,7 @@ graft docs
 prune docs/_build
 include h/accounts/blacklist
 include h/assets.ini
+include h/assets_client.ini
 graft h/migrations
 graft h/static
 graft h/templates

diff --git a/h/app.py b/h/app.py
@@ -11,19 +11,18 @@
 from pyramid.settings import asbool
 from pyramid.tweens import EXCVIEW
 
-from h import assets
 from h.auth.policy import AuthenticationPolicy
 from h.config import configure
 
 log = logging.getLogger(__name__)
 
 
 def configure_jinja2_assets(config):
-    assets_env = assets.Environment('/assets',
-                                    'h/assets.ini',
-                                    'build/manifest.json')
     jinja2_env = config.get_jinja2_environment()
-    jinja2_env.globals['asset_urls'] = assets_env.urls
+    jinja2_env.globals['asset_urls'] = config.registry['assets_env'].urls
+    # FIXME: this should not be used by new templates. Once no more templates
+    # depend on app_css this should go.
+    jinja2_env.globals['asset_client_urls'] = config.registry['assets_client_env'].urls
 
 
 def in_debug_mode(request):

diff --git a/h/assets.ini b/h/assets.ini
@@ -14,38 +14,6 @@ admin_js =
   scripts/admin-site.bundle.js
 
 
-# The H client application
-app_js =
-  scripts/raven.bundle.js
-  scripts/angular.bundle.js
-  scripts/katex.bundle.js
-  scripts/showdown.bundle.js
-  scripts/polyfills.bundle.js
-  scripts/unorm.bundle.js
-  scripts/app.bundle.js
-
-app_css =
-  styles/angular-csp.css
-  styles/angular-toastr.css
-  styles/icomoon.css
-  styles/katex.min.css
-  styles/app.css
-
-
-# The inject bundle is intended to be loaded into pages for bootstrapping the
-# application. It sets up RPC channels for cross-domain communication between
-# frames participating in annotation by using the annotator bridge plugin.
-inject_js =
-  scripts/polyfills.bundle.js
-  scripts/jquery.bundle.js
-  scripts/injector.bundle.js
-
-inject_css =
-  styles/icomoon.css
-  styles/inject.css
-  styles/pdfjs-overrides.css
-
-
 # The H website
 site_js =
   scripts/jquery.bundle.js

diff --git a/h/assets.py b/h/assets.py
@@ -118,16 +118,35 @@ def _load_bundles(fp):
     parser.readfp(fp)
     return {k: aslist(v) for k, v in parser.items('bundles')}
 
+# Site assets
+assets_view = static_view('h:../build',
+                          cache_max_age=None,
+                          use_subpath=True)
+assets_view = _add_cors_header(assets_view)
+
+
+# Client assets
+assets_client_view = static_view('h:../node_modules/hypothesis/build',
+                                 cache_max_age=None,
+                                 use_subpath=True)
+assets_client_view = _add_cors_header(assets_client_view)
+
 
 def includeme(config):
-    assets_view = _add_cors_header(static_view('h:../build',
-        cache_max_age=None,
-        use_subpath=True))
-    config.add_view(view=assets_view, route_name='assets')
+    config.add_view(route_name='assets', view=assets_view)
+    config.add_view(route_name='assets_client', view=assets_client_view)
+
+    config.add_route('assets_client', '/assets/client/*subpath')
     config.add_route('assets', '/assets/*subpath')
 
     assets_env = Environment('/assets',
                              'h/assets.ini',
                              'build/manifest.json')
-    config.add_request_method(
-        lambda r: assets_env, name='assets_env', reify=True)
+    assets_client_env = Environment('/assets/client',
+                                    'h/assets_client.ini',
+                                    'node_modules/hypothesis/build/manifest.json')
+
+    # We store the environment objects on the registry so that the Jinja2
+    # integration can be configured in app.py
+    config.registry['assets_env'] = assets_env
+    config.registry['assets_client_env'] = assets_client_env
diff --git a/h/assets_client.ini b/h/assets_client.ini
@@ -0,0 +1,31 @@
+[bundles]
+# The H client application
+app_js =
+  scripts/raven.bundle.js
+  scripts/angular.bundle.js
+  scripts/katex.bundle.js
+  scripts/showdown.bundle.js
+  scripts/polyfills.bundle.js
+  scripts/unorm.bundle.js
+  scripts/app.bundle.js
+
+app_css =
+  styles/angular-csp.css
+  styles/angular-toastr.css
+  styles/icomoon.css
+  styles/katex.min.css
+  styles/app.css
+
+
+# The inject bundle is intended to be loaded into pages for bootstrapping the
+# application. It sets up RPC channels for cross-domain communication between
+# frames participating in annotation by using the annotator bridge plugin.
+inject_js =
+  scripts/polyfills.bundle.js
+  scripts/jquery.bundle.js
+  scripts/injector.bundle.js
+
+inject_css =
+  styles/icomoon.css
+  styles/inject.css
+  styles/pdfjs-overrides.css
diff --git a/h/templates/accounts/claim_account_legacy.html.jinja2 b/h/templates/accounts/claim_account_legacy.html.jinja2
@@ -3,7 +3,7 @@
 {% block page_title %}Claim account{% endblock page_title %}
 
 {% block styles %}
-{% for url in asset_urls("app_css") %}
+{% for url in asset_client_urls("app_css") %}
 <link rel="stylesheet" href="{{ url }}">
 {% endfor %}
 {% endblock %}

diff --git a/h/templates/accounts/forgot_password.html.jinja2 b/h/templates/accounts/forgot_password.html.jinja2
@@ -3,7 +3,7 @@
 {% block page_title %}Password reset{% endblock %}
 
 {% block styles %}
-{% for url in asset_urls("app_css") %}
+{% for url in asset_client_urls("app_css") %}
 <link rel="stylesheet" href="{{ url }}">
 {% endfor %}
 {% endblock styles  %}

diff --git a/h/templates/accounts/login.html.jinja2 b/h/templates/accounts/login.html.jinja2
@@ -3,7 +3,7 @@
 {% block page_title %}Sign in{% endblock %}
 
 {% block styles %}
-{% for url in asset_urls("app_css") %}
+{% for url in asset_client_urls("app_css") %}
 <link rel="stylesheet" href="{{ url }}">
 {% endfor %}
 {% endblock styles  %}

diff --git a/h/templates/accounts/register.html.jinja2 b/h/templates/accounts/register.html.jinja2
@@ -3,7 +3,7 @@
 {% block page_title %}Create an account{% endblock %}
 
 {% block styles %}
-{% for url in asset_urls("app_css") %}
+{% for url in asset_client_urls("app_css") %}
 <link rel="stylesheet" href="{{ url }}">
 {% endfor %}
 {% endblock styles  %}

diff --git a/h/templates/accounts/reset_password.html.jinja2 b/h/templates/accounts/reset_password.html.jinja2
@@ -3,7 +3,7 @@
 {% block page_title %}Password reset{% endblock %}
 
 {% block styles %}
-{% for url in asset_urls("app_css") %}
+{% for url in asset_client_urls("app_css") %}
 <link rel="stylesheet" href="{{ url }}">
 {% endfor %}
 {% endblock styles  %}

diff --git a/h/templates/layouts/profile.html.jinja2 b/h/templates/layouts/profile.html.jinja2
@@ -9,7 +9,7 @@
 {% block page_title %}{{ page_title }}{% endblock %}
 
 {% block styles %}
-{% for url in asset_urls("app_css") %}
+{% for url in asset_client_urls("app_css") %}
 <link rel="stylesheet" href="{{ url }}">
 {% endfor %}
 {% endblock styles  %}

diff --git a/h/templates/notfound.html.jinja2 b/h/templates/notfound.html.jinja2
@@ -3,7 +3,7 @@
 {% block title %}Page Not Found{% endblock %}
 
 {% block styles %}
-{% for url in asset_urls("app_css") %}
+{% for url in asset_client_urls("app_css") %}
 <link rel="stylesheet" href="{{ url }}">
 {% endfor %}
 {% endblock %}

diff --git a/h/templates/unsubscribe.html.jinja2 b/h/templates/unsubscribe.html.jinja2
@@ -3,7 +3,7 @@
 {% block title %}Unsubscribed{% endblock %}
 
 {% block styles %}
-{% for url in asset_urls("app_css") %}
+{% for url in asset_client_urls("app_css") %}
 <link rel="stylesheet" href="{{ url }}">
 {% endfor %}
 {% endblock %}

diff --git a/h/views/client.py b/h/views/client.py
@@ -23,7 +23,7 @@ def render_app(request, extra=None):
     """Render a page that serves a preconfigured annotation client."""
     client_sentry_dsn = request.registry.settings.get('h.client.sentry_dsn')
     html = client.render_app_html(
-        assets_env=request.assets_env,
+        assets_env=request.registry['assets_client_env'],
         # FIXME: The '' here is to ensure this has a trailing slash. This seems
         # rather messy, and is inconsistent with the rest of the application's
         # URLs.
@@ -62,7 +62,7 @@ def annotator_token(request):
 def embed(context, request):
     request.response.content_type = b'text/javascript'
     request.response.text = client.render_embed_js(
-        assets_env=request.assets_env,
+        assets_env=request.registry['assets_client_env'],
         app_html_url=request.route_url('widget'),
         base_url=request.route_url('index'))
     return request.response

diff --git a/tests/h/views/main_test.py b/tests/h/views/main_test.py
@@ -53,9 +53,10 @@ def document_title(patch):
 
 
 @pytest.fixture
-def pyramid_request(pyramid_request):
-    pyramid_request.assets_env = mock.Mock()
-    return pyramid_request
+def pyramid_config(pyramid_config):
+    # Pretend the client assets environment has been configured
+    pyramid_config.registry['assets_client_env'] = mock.Mock()
+    return pyramid_config
 
 
 @pytest.fixture