Merge pull request #91 from patrick-steele-idem/issue-73

Fixes #73 - Prevent same taglib from being loaded multiple times

compiler/taglibs/Taglib/index.js

@@ -18,9 +18,9 @@
 var forEachEntry = require('raptor-util').forEachEntry;
 var ok = require('assert').ok;
 
-function Taglib(id) {
-    ok(id, '"id" expected');
-    this.id = id;
+function Taglib(path) {
+    ok(path, '"path" expected');
+    this.path = path;
     this.dirname = null;
     this.tags = {};
     this.textTransformers = [];

compiler/taglibs/taglib-loader/loader-taglib.js

@@ -33,7 +33,7 @@ function handleTag(taglibHandlers, tagName, path) {
 
         tagDirname = nodePath.dirname(path);
         if (!exists(path)) {
-            throw new Error('Tag at path "' + path + '" does not exist. Taglib: ' + taglib.id);
+            throw new Error('Tag at path "' + path + '" does not exist. Taglib: ' + taglib.path);
         }
 
         try {
@@ -44,7 +44,7 @@ function handleTag(taglibHandlers, tagName, path) {
     } else {
         tagDirname = dirname; // Tag is in the same taglib file
         tagObject = path;
-        path = '<' + tagName + '> tag in ' + taglib.id;
+        path = '<' + tagName + '> tag in ' + taglib.path;
     }
 
 
@@ -207,6 +207,20 @@ TaglibHandlers.prototype = {
         ok(transformer.path, '"path" is required for transformer');
 
         taglib.addTextTransformer(transformer);
+    },
+
+    /**
+     * Allows an ID to be explicitly assigned to a taglib.
+     * The taglib ID is used to prevent the same taglib  (even if different versions)
+     * from being loaded multiple times.
+     *
+     * NOTE: Introduced as part of fix for #73
+     *
+     * @param  {String} value The taglib ID
+     */
+    taglibId: function(value) {
+        var taglib = this.taglib;
+        taglib.id = value;
     }
 };
 
@@ -242,6 +256,32 @@ exports.loadTaglib = function(path) {
 
     propertyHandlers(taglibProps, taglibHandlers, path);
 
-    taglib.id = taglib.path = path;
+    taglib.path = path;
+
+    if (!taglib.id) {
+        // Fixes #73
+        // See if there is a package.json in the same directory as the taglib file.
+        // If so, and if that package.json file has a "name" property then we will
+        // use the the name as the "taglib ID". The taglib ID is used to uniquely
+        // identity a taglib (ignoring version) and it is used to prevent the same
+        // taglib from being loaded multiple times.
+        //
+        // Using the file path as the taglib ID doesn't work so well since we might find
+        // the same taglib multiple times in the Node.js module search path with
+        // different paths.
+        var dirname = nodePath.dirname(path);
+        var packageJsonPath = nodePath.join(dirname, 'package.json');
+
+
+        try {
+            var pkg = require(packageJsonPath);
+            taglib.id = pkg.name;
+        } catch(e) {}
+
+        if (!taglib.id) {
+            taglib.id = path;
+        }
+    }
+
     return taglib;
 };

compiler/taglibs/taglib-lookup.js

@@ -20,8 +20,12 @@ function buildLookup(dirname) {
 	var lookup = lookupCache[lookupCacheKey];
 	if (lookup === undefined) {
 		lookup = new TaglibLookup();
-
-        for (var i=taglibs.length-1; i>=0; i--) {
+		// The taglibs "closer" to the template will be earlier in the list
+		// and the taglibs "farther" from the template will be later. We
+		// want closer taglibs to take precedence (especially when de-duping)
+		// so we loop from beginning to end. We used to loop from the end
+		// to the beginning, but that appears to have been a mistake.
+        for (var i=0; i<taglibs.length; i++) {
 			var taglib = taglibs[i];
 			lookup.addTaglib(taglib);
 

taglibs/caching/marko-taglib.json

@@ -1,4 +1,5 @@
 {
+    "taglib-id": "marko-caching",
     "tags": {
         "cached-fragment": {
             "renderer": "./cached-fragment-tag",

taglibs/core/marko-taglib.json

@@ -1,4 +1,5 @@
 {
+    "taglib-id": "marko-core",
     "tags": {
         "c-template": {
             "attributes": {

taglibs/html/marko-taglib.json

@@ -1,4 +1,5 @@
 {
+    "taglib-id": "marko-html",
     "tags": {
         "html": {
             "attributes": {

test/fixtures/taglib-duplicate/foo-renderer.js

@@ -0,0 +1,3 @@
+exports.render = function(input, out) {
+
+};

test/fixtures/taglib-duplicate/marko-taglib.json

@@ -0,0 +1,6 @@
+{
+    "taglib-id": "taglib-duplicate",
+    "<duplicate-foo>": {
+        "renderer": "./foo-renderer.js"
+    }
+}

test/fixtures/taglib-duplicate/taglib-duplicate/bar-renderer.js

@@ -0,0 +1,3 @@
+exports.render = function(input, out) {
+
+};

test/fixtures/taglib-duplicate/taglib-duplicate/marko-taglib.json

@@ -0,0 +1,6 @@
+{
+    "taglib-id": "taglib-duplicate",
+    "<duplicate-bar>": {
+        "renderer": "./bar-renderer.js"
+    }
+}

test/taglib-lookup-test.js

@@ -182,4 +182,20 @@ describe('taglib-lookup' , function() {
         expect(transformers[2].path.indexOf('html-tag-transformer')).to.not.equal(-1);
     });
 
+    it('should de-duplicate taglibs', function() {
+        var taglibLookup = require('../compiler').taglibs.lookup;
+        var lookup = taglibLookup.buildLookup(nodePath.join(__dirname, 'fixtures/taglib-duplicate/taglib-duplicate'));
+
+        // The "duplicate-bar" tag was declared in the lower
+        // taglib so it should have been found since the taglib
+        // should not have been de-duped.
+        var barTag = lookup.getTag('duplicate-bar');
+        expect(barTag != null).to.equal(true);
+
+        // The "duplicate-foo" tag was declared in the higher
+        // up taglib so it should have been discarded
+        var fooTag = lookup.getTag('duplicate-foo');
+        expect(fooTag == null).to.equal(true);
+    });
+
 });