Skip to content
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

mix works inconsistently #86

Open
iamstarkov opened this issue Sep 3, 2014 · 21 comments
Open

mix works inconsistently #86

iamstarkov opened this issue Sep 3, 2014 · 21 comments

Comments

@iamstarkov
Copy link

mix mix classes and not tags, attrs and other mixes and it's a consistency problem.

tags

I want to use link block as mixin and not to hack it every time, so I want to write this way, but it's not working:

{
    block: 'media',
    elem: 'link',
    mix: [{ block: 'link', url: 'http://github.com/' }],
    content: [
        {
            elem: 'icon',
            url: 'duck.png'
        },
        {
            elem: 'title',
            content: 'comment'
        }
    ]
}

So I have to write this ugly code:

{
    block: 'link',
    mix: [{ block: 'media', elem: 'link' }],
    url: 'http://github.com/',
    content: [
        {
            block: 'media', elem: 'icon',
            url: 'duck.png'
        },
        {
            block: 'media', elem: 'title',
            content: 'comment'
        },
    ]
}

attrs

Then, I want to use one block for analytics tracking, and it have onclick attrs and goal option. I want to mix it and expect to see onclick attr on parent block like this way:

i-track.bh.js:

bh.match('i-track', function (ctx, json) {
    ctx.attr('onclick', 'track("' + json.goal + '")');
});

and use it like this:

{   
    block: 'link',
    mix: [{ block: 'i-track', goal: 'clck_goal' }],
    url: 'http://github.com/',
    content: 'Tracking link'
}

But it's not working too.

other mixes

I want to use custom font on my site. So I have one block i-face with mods for each custom font, but in reality only one font is being used in project, so it's reasonable to create shortcut block textbook, which have it's own mix with concrete textbook font.

In other words, I don't want to write mix: [{ block: 'i-font', mods: { face: 'textbook' }}] every time, because it's hard to remember and want to short it to this: mix: [{ block: 'textbook' }], where textbook block have such bh file:

bh.match('textbook', function (ctx, json) {
    ctx.mix([{ block: 'i-font', mods: { face: 'textbook' }}]);
});

And finally it's not working at all.

@iamstarkov iamstarkov changed the title mix works incorectly mix works inconsistently Sep 3, 2014
@iamstarkov
Copy link
Author

polite ping

@mishanga
Copy link
Member

mishanga commented Sep 5, 2014

It's not about consistency, it's because of ambiguity and conflicts.
Here is an example:

bh.match('button', function(ctx) {
    ctx.tag('span');
}).match('link', function(ctx) {
    ctx.tag('a');
}).apply({ block: 'button', mix: { block: 'link' } });

What result it should return?

@mishanga
Copy link
Member

mishanga commented Sep 5, 2014

//cc @veged

@iamstarkov
Copy link
Author

I think it should work as simple as CSS, last matcher should have more priority, then previous ones.

@mishanga
Copy link
Member

mishanga commented Sep 5, 2014

Лень по-буржуйски писать ;)
А как быть с content, например? А если в шаблоне link написано return { elem: 'wrap' }? Какой html будет на выходе?
И как тогда примиксовать блок, к которому ты не хочешь применять шаблоны (как раз чтобы враппер не включился, например)?

@veged
Copy link
Member

veged commented Sep 5, 2014

аналогичная проблема с исполнением миксов есть и в bemhtml — с теоретической точки зрения там всё решаемо, можно договориться про приоритеты и переопределения, так чтобы content перезатирал, а атрибуты миксовались — это будет покрывать все реальные кейсы использования

@mishanga
Copy link
Member

mishanga commented Sep 5, 2014

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

Сейчас mix хорош тем, что для него не исполняются шаблоны, ты просто в удобном bemjson-формате описываешь, какие классы и параметры добавить на текущую ноду. А если начать делать шаблонизацию миксов, мозг начнёт плавиться, все будут ляпать ctx.cls().

Кроме того, может заметно упать скорость шаблонизации.
И непонятно, как оставить возможность миксовать блоки без применения к ним шаблонов.

Прямо сейчас в любом месте можно явно сделать processBemJson на нужный микс и сделать extend. Не вижу смысла и необходимости делать это везде.

UPD: возможно, имело бы добавить какое-то новое API типа deepMix или extend, специально для таких штук.

@mishanga
Copy link
Member

mishanga commented Sep 5, 2014

//cc @dfilatov @denchistyakov ?

@veged
Copy link
Member

veged commented Sep 5, 2014

про поломки не очень согласен — сейчас шаблоны в основном пишутся так, что у штук, которые подразумеваютя к миксованию нет ничего особо своего (т.к. это всё равно не будет щас работать)

про плавится мозг тоже странно — сейчас же с точки зрения CSS миксы себя ведут именно так, всё смешивается и ни у кого мозг не плавится

по хорошему, миксы это также как модификаторы — в CSS это уже сейчас так, а в шаблонах модификаторы и миксы ведут себя по разному

скорость да — это то, почему я написал "с теоретической точки зрения там всё решаемо", т.к. на практике есть ещё допрасходы

случаев, когда нужно хотеть миксовать блоки без применения шаблонов я не могу представить (разве что то, что сейчас как-то так работает и люди привыкли)

@mishanga
Copy link
Member

mishanga commented Sep 5, 2014

