diff --git a/js-extensions/.gitignore b/js-extensions/.gitignore index 6bb61551..e8280cb8 100644 --- a/js-extensions/.gitignore +++ b/js-extensions/.gitignore @@ -1 +1,3 @@ -ExtensionPoint.js \ No newline at end of file +ExtensionPoint.js +dist/ + diff --git a/js-extensions/index.js b/js-extensions/index.js index 61527285..3fbbdc4e 100644 --- a/js-extensions/index.js +++ b/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'); diff --git a/js-extensions/package.json b/js-extensions/package.json index f8bc6eeb..6a5db54a 100644 --- a/js-extensions/package.json +++ b/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" }, diff --git a/js-extensions/ExtensionPoint.jsx b/js-extensions/src/ExtensionPoint.jsx similarity index 60% rename from js-extensions/ExtensionPoint.jsx rename to js-extensions/src/ExtensionPoint.jsx index c17cf427..c4238c2a 100644 --- a/js-extensions/ExtensionPoint.jsx +++ b/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; diff --git a/js-extensions/store.js b/js-extensions/src/store.js similarity index 98% rename from js-extensions/store.js rename to js-extensions/src/store.js index a4cd1949..ac38c537 100644 --- a/js-extensions/store.js +++ b/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.