fix issues with svg build. Fixes #29 Refs #30

Author: Youcef Mammar

README.md

@@ -72,6 +72,6 @@ This plugin provides an ES6 Promise implementation. If the browser does not prov
 See [docs/Promise.md](./docs/Promise.md) for documentation.
 
 ## svg
-This plugins provide an API for loading and bundling svg graphics.
+This plugin loads an svg graphic and defines it in the DOM, so you can reference it in a `<use>` tag.
 
 See [docs/svg.md](./docs/svg.md) for documentation.

bower.json

@@ -5,7 +5,8 @@
 	"dependencies": {
 		"lie": ">=2.8",
 		"requirejs": "2.1.x",
-		"requirejs-text": "2.0.x"
+		"requirejs-text": "2.0.x",
+		"requirejs-domready": "2.0.x"
 	},
 	"devDependencies": {
 		"jquery": ">=2.1"

docs/svg.md

@@ -5,20 +5,20 @@ title: requirejs-dplugins/svg
 
 # requirejs-dplugins/svg!
 
-This plugin load svg graphics and declares them in one sprite. This sprite is automatically added to your DOM, so that you can reference included graphics in a `<use>` tag.
+This plugin loads an svg graphic and defines it in the DOM, so you can reference it in a `<use>` tag.
 
 ## Example
 
 ```js
 define([
-    "requirejs-dplugins/svg!./icon1.svg",
-    "requirejs-dplugins/svg!./icon2.svg"
+    "requirejs-dplugins/svg!./icon1.svg", // <svg id="icon1"...
+    "requirejs-dplugins/svg!./icon2.svg"  // <svg id="icon2"...
 ], function(id1, id2){
 	// id1 === "icon1" and id2 === "icon2"
 })
 ```
 
-This will fetch `icon1.svg` and `icon2.svg` and add two symbols to the sprite.
+This will fetch `icon1.svg` and `icon2.svg` and define two symbols in the DOM
 ```svg
 <svg>
 	...
@@ -27,27 +27,32 @@ This will fetch `icon1.svg` and `icon2.svg` and add two symbols to the sprite.
 </svg>
 ```
 
-You can then use the icons anytime only with
+You can then use the icons anytime with
 
 ```
 <svg>
 	<use xlink:href="#icon1"></use>
 </svg>
 ```
 
-If the first `<svg>` tag of your graphic holds an `id` attribute, this id will be used as the reference. Otherwise, the name of the file is used.
+Note that the first `<svg>` tag of your graphic should have an `id` attribute which will be used as the reference.
+It should also have a `viewBox` attribute. 
 
-## Build
-The build step will merge all graphics in one sprite beforehand and save the result in a `<layer>.svg`. 
-When running the built version, this sprite is fetched as soon as one of the graphic inside is required.
-
-
-## Creating graphics that work well with this plugin 
-
-To work properly, your graphic should include a `viewBox` attribute. The `id` is optional. 
-As an example, here is the minimal markup your graphic should include:
+As an example, here is the minimal markup your graphic should follow:
 
 ```svg
 <svg id="my-graphic" viewBox="0 0 80 120"> ... </svg>
 ```
 
+## Build
+`jsdom` is used during the build step to merge all graphics in one sprite beforehand and save the result in a `<layer>.svg`. 
+When running the built version, this sprite is fetched as soon as one of the graphics inside is required.
+
+Note that `jsdom` should be added to your application `devDependencies` property in `package.json` so it is
+automatically installed with `npm install`.
+The following command will do that automatically:
+
+```bash
+$ npm install --save-dev jsdom
+``` 
+

samples/svg.html

@@ -7,14 +7,11 @@
 
 	<script type="text/javascript">
 		require.config({
-			baseUrl: "..",
-			paths: {
-				'requirejs-text': '../requirejs-text/'
-			}
+			baseUrl: "../.."
 		});
 		require([
-			"svg!./samples/svg/add.svg",
-			"svg!./samples/svg/download.svg"
+			"requirejs-dplugins/svg!requirejs-dplugins/samples/svg/add.svg",
+			"requirejs-dplugins/svg!requirejs-dplugins/samples/svg/download.svg"
 		]);
 	</script>
 </head>

svg.js

@@ -1,13 +1,13 @@
 /**
  * Svg loading plugin.
  *
- * This plugins loads SVG files and merges them into one SVG sprite
+ * This plugin loads an svg graphic and defines it in the DOM, so you can reference it in a `<use>` tag.
  *
  * @example:
  *      To load the svg file `myicons/myicon.svg`:
  *      ```
- *      require(["requirejs-dplugins/svg!myicons/myicon.svg"], function (){
- *          // myicon was added to the sprite
+ *      require(["requirejs-dplugins/svg!myicons/myicon.svg"], function (myiconId){
+ *          // myicon was added to the DOM
  *      });
  *      ```
  * @module requirejs-dplugins/svg
@@ -16,11 +16,14 @@
 define([
 	"./has",
 	"./Promise!",
-	"module"
-], function (has, Promise, module) {
+	"module",
+	"require",
+	"requirejs-text/text",
+	"requirejs-domready/domReady"
+], function (has, Promise, module, localRequire, textPlugin) {
 	"use strict";
 
-	var cache = {}, // paths of loaded svgs
+	var loaded = {}, // paths of loaded svgs
 		SPRITE_ID = 'requirejs-dplugins-svg',
 		sprite = null;
 
@@ -31,47 +34,43 @@ define([
 		/**
 		 * Loads an svg file.
 		 * @param {string} path - The svg file to load.
-		 * @param {Function} require - A local require function to use to load other modules.
+		 * @param {Function} resourceRequire - A require function local to the module calling the svg plugin.
 		 * @param {Function} onload - A function to call when the specified svg file have been loaded.
 		 * @method
 		 */
-		load: function (path, require, onload) {
+		load: function (path, resourceRequire, onload) {
 			if (has("builder")) { // when building
-				cache[path] = true;
+				loaded[path] = true;
 				onload();
 			} else { // when running
-
 				// special case: when running a built version
 				// Replace graphic by corresponding sprite.
+				var idInLayer;
 				var layersMap = module.config().layersMap;
-				if (layersMap) {
-					path = layersMap[path] || path;
+				if (layersMap && layersMap[path]) {
+					idInLayer = layersMap[path].id;
+					path = layersMap[path].redirectTo;
 				}
 
-				var filename = getFilename(path);
-				if (path in cache) {
-					cache[path].then(function (graphic) {
-						onload(graphic.id);
-					});
-				} else {
-					if (!sprite) {
-						sprite = createSprite(document, SPRITE_ID);
-						document.body.appendChild(sprite);
-					}
-					cache[path] = new Promise(function (resolve) {
-						require(['requirejs-text/text!' + path], function (svgText) {
-							var graphic = extractGraphic(document, svgText, filename),
-								symbol = createSymbol(document, graphic.id, graphic.element, graphic.viewBox);
-							sprite.appendChild(symbol);
-							cache[path] = graphic.id;
-							resolve(graphic);
+				if (!(path in loaded)) {
+					loaded[path] = new Promise(function (resolve) {
+						localRequire(["requirejs-domready/domReady!"], function () {
+							textPlugin.load(path, resourceRequire, function (svgText) {
+								if (!sprite) {
+									sprite = createSprite(document, SPRITE_ID);
+									document.body.appendChild(sprite);
+								}
+								var symbol = extractGraphicAsSymbol(document, svgText);
+								sprite.appendChild(symbol);
+								resolve(symbol.getAttribute("id"));
+							});
 						});
 					});
-
-					cache[path].then(function (graphic) {
-						onload(graphic.id);
-					});
 				}
+
+				loaded[path].then(function (symbolId) {
+					onload(idInLayer || symbolId);
+				});
 			}
 		}
 	};
@@ -89,8 +88,8 @@ define([
 			 *     config: {
 			 *         "requirejs-dplugins/svg": {
 			 *             layersMap: {
-			 *                 "file1.svg": "path/to/layer.svg",
-			 *                 "file2.svg": "path/to/layer.svg"
+			 *                 "file1.svg": {redirectTo: "path/to/layer.svg", id: "id-inside-file-1"},
+			 *                 "file2.svg": {redirectTo: "path/to/layer.svg", id: "id-inside-file-2"}
 			 *             }
 			 *         }
 			 *     }
@@ -101,19 +100,19 @@ define([
 			 * and writes it to the modules layer.
 			 * @param {string} mid - Current module id.
 			 * @param {string} dest - Current svg sprite path.
-			 * @param {Array} loadList - List of svg files contained in current sprite.
+			 * @param {Array} loaded - Maps the paths of the svg files contained in current sprite to their ids.
 			 */
-			writeConfig: function (write, mid, destMid, loadList) {
+			writeConfig: function (write, mid, destMid, loaded) {
 				var svgConf = {
 					config: {},
 					paths: {}
 				};
 				svgConf.config[mid] = {
 					layersMap: {}
 				};
-				loadList.forEach(function (path) {
-					svgConf.config[mid].layersMap[path] = destMid;
-				});
+				for (var path in loaded) {
+					svgConf.config[mid].layersMap[path] = {redirectTo: destMid, id: loaded[path]};
+				}
 
 				write("require.config(" + JSON.stringify(svgConf) + ");");
 			},
@@ -124,84 +123,101 @@ define([
 			 * @param {Function} writePluginFiles - The write function provided by the builder to `writeFile`.
 			 * and writes it to the modules layer.
 			 * @param {string} dest - Current svg sprite path.
-			 * @param {Array} loadList - List of svg files contained in current sprite.
+			 * @param {Array} loaded - Maps the paths of the svg files contained in current sprite to their ids.
 			 */
-			writeLayer: function (writePluginFiles, dest, loadList) {
-				var fs = require.nodeRequire("fs"),
-					jsdom = require.nodeRequire("jsdom").jsdom;
-
-				var document = jsdom("<html></html>").parentWindow.document;
-				var sprite = createSprite(document);
+			writeLayer: function (writePluginFiles, dest, loaded) {
+				function tryRequire(paths) {
+					var module;
+					var path = paths.shift();
+					if (path) {
+						try {
+							// This is a node-require so it is synchronous.
+							module = require.nodeRequire(path);
+						} catch (e) {
+							if (e.code === "MODULE_NOT_FOUND") {
+								return tryRequire(paths);
+							} else {
+								throw e;
+							}
+						}
+					}
+					return module;
+				}
 
-				loadList.forEach(function (path) {
-					var filename = getFilename(path),
-						svgText = fs.readFileSync(require.toUrl(path), "utf8"),
-						graphic = extractGraphic(document, svgText, filename),
-						symbol = createSymbol(document, graphic.id, graphic.element, graphic.viewBox);
-					sprite.appendChild(symbol);
-				});
+				var fs = require.nodeRequire("fs"),
+					jsDomPath = require.getNodePath(require.toUrl(module.id).replace(/[^\/]*$/, "node_modules/jsdom")),
+					jsDomModule = tryRequire([jsDomPath, "jsdom"]);
+
+				var path, url, svgText;
+				if (!jsDomModule) {
+					console.log(">> WARNING: Node module jsdom not found. Skipping SVG bundling. If you" +
+						" want SVG bundling run 'npm install jsdom' in your console.");
+					for (path in loaded) {
+						url = require.toUrl(path);
+						svgText = fs.readFileSync(url, "utf8");
+						writePluginFiles(url, svgText);
+					}
+					return false;
+				} else {
+					var jsdom = jsDomModule.jsdom,
+						document = jsdom("<html></html>"),
+						sprite = createSprite(document);
+
+					for (path in loaded) {
+						url = require.toUrl(path);
+						svgText = fs.readFileSync(url, "utf8");
+						var symbol = extractGraphicAsSymbol(document, svgText);
+						sprite.appendChild(symbol);
+						loaded[path] = symbol.getAttribute("id");
+					}
 
-				writePluginFiles(dest, sprite.outerHTML);
+					writePluginFiles(dest, sprite.outerHTML);
+					return true;
+				}
 			}
 		};
 
-
 		loadSVG.writeFile = function (pluginName, resource, require, write) {
 			writePluginFiles = write;
 		};
 
-		loadSVG.addModules = function (pluginName, resource, addModules) {
-			addModules(["requirejs-text/text"]);
-		};
-
 		loadSVG.onLayerEnd = function (write, layer) {
 			if (layer.name && layer.path) {
 				var dest = layer.path.replace(/^(.*\/)?(.*).js$/, "$1/$2.svg"),
 					destMid = layer.name + ".svg";
 
-				var loadList = Object.keys(cache);
-
 				// Write layer file and config
-				buildFunctions.writeLayer(writePluginFiles, dest, loadList);
-				buildFunctions.writeConfig(write, module.id, destMid, loadList);
+				var success = buildFunctions.writeLayer(writePluginFiles, dest, loaded);
+				success && buildFunctions.writeConfig(write, module.id, destMid, loaded);
 
 				// Reset cache
-				cache = {};
+				loaded = {};
 			}
 		};
 
 	}
 
 	return loadSVG;
 
-
-	// takes a path and returns the filename
-	function getFilename(filepath) {
-		return filepath.replace(/.*\/(.*)\.svg$/, "$1");
-	}
-
 	// makes a symbol out of an svg graphic
-	function extractGraphic(document, svgText, filename) {
+	function extractGraphicAsSymbol(document, svgText) {
 		var div = document.createElement("div");
 		div.innerHTML = svgText;
 		var element = div.querySelector("svg"),
-			id = element.getAttribute("id") || filename,
-			viewBox = element.getAttribute("viewbox") || element.getAttribute("viewBox") || "";
-		return {
-			id: id,
-			viewBox: viewBox,
-			element: element
-		};
+			id = element.getAttribute("id"),
+			viewBox = element.getAttribute("viewbox") || element.getAttribute("viewBox"),
+			symbol = createSymbol(document, id, element, viewBox);
+		return symbol;
 	}
 
 	// makes symbol from svg element
 	function createSymbol(document, id, element, viewBox) {
 		var symbol = document.createElementNS("http://www.w3.org/2000/svg", "symbol");
-		symbol.setAttribute("id", id);
 		while (element.firstChild) {
 			symbol.appendChild(element.firstChild);
 		}
-		viewBox && symbol.setAttribute("viewBox", viewBox);
+		typeof id === "string" && symbol.setAttribute("id", id);
+		typeof viewBox === "string" && symbol.setAttribute("viewBox", viewBox);
 		return symbol;
 	}