bem / bem-xjst

bem-xjst (eXtensible JavaScript Templates): declarative template engine for the browser and server
https://bem.github.io/bem-xjst
Other
116 stars 48 forks source link

Semantic: change behaviour of `mix`, `js` and `attrs` modes #261

Closed miripiruni closed 8 years ago

miripiruni commented 8 years ago

.mix()('b') should set b as mix. Last call of mix() set new value which is not extended previous. .addMix()('newMix') should extend value of previous mix value.

The same for js() and attrs().

Also, I think we need addContent() mode to avoid applyNext() copypaste in chain of the same mode templates.

BTW, see https://gist.github.com/miripiruni/7561cc8ad56d0d4d5184

tadatuta commented 8 years ago

Please refer to old issues https://github.com/bem/bem-xjst/issues/242 (based on https://github.com/bem/bem-core/issues/820)

tadatuta commented 8 years ago

Discussed there's an issue with possibility to dismiss addMix() declared on previous level.

qfox commented 8 years ago

I think we need addContent() mode to avoid applyNext()

I don't see what we get with avoiding applyNext() calls. addContent, addMix, addAnythingElse won't make API easier. Compare:

block('a').addMix()(() => {elem: 'e'});
// vs
block('a').mix()(() => applyNext().concat({elem: 'e'}));

// and
block('a').addContent()(() => {elem: 'e', content: 'qwe'});
// vs
block('a').content()(() => applyNext().concat({elem: 'e', content: 'qwe'}));

Worth?

miripiruni commented 8 years ago

@zxqfox Incorrect example. Look: block('a').mix()(() => applyNext().concat({elem: 'e'})); will fall if applyNext() return not an Array. First of all you need to check type.

With addMix/addAttrs/etc we’ll get expressiveness and brevity. The correct compare:

block('page').addMix()(() => 'mixed');
// vs
block('page').mix()(function() {
    var mixes = applyNext();
    if (!Array.isArray(mixes)) mixes = [ mixes ];
    return mixes.concat('mixed');
});
qfox commented 8 years ago

Ok, but let's do mix an array. It's much easier and cleaner, isn't it?

miripiruni commented 8 years ago

@zxqfox OK, but where you would like to change it?

miripiruni commented 8 years ago

@zxqfox я процитирую себя самого:

если мы хотим сформировать выход (часть выходного дерева) то нужно использовать режимы. То есть вся «запись» должна происходить в них. А чтение, с целью проверить что-либо, нужно производить в нормализованных значениях.

proof

qfox commented 8 years ago

Ну так давай нормализовывать this.mix, равно как мы это делаем с this.mods? Аналогично this.js, this.attrs

miripiruni commented 8 years ago

@zxqfox the only reason why we change this.mods/this.elemMods now is that we don’t have any other way to do it in current version. We need change it. We need more explicit way to make output not by side effects but in «clear declarative functions» way.

Compare:

block('a').def()(function() {
    this.mix.push('mixed');
    return applyNext();
});
// vs
block('a').addMix()('mixed'); // Much more explicitly than def() with side effect.
qfox commented 8 years ago

Т.е. в будущем мы не будем нормализовывать mods, elemMods? Я не понял аргументации ПРОТИВ нормализации mix, js, attrs.

miripiruni commented 8 years ago

@zxqfox я против записи в this.*, но не против нормализации.

qfox commented 8 years ago

@miripiruni Я против записи в ctx ;-)

miripiruni commented 8 years ago

https://github.com/bem/bem-xjst/releases/tag/v8.0.0