Frontend asset loading #49

rtibbles · 2016-03-08T01:11:53Z

Summary

This is a WIP to implement and provide an example hook for base.html of both synchronous and asynchronous frontend asset loading.

TODO

Have tests been written for the new code?
Has documentation been written/updated?
New dependencies (if any) added to requirements file
Add an entry to CHANGELOG.rst
Add yourself it AUTHORS.rst if you don't appear there

Reviewer guidance

This PR also adds in JS Linting during the webpack build process, in order to compensate for the fact that the JSHint precommit hook doesn't work on Windows and had to be removed.

In addition, it simplifies the naming of frontend assets to provide a simpler naming convention, based solely on the name of their Python module in Kolibri, and the name of the Class in which they are defined in kolibri_plugins.py.

Finally, it provides a Javascript asynchronous frontend asset loader that can load both CSS and Javascript, in order to allow plugins to be registered to the frontend asynchronously and loaded when needed.

Adds a function for creating template tags with a hook to allow arbitrary plugins to plugin synchronously and asynchronously into any template.

Creates tags for these for base.html to allow for extension of base.html frontend functionality and as an example.

Issues addressed

Fixes #41, and fixes #38

Documentation

Much inline documentation. Also updates frontend.rst.

…definition.

rtibbles · 2016-03-09T00:26:29Z

Woops, broke coverage.

codecov-io · 2016-03-09T00:29:51Z

Current coverage is `88.29%`

Merging #49 into master will increase coverage by +4.03% as of 8050f0d

@@            master     #49   diff @@
======================================
  Files           38      40     +2
  Stmts         1017    1188   +171
  Branches       102     129    +27
  Methods          0       0       
======================================
+ Hit            857    1049   +192
+ Partial         23      22     -1
+ Missed         137     117    -20

Review entire Coverage Diff as of 8050f0d

Powered by Codecov. Updated on successful CI builds.

rtibbles · 2016-03-09T00:41:24Z

Woot, the asset-loader module has full code coverage!

.jshintrc

@@ -0,0 +1,20 @@
+{


aronasorman · 2016-03-10T18:13:38Z

Making the frontend asset naming consistent across JS and Python, some linting config. Changes look OK.

…egistration.

… Mediator.

Modify example plugin to use new Plugin object as Base. Ignore injected global __plugin_name in linting.

In order to extract the events data required for async loading.

…or the core app. Return async files from plugin registration.

Adds template tags for the base template sync and async frontend hooks. Adds async example to example plugin.

rtibbles · 2016-03-11T23:23:45Z

Pushing latest changes in case anyone wants to look at them.

Tests have not been updated, so will break. Documentation needs to be written.

Update build process accordingly. Update docs.

rtibbles · 2016-04-09T00:31:31Z

After some discussion with @indirectlylit and @MCGallaspy, and due to some issues with the approach taken to inspect the Javascript code and extract the events and once hashes at compilation time to allow for asynchronous loading - I have now changed the events and once hashes to be defined within the classes subclassed from KolibriFrontEndBase.

Configuration of these hashes now happens from within kolibri_plugin.py, and is then injected into the Javascript code at build time.

MCGallaspy · 2016-04-11T18:40:33Z

Can you also update this section: https://github.com/rtibbles/kolibri/blob/frontend_asset_loading/docs/dev/frontend.rst#writing-frontend-plugins

Also, who is responsible for merging this PR?

rtibbles · 2016-04-11T18:42:21Z

Good catch. Changing.

rtibbles · 2016-04-11T18:42:55Z

@aronasorman is assigned - I'll see if he is happy to sign it off.

kolibri/plugins/assets/src/plugin_base/plugin_base.js

+// The Backbone Model extend method is a standalone function that is used to extend many Backbone objects.
+// We use it here in preference to rolling our own to allow for extension of Plugins.
+
+Plugin.extend = Backbone.Model.extend;


kolibri/core/kolibri_plugin.py

@@ -38,6 +38,7 @@ class KolibriCoreFrontEnd(KolibriFrontEndPluginBase):
    """
    entry_file = "assets/src/kolibri_core_app.js"
    external = True
+    core = True


indirectlylit · 2016-04-11T19:58:49Z

I'm a bit confused by the various webpack config files in the root directory:

parse_bundle_plugin.js and karma.conf.js reference webpack.config.base.js
webpack.config.dev.js references webpack.config.js
webpack.config.base.js and webpack.config.js do not reference each other

Seems like perhaps both webpack.config.base.js and webpack.config.js are trying to play the "base" role?

rtibbles · 2016-04-11T20:03:43Z

webpack.config.base.js is the base config that gets used for all other webpack configs (so shared across tests, production building, and dev building).

parse_bundle_plugin.js is at the end of a chain of three functions that build the configuration for webpack.config.js, so acts as the point of entry for the base config to get into the webpack configuration.

I will annotate the relevant files to make their roles more clear. Thanks for the feedback!

kolibri/core/templatetags/kolibri_tags.py

+    return frontend_sync(BASE_FRONTEND_SYNC)
+
+
+def frontend_async(hook):


…sions. Removes unnecessary jsdom dependency.

Adds sourcemaps to karma tests for easier debugging.

kolibri/core/templatetags/kolibri_tags.py

        if chunk['name'].endswith('.js'):
-            tags.append('<script type="text/javascript" src="{static}/{url}"></script>'.format(static=static, url=url))
+            tags.append('<script type="text/javascript" src="{url}"></script>'.format(url=render_as_url(chunk)))


rtibbles · 2016-04-12T16:23:41Z

This PR is now a month old. It is also being used in active code development by @MCGallaspy and @DXCanas - I believe all interested parties have had the opportunity to review, and I have responded to all comments.

@aronasorman has said to me that he does not have the time or familiarity to give the go ahead, as such, and due to the foregoing that there has been ample review, I will be merging this at EOD today if there are no strenuous objections raised prior to then.

MCGallaspy · 2016-04-12T20:34:40Z

Yeah, I think further iteration on this can be addressed in subsequent PRs.

rtibbles added 4 commits March 5, 2016 00:12

Adds jshint preloader to webpack. Adds loglevel to Kolibri core package.

7c8fdf6

Simplify frontend asset naming to be directly tied to kolibri plugin …

c435277

…definition.

Fixes js linting.

e4652e5

Makes convenience method to return plugin name.

be0e9f6

rtibbles assigned aronasorman Mar 8, 2016

Creates an asynchronous frontend asset loader with test suite.

3572c04

rtibbles force-pushed the frontend_asset_loading branch from 9c628bc to 3572c04 Compare March 9, 2016 00:27

aronasorman reviewed Mar 9, 2016
View reviewed changes

.jshintrc

@@ -0,0 +1,20 @@

{

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

rtibbles added 7 commits March 11, 2016 15:04

Fleshes out Mediator object for synchronous and asynchronous plugin r…

1c3796f

…egistration.

Implements Facade within kolibri_core_app to expose public methods of…

bd96fcd

… Mediator.

Reconfigure webpack to handle dependencies bundled inside Facade.

71385fd

Add base plugin object.

e6a1c63

Modify example plugin to use new Plugin object as Base. Ignore injected global __plugin_name in linting.

Creates webpack plugin that introspects a plugin bundle on completion,

eee08d1

In order to extract the events data required for async loading.

Adds Python support for 'core' property of plugins - a special flag f…

8de72e1

…or the core app. Return async files from plugin registration.

Adds templating functions for rendering synchronous and async loading.

55fc6c7

Adds template tags for the base template sync and async frontend hooks. Adds async example to example plugin.

rtibbles added 3 commits March 11, 2016 16:40

Fixes existing Python and Mocha tests.

fba87b1

Share webpack config between building and tests.

9da0ce5

Adds some tests for mediator.

4670aa2

rtibbles force-pushed the frontend_asset_loading branch 4 times, most recently from 6d0264b to fec6aab Compare March 12, 2016 17:26

Force node version on travis.

0e74c67

rtibbles force-pushed the frontend_asset_loading branch from fec6aab to 0e74c67 Compare March 12, 2016 17:41

rtibbles added 2 commits April 8, 2016 16:00

Flesh out frontend docs. Add explicit async_frontend tag.

97fb91f

Move events and once hashes from Javascript to Python.

709f432

Update build process accordingly. Update docs.

Clean up the Frontend docs a little more.

b823469

rtibbles force-pushed the frontend_asset_loading branch from c97dbe5 to 30dd424 Compare April 11, 2016 19:16

indirectlylit reviewed Apr 11, 2016
View reviewed changes

rtibbles force-pushed the frontend_asset_loading branch from 30dd424 to c048e5d Compare April 11, 2016 19:33

indirectlylit reviewed Apr 11, 2016
View reviewed changes

rtibbles force-pushed the frontend_asset_loading branch 2 times, most recently from e2ca7f7 to 2f50a55 Compare April 11, 2016 19:57

rtibbles force-pushed the frontend_asset_loading branch 6 times, most recently from 7504170 to c8af859 Compare April 11, 2016 21:13

indirectlylit reviewed Apr 11, 2016
View reviewed changes

Attempt to force node version and run on all three supported node ver…

2013a7a

…sions. Removes unnecessary jsdom dependency.

rtibbles force-pushed the frontend_asset_loading branch from c8af859 to 2013a7a Compare April 11, 2016 21:24

Adds some annotations to webpack config files.

c0df091

Adds sourcemaps to karma tests for easier debugging.

indirectlylit reviewed Apr 12, 2016
View reviewed changes

MCGallaspy merged commit c57f4e8 into learningequality:master Apr 12, 2016

rtibbles deleted the frontend_asset_loading branch September 8, 2016 17:15

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Frontend asset loading #49

Frontend asset loading #49

rtibbles commented Mar 8, 2016

rtibbles commented Mar 9, 2016

codecov-io commented Mar 9, 2016

rtibbles commented Mar 9, 2016

This comment was marked as spam.

This comment was marked as spam.

aronasorman commented Mar 10, 2016

rtibbles commented Mar 11, 2016

rtibbles commented Apr 9, 2016

MCGallaspy commented Apr 11, 2016

rtibbles commented Apr 11, 2016

rtibbles commented Apr 11, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

indirectlylit commented Apr 11, 2016

rtibbles commented Apr 11, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

rtibbles commented Apr 12, 2016

MCGallaspy commented Apr 12, 2016

		return frontend_sync(BASE_FRONTEND_SYNC)


		def frontend_async(hook):

Frontend asset loading #49

Frontend asset loading #49

Conversation

rtibbles commented Mar 8, 2016

Summary

TODO

Reviewer guidance

Issues addressed

Documentation

rtibbles commented Mar 9, 2016

codecov-io commented Mar 9, 2016

Current coverage is 88.29%

rtibbles commented Mar 9, 2016

This comment was marked as spam.

This comment was marked as spam.

aronasorman commented Mar 10, 2016

rtibbles commented Mar 11, 2016

rtibbles commented Apr 9, 2016

MCGallaspy commented Apr 11, 2016

rtibbles commented Apr 11, 2016

rtibbles commented Apr 11, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

indirectlylit commented Apr 11, 2016

rtibbles commented Apr 11, 2016

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

rtibbles commented Apr 12, 2016

MCGallaspy commented Apr 12, 2016

Current coverage is `88.29%`