Permalink
Browse files

Fixed merge util

  • Loading branch information...
1 parent 4ac9f02 commit 9108ecc5252926ae041679a8a9a0126253c7eac0 @tj tj committed May 21, 2012
Showing with 36 additions and 25 deletions.
  1. +28 −17 lib/runtime.js
  2. +8 −8 test/unit.js
View
@@ -33,31 +33,30 @@ if (!Object.keys) {
/**
* Merge two attribute objects giving precedence
- * to values in object `b`.
+ * to values in object `b`. Classes are special-cased
+ * allowing for arrays and merging/joining appropriately
+ * resulting in a string.
*
* @param {Object} a
* @param {Object} b
- * @param {Boolean} `true` if these objects are `escaped` objects, not attributes
* @return {Object} a
* @api private
*/
-exports.merge = function(a, b, escaped) {
- // Special treatment for "class" attribute
- if (!escaped) {
- var val;
-
- if (val = a['class'] && Array.isArray(val)) {
- a['class'] = val.join(' ');
- }
-
- if (val = b['class']) {
- if (Array.isArray(val)) val = val.join(' ');
- if (a['class']) a['class'] += ' ' + val;
- else a['class'] = val;
- }
+exports.merge = function merge(a, b) {
+ var ac = a.class;
+ var bc = b.class;
+
+ if (ac || bc) {
+ ac = ac || [];
+ bc = bc || [];
+ if (!Array.isArray(ac)) ac = [ac];
+ if (!Array.isArray(bc)) bc = [bc];
+ ac = ac.filter(nulls);
+ bc = bc.filter(nulls);
+ a.class = ac.concat(bc).join(' ');
}
-
+
for (var key in b) {
if ('class' == key) continue;
a[key] = b[key];
@@ -67,6 +66,18 @@ exports.merge = function(a, b, escaped) {
};
/**
+ * Filter null `val`s.
+ *
+ * @param {Mixed} val
+ * @return {Mixed}
+ * @api private
+ */
+
+function nulls(val) {
+ return val != null;
+}
+
+/**
* Render the given attributes object.
*
* @param {Object} obj
View
@@ -8,22 +8,22 @@ describe('merge(a, b, escaped)', function(){
.should.eql({ foo: 'bar', bar: 'baz' });
merge({ class: [] }, {})
- .should.eql({ class: [] }); // broken
+ .should.eql({ class: '' });
merge({ class: [] }, { class: [] })
- .should.eql({ class: ' ' }); // ok but broken
+ .should.eql({ class: '' });
merge({ class: [] }, { class: ['foo'] })
- .should.eql({ class: ' foo' });
+ .should.eql({ class: 'foo' });
merge({ class: ['foo'] }, {})
- .should.eql({ class: ['foo'] }); // broken
+ .should.eql({ class: 'foo' });
merge({ class: ['foo'] }, { class: ['bar'] })
.should.eql({ class: 'foo bar' });
merge({ class: ['foo', 'raz'] }, { class: ['bar', 'baz'] })
- .should.eql({ class: 'foo,raz bar baz' }); // broken
+ .should.eql({ class: 'foo raz bar baz' });
merge({ class: 'foo' }, { class: 'bar' })
.should.eql({ class: 'foo bar' });
@@ -35,9 +35,9 @@ describe('merge(a, b, escaped)', function(){
.should.eql({ class: 'foo bar baz' });
merge({ class: ['foo', 'bar'] }, { class: 'baz' })
- .should.eql({ class: 'foo,bar baz' }); // broken
+ .should.eql({ class: 'foo bar baz' });
- merge({ class: ['foo', null, 'bar'] }, { class: [undefined, null, 1, 'baz'] })
- .should.eql({ class: 'foo,,bar 1 baz' }); // broken
+ merge({ class: ['foo', null, 'bar'] }, { class: [undefined, null, 0, 'baz'] })
+ .should.eql({ class: 'foo bar 0 baz' });
})
})

2 comments on commit 9108ecc

@chowey
Contributor
chowey commented on 9108ecc May 21, 2012

Wow awesome. Something like this needs to be done in the parser (#610).

@tj
Contributor
tj commented on 9108ecc May 21, 2012

yeah definitely, might have to wrap class expressions in classes(expr) or something, kinda slow for the other use-cases though. some lame regexp hack to see if it doesn't start with a quote maybe haha

Please sign in to comment.