bem/bem-xjst

wrap() mutates bemjson which can lead to error for second run for same input data

tadatuta opened this issue · 5 comments

Input code or something about issue background

const bemhtml = require('bem-xjst').bemhtml;
const tmpl = bemhtml.compile(function() {
    block('b1').wrap()(node => ({
        block: 'b2',
        content: node.ctx
    }));
});

const bemjson = { block: 'b1' };

const firstTime = tmpl.apply(bemjson);
const secondTime = tmpl.apply(bemjson);

console.log('firstTime:', firstTime);
console.log('secondTime:', secondTime);

Important!

  1. bemjson should be passed by reference
  2. the same instance of compiled template should be used

Expected Behavior

firstTime: <div class="b2"><div class="b1"></div></div>
secondTime: <div class="b2"><div class="b1"></div></div>

Actual Behavior

firstTime: <div class="b2"><div class="b1"></div></div>
secondTime: <div class="b1"></div>

Your Environment

Any version of bem-xjst with wrap() support.

// cc @vithar

The same issue with extend():

$ cat extend.js
const bemhtml = require('./').bemhtml;
const tmpl = bemhtml.compile(function() {
    block('b1').extend()({ 'ctx.content': 42 });
});

const bemjson = { block: 'b1' };

const firstTime = tmpl.apply(bemjson);
console.log('firstTime:', firstTime);
console.log(bemjson);

const secondTime = tmpl.apply(bemjson);
console.log('secondTime:', secondTime);
console.log(bemjson);

$ node extend.js
firstTime: <div class="b1">42</div>
{ block: 'b1', content: undefined }
secondTime: <div class="b1"></div>
{ block: 'b1', content: undefined }

@tadatuta I see only one solution: copy bemjson to a new object in first line of bemhtml.apply(). Is it OK?

qfox commented

Prob if we can't fix this, we can deny this.

Actual Behavior

firstTime: <div class="b2"><div class="b1"></div></div>
secondTime: Exception with xjst can't go in to the same river twice

Probably we can generate some id for the xjst passes on the apply call and use it for _wrap flag?
Cuz cloning objects is a thing we don't want to do because of speed.

Or create an ctx with a Set of used wrappers and extends for a node (object) in a bemjson tree (invert the structure) to prevent mutations of bemjson at all.

qfox commented

Guess, right now we have the same thing with deeper placed objects:

  it('should work with several apply() calls', function() {
    var bemjson = [ { tag: 'span', content: { block: 'b1' } } ];
    var expected = '<span><div class="b1">42</div></span>';
    var tmpl = fixtures.compile(function() {
      block('b1').extend()({ 'ctx.content': 42 });
    });
 
    assert.equal(
      tmpl.apply(bemjson),
      expected,
      'first apply() call returns not expected value'
    );
 
    assert.equal(
      tmpl.apply(bemjson),
      expected,
      'second apply() call returns not expected value'
    );
  });

Any news here?