bem / bem-xjst

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

Introduce mods()({ modeName: 'modeVal' }) #107

Closed tadatuta closed 7 years ago

tadatuta commented 8 years ago

In base templates in bem-core, this.ctx.mods is always created.

If it's cheap in performance POV, I think it should be the same in bem-xjst@4 for backward compatibility.

mishanga commented 8 years ago

this.mods, а не this.ctx.mods

qfox commented 8 years ago

@mishanga Именно this.ctx.mods, this.mods и this.elemMods уже always.

@tadatuta This should be marked as suggestion, I guess. Not sure why this still not answered by maintainer ;-(

veged commented 8 years ago

linked with #111

indutny commented 8 years ago

Let's talk a bit about why I chose to not create them on every run. This is potentially going to slow down all templates down the way, because it will create a new hidden-class for each nested JS object in input JSON. Are we sure that we want this?

I could be convinced that it is fine, if benchmarks will tell so. But usually I try to avoid creating hidden class just for having a bit more flexibility in user code.

veged commented 8 years ago

it's not only for "flexibility" but for consistency with this.mods and this.elemMods

mishanga commented 8 years ago

this.ctx — это текущий узел в BEMJSON. Он должен приходить в шаблон as is, без модификаций. this — это контекст, его можно делать каким угодно. Я голосую против этого issue. Предлагаю закрыть по wont't fix.

veged commented 8 years ago

это выливается в то, что в коде нужно постоянно писать (this.mods || (this.mods = {}))['my-mod'] = 'my-val'

я не понимаю, в чём практическая польза соблюдения формальной строгости «as is без модификаций»?

veged commented 8 years ago

если этот тикет закрывать, нужно делать обратный для того, чтобы this.ctx.mods не создавались — сейчас неконсистентность между this.ctx.mods и this.ctx.elemMods

mishanga commented 8 years ago

Да, this.ctx.mods тоже создавать не надо. А пример выше про (this.mods || (this.mods = {})) показывает, что нужно сделать API для выставления значения модификатора в движке (как это сделано в BH — ctx.mod()) и последующим применением для этих вновь добавленных значений соответствующих шаблонов.

veged commented 8 years ago

@mishanga ты не ответил на вопрос: в чём практическая польза соблюдения формальной строгости «as is без модификаций»? можно сделать API, но уже есть простой и нативный способ this.ctx.mods.m = 'v'

mishanga commented 8 years ago

Практическая польза очень простая — не плодить неочевидное поведение. У нас есть "сырой" (raw) контекст и кастомный контекст. Это понятно и удобно. Не надо смешивать одно с другим. А для прямого присвоения модификатора можно писать this.ctx.mods = this.extend(this.mods, { a: 'b' }) в прямом или обратном порядке, получая возможность задавать дефолты в BEMJSON.

veged commented 8 years ago

кто и как может пострадать от такого не очевидного поведения? разве кто-то пишет шаблоны в расчёте на то, что this.ctx.mods будут undefined, если во входном BEMJSON нет этого поля?

this.ctx.mods = this.extend(this.mods, { a: 'b' }) это и длиннее писать и медленнее чем this.ctx.mods.a = 'b'

narqo commented 8 years ago

@veged @mishanga @tadatuta кажется в описание задачи некоторая подмена, что для чего делалось.

В bemhtml this.ctx.mods = {} делалось не для того, чтобы он «был всегда», а для того, что this.mods ссылался на тот же объект, что и this.ctx.mods. После этого, мы всем говорили (настоятельно), что в шаблонах трогать this.ctx.mods (и вообще this.ctx) не надо, что это входные параметры, которые должны быть «иммутабельны», что это был баг который мы исправили, и что всегда писать надо this.mods.a = 'b'. PR: https://github.com/bem/bem-core/pull/446 (как щас помню, мы это с @veged делали на рабочей встрече).

В bem-xjst что-то поменялось и теперь снова «пиши как хочешь — наверняка один из вариантов сработает»?

qfox commented 8 years ago

Кстати, Object.freeze(this.ctx.mods); в дебаг режиме может будем делать?

mishanga commented 8 years ago

в шаблонах трогать this.ctx.mods (и вообще this.ctx) не надо

@narqo я на этой встрече не был, но подход мне нравится. Если будет православный способ выставить модификатор в шаблоне, я обеими руками "за".

veged commented 8 years ago

@mishanga да, @narqo всё правильно пишет, я просто везде в коментах не правильно писал this.ctx.mods — нужно читать как this.mods

mishanga commented 8 years ago

Ну, значит мы договорились. Надо тогда переформулировать это issue и завести другое: про добавление моды "mods" для добавления модификаторов.

block('button')
    .match(function() { return this.isDisabled })
    .mods()({ disabled: true })
miripiruni commented 8 years ago

@mishanga :+1: