bem / bh

BH template engine
http://bem.github.io/bh/
MIT License
68 stars 31 forks source link

Do not ignore `undefined` value in BEMJSON in attrs #42

Closed sipayRT closed 10 years ago

sipayRT commented 10 years ago

For example, we have BEMJSON

{
    block : 'bla',
    attrs : { a : undefined },
    content : 'bla'
}

and BH-template

bh.match('bla', function(ctx) {
    ctx.attrs({
        a : 0,
        b : 'some'
    });
});

and HTML

<div class="bla" a="0" b="some">bla</div>

So param from BEMJSON was ignored. But we expect that parameter will not be added

mishanga commented 10 years ago

Undefined is undefined. You should not use it to prevent default values. Use null instead.

tadatuta commented 10 years ago

We used to use undefined in BEMJSON to drop values set in templates and it actually works fine in BEMHTML. Don't you think we should support the same in BH?

mishanga commented 10 years ago

No, I don't think so. You definitely should use null to drop values. Can you show me some examples of using undefined?

tadatuta commented 10 years ago

E.g.

attrs()(function() {
    return {
        someAttr: this.ctx.bla // where this.ctx.bla can be undefined
    };
});
mishanga commented 10 years ago

@tadatuta если в this.ctx.bla лежит undefined, это равносильно пустому объекту. Если в bemhtml не так, то это надо исправить.

tadatuta commented 10 years ago

@mishanga в i-bem.bemhtml для мержа результата выполнения моды attrs с полем attrs из bemjson используется extend, где проверяется hasOwnProperty, так что если в bemjson окажется { attrs: { f1: undefined }, а в шаблоне attrs()({ f1: 'bla' }), то в результирующий html f1 не попадет. ну и для меня это интуитивно ожидаемое поведение, если честно.

mishanga commented 10 years ago

@tadatuta получается, что ты кладешь в поле какое-то значение, которое переопределяет шаблон, а в конечный html не попадает. Это баг и сатанизм, на мой взгляд. @veged @dfilatov коллеги, ваше мнение?

veged commented 10 years ago

у нас специально было сделано, что значение undefined в attrs означает отсутствие атрибута — поэтому, кажется логичным желание, передавая undefined перетереть ранее установленный атрибут (хоть мы и не задумывали это)

но я не понял, почему оно не работает при extendhasOwnProperty этож не typeof ... !== 'undefined'

и отдельно история про то, что от hasOwnProperty надо везде отказаться

tadatuta commented 10 years ago

@veged оно как раз работает при extend, но в BH extend в этом месте не используется, отсюда и разное поведение BEMHTML vs BH.

mishanga commented 10 years ago

@veged чтобы удовлетворить логичное желание перетереть атрибут, можно передать null.

mishanga commented 10 years ago

Поговорили с @veged, решили привести логику к естественной для JS, то есть учитывать поля, если они явно заданы, пусть даже и в значении undefined. То есть сделать как в bemhtml. Это надо не только в методе attrs исправить, я сделаю в ближайшие дни.