-
Notifications
You must be signed in to change notification settings - Fork 48
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
More tests for coverage #405
Conversation
isBEM = entity.block || entity.elem; | ||
else | ||
isBEM = ctx.bem; | ||
isBEM = entity.block || entity.elem; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of bem
in line 82 is result of bem()
mode execution. And bem()
returns this.ctx.bem
by default.
@@ -93,7 +90,7 @@ BEMHTML.prototype.render = function render(context, | |||
|
|||
var addJSInitClass = jsParams && ( | |||
this._elemJsInstances ? | |||
(entity.block || entity.elem) : | |||
entity.block : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of entity.block
is always available.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure it's useless case but it works now:
BEMHTML.apply({ elem: 'e1' }); // '<div class="__e1"></div>'
@@ -284,9 +281,6 @@ BEMHTML.prototype.renderMix = function renderMix(entity, | |||
context.elem = oldElem; | |||
context.block = oldBlock; | |||
|
|||
if (!nestedMix) | |||
continue; | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of check in line 278 already the same.
@@ -8,7 +8,7 @@ var ClassBuilder = require('./class-builder').ClassBuilder; | |||
var utils = require('./utils'); | |||
|
|||
function BEMXJST(options) { | |||
this.options = options || {}; | |||
this.options = options; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because of options
is always object.
// Reuse function if it already has right arguments | ||
if (typeof code === 'function' && code.length === args.length) | ||
return code; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this case is fantastic:
bemxjst.compile(function(block, elem, mod, addMods, content, …) { … })
Nobody wants to enumerate all modes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure. We can generate code with all modes ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nvm, you're right
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The rest looks good
// Reuse function if it already has right arguments | ||
if (typeof code === 'function' && code.length === args.length) | ||
return code; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure. We can generate code with all modes ;-)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍
@@ -93,7 +90,7 @@ BEMHTML.prototype.render = function render(context, | |||
|
|||
var addJSInitClass = jsParams && ( | |||
this._elemJsInstances ? | |||
(entity.block || entity.elem) : | |||
entity.block : |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure it's useless case but it works now:
BEMHTML.apply({ elem: 'e1' }); // '<div class="__e1"></div>'
@@ -146,5 +146,60 @@ describe('API compile', function() { | |||
template.apply({ block: 'b1' }); | |||
}); | |||
}); | |||
|
|||
it('should throw bem-xjst error on template with no block', function() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
without block
No description provided.