Отвечу по порядку.

  1. Про поломки: очень даже пишутся миксы, у которых есть шаблоны — https://github.com/bem/bem-components/blob/53b01d0/common.blocks/select/__button/select__button.bh.js#L10

  2. Мозг будет плавиться от попыток понять, как оно так получилось. CSS можно легко поинспектить, какое свойство от какого селектора пришло, а в шаблонизаторе такой возможности нет.

  3. Миксы изначально придумывались как возможность добросить классов и параметров, но не так хардкорно, как тупо cls. И миксы — это не модификаторы, это самостоятельные сущности, наличие которых блок может даже не заметить на себе.

  4. Про скорость понятно. Но я-то именно с практической точки зрения веду обсуждение ;)

  5. Дело не в привычке, а в достаточности. Миксы сейчас работают по принципу: простое должно быть просто, а сложное — возможно. Нужно сложное — сделай applyCtx/processBemJson.

@mishanga
Copy link
Member

mishanga commented Sep 5, 2014

Или вот еще пример: { block: 'logo', url: '/', mix: { block: 'link'} }
Должен ли в шаблоне для link быть доступен контекст блока logo, в частности поле url?

@veged
Copy link
Member

veged commented Sep 5, 2014

  1. я не вижу каких-то шаблонов на select__button как таковой — понятно, что их нет, т.к. они всё равно щас не будут работать при таком использовании через микс

  2. не думал, что проблема в дебаг тулзах — но этож не дебаггер, можно сделать так, чтобы была информация откуда что пришло

  3. миксы изначально придумывались в CSS и там они себя как раз ведут как и модификаторы — потом когда делали bemhtml это не сделали так же отчасти из-за производительности, отчасти потому что нехватало реальных примеров с шаблонами (сейчас на практике видно, что этому есть необходимость, см. примеры из тикета, пример про serp-item+колдунщик и т.п.)

  4. я примерно представляю, как это можно сделать условно не дорого в bemhtml — c bh сложнее, т.к. нет возможности раздельно применять моду

  5. то, что можно было бы варазить полными миксами в шаблонах сейчас никак не сделать через applyCtx/processBemJson

в целом я не горю желанием делать это — именно поэтому я только давно обдумываю, но ещё не реализовал — но, если наберётся некая критическая масса, то хорошо бы сделать

@iamstarkov
Copy link
Author

я согласен с частью про плохую обратную совместимость.

@iamstarkov
Copy link
Author

Прямо сейчас в любом месте можно явно сделать processBemJson на нужный микс и сделать extend. Не вижу смысла и необходимости делать это везде.

расскажите про это подробнее

@iamstarkov
Copy link
Author

UPD: возможно, имело бы добавить какое-то новое API типа deepMix или extend, специально для таких штук.

тоже об этом подумал на след, день после того, как увидел сообшение о том, что изначальная идея может всё сломать

mishanga added a commit that referenced this issue Sep 9, 2014
mishanga added a commit that referenced this issue Sep 11, 2014
@qfox
Copy link
Member

qfox commented Nov 25, 2014

Вообще, странно осозновать, что теги и прочее не миксуется. Т.е. получается, что у меня уже сломанное сознание.

Насколько я понимаю, то текущий mix это что-то вроде mixCls или mixCss. А речь тут идет про mix для всего. Я не верю, что оно не сломает шаблоны bem-components, но там все покрыто тестами и от этого не очень страшно.

В общем, нужны спеки, сделать — вопрос техники.

@mishanga
Copy link
Member

Проблема в том, что не всё можно миксовать. Например, классы, можно. А вот теги — уже нет. Придется выбирать один тег из нескольких, возникает проблема выбора: какой тег важнее, тег базового блока или тег миксуемого?
То же самое с контентом, как его миксовать? Выбирать один из двух? Тогда какой? Или оставлять оба, но тогда в каком порядке?
Проблема-то как раз в том, чтобы придумать в этом месте стройную концепцию. Пока что ничего хорошего не получилось.

@qfox
Copy link
Member

qfox commented Nov 25, 2014

@mishanga какая же это проблема, если матчеры выполняются не параллельно, а подряд? соотв., логика та же самая, что как и везде: если стоит и без форса — пропускаем, если с форсом или не выставлен — ставим.

А вот контент миксовать — это интересно. Надо ли его миксовать?

Лишь бы не получилось так, что из-за непоняток с контентом мы теряли бы возможность миксовать все остальное.

@qfox
Copy link
Member

qfox commented Nov 25, 2014

На мой взгляд, вот так:

bh.apply({block: 'a', mix: [{block: 'b'}, {block: 'c'}]});

Приоритет одиночных параметров, типа тегов, у a (базового блока), далее b, и c (в порядке передачи).
a→b→c

Явная передача тега в миксах вообще не должна работать, а у базового блока имеет наивысший приоритет. Как всегда.

Переопределение тега с форсом — ломает приоритеты, и переопределяет тег всегда, потому что не учитывается проставлен ли параметр с форсом или обычным образом — но так везде работает и хочется верить, что используется этот флаг не часто.

Для контента сложнее, потому что там будут дополнительные проходы. Опять же, если возвращать контент из матчера с return — тоже отдельная песня. Оно же приостанавливает работу матчеров, так?

@mishanga
Copy link
Member

return не приостанавливает работу матчеров; return вставляет новый узел в дерево на место старого и заново запускает все матчеры над ним.

@qfox
Copy link
Member

qfox commented Dec 31, 2014

@mishanga простите за косноязычность и смуту. все верно.

content можно либо склеивать, либо по аналогии с остальными полями. Если склеивать — то в обычном порядке. Но реальных use-кейсов я для этого не знаю, но не потому, что их нет, а потому, что не сталкивался.

Хочу только добавить, что не так давно решалась задача с declMix для i-bem. И задача та по смыслу очень похожа на эту. bem/bem-bl#255

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants