#UX-33 * Some small improvements in js-extensions, and bump version number in anticipation of re-publish

js-extensions/.gitignore

@@ -1 +1,3 @@
-ExtensionPoint.js
+ExtensionPoint.js
+dist/
+

js-extensions/index.js

@@ -1,3 +1,3 @@
 
-exports.store = require('./store.js');
-exports.ExtensionPoint = require('./ExtensionPoint.js');
+exports.store = require('./dist/store.js');
+exports.ExtensionPoint = require('./dist/ExtensionPoint.js');

js-extensions/package.json

@@ -1,10 +1,11 @@
 {
   "name": "@jenkins-cd/js-extensions",
-  "version": "0.0.7",
+  "version": "0.0.8",
   "description": "Jenkins Extension Store",
   "main": "index.js",
+  "files": ["index.js","dist","README.md"],
   "scripts": {
-    "build": "jsx ExtensionPoint.jsx > ExtensionPoint.js",
+    "build": "jsx src/ExtensionPoint.jsx > dist/ExtensionPoint.js && cp src/store.js dist/",
     "test": "echo \"Error: no test specified\" && exit 1",
     "prepublish": "npm run build"
   },

js-extensions/src/ExtensionPoint.jsx

@@ -2,6 +2,8 @@ var React = require('react');
 var ReactDOM = require('react-dom');
 var store = require('./store.js');
 
+// TODO: Move this package to babel, and update this to ES6
+
 var ExtensionPoint = React.createClass({
 
     getInitialState: function () {
@@ -11,7 +13,7 @@ var ExtensionPoint = React.createClass({
 
     componentDidMount: function() {
         var thisEp = this;
-        store.loadExtensions(this.props.name, function(extensions) {
+        ExtensionPoint.registerExtensionPoint(this.props.name, function(extensions) {
             thisEp.setState({
                 extensions: extensions
             });
@@ -26,9 +28,12 @@ var ExtensionPoint = React.createClass({
         this._unmountAllExtensions();
     },
 
-    // TODO: resolve possible differences/inconsistencies between render, _renderAllExtensions and _renderExtension
-    // Something looks wrong with all of that... nested render, render, render
-
+    /**
+     * This method renders the "leaf node" container divs, one for each registered extension, that live in the same
+     * react hierarchy as the <ExtensionPoint> instance itself. As far as our react is concerned, these are
+     * childless divs that are never updated. Actually rendering the extensions themselves is done by
+     * _renderAllExtensions.
+     */
     render: function() {
         var extensions = this.state.extensions;
 
@@ -49,8 +54,15 @@ var ExtensionPoint = React.createClass({
         }
     },
 
+    /**
+     * For each extension, we have created a "leaf node" element in the DOM. This method creates a new react hierarchy
+     * for each, and instructs it to render. From that point on we have a separation that keeps the main app insulated
+     * from any plugin issues that may cause react to throw while updating. Inspired by Nylas N1.
+     */
     _renderAllExtensions: function() {
-        // TODO: This needs to be a lot cleverer if the list of extensions for a specific point can change
+        // NB: This needs to be a lot cleverer if the list of extensions for a specific point can change;
+        // We will need to link each extension with its containing element, in some way that doesn't leak :) Easy in
+        // browsers with WeakMap, less so otherwise.
         const el = ReactDOM.findDOMNode(this);
         if (el) {
             const children = el.children;
@@ -84,11 +96,23 @@ var ExtensionPoint = React.createClass({
         }
     },
 
-    /** Clean up child extensions */
+    /**
+     * Clean up child extensions' react hierarchies. Necessary because they live in their own react hierarchies that
+     * would otherwise not be notified when this is being unmounted.
+     */
     _unmountAllExtensions: function() {
         var children = ReactDOM.findDOMNode(this).children;
         for (var i = 0; i < children.length; i++) {
-            ReactDOM.unmountComponentAtNode(children[i]); // TODO: Can this throw?
+            var child = children[i];
+            try {
+                if (child) {
+                    ReactDOM.unmountComponentAtNode(child);
+                }
+            }
+            catch (err) {
+                // Log and continue, don't want to stop unmounting children
+                console.log("Error unmounting component", child, err);
+            }
         }
     }
 });
@@ -102,4 +126,20 @@ ExtensionPoint.propTypes = {
     wrappingElement: React.PropTypes.oneOfType([React.PropTypes.string, React.PropTypes.element])
 };
 
+/**
+ * Provide a static helper to avoid having to expose the store
+ */
+ExtensionPoint.getExtensions = function getExtensions(name) {
+    return store.getExtensions(name);
+};
+
+/**
+ * Register the existence of an ExtensionPoint and load the extensions. onLoad is (extensions)=>{}
+ */
+ExtensionPoint.registerExtensionPoint = function registerExtensionPoint (name, onLoad) {
+    store.loadExtensions(name, (extensions) => {
+        if (typeof onLoad === "function") onLoad(extensions);
+    });
+};
+
 module.exports = ExtensionPoint;

js-extensions/src/store.js

@@ -59,6 +59,7 @@ LoadCountMonitor.prototype.onchange = function(callback) {
 };
 
 function loadBundles(extensionPointId, onBundlesLoaded) {
+
     var jsModules = require('@jenkins-cd/js-modules');
     var loadCountMonitor = new LoadCountMonitor();
 
@@ -100,9 +101,12 @@ function loadBundles(extensionPointId, onBundlesLoaded) {
     // the extension point .js bundle (if not already loaded) for each of the
     // plugins that implement the specified extensionPointId.
     for(var i1 = 0; i1 < extensionPointList.length; i1++) {
+
         var pluginMetadata = extensionPointList[i1];
         var extensions = pluginMetadata.extensions;
 
+        console.log("md", pluginMetadata, "extensions", extensions);
+
         for(var i2 = 0; i2 < extensions.length; i2++) {
             if (extensions[i2].extensionPoint === extensionPointId) {
                 // This plugin implements the ExtensionPoint.