Match: check mods/elemMods subpredicates after it’s converting to strings #266

miripiruni · 2016-05-12T12:21:27Z

Benchmarks result within the error.

qfox · 2016-05-12T12:40:15Z

lib/bemxjst/match.js

+ return false;
+ }
+
+ return val[this.keys[i]].toString() === this.value.toString();


Check please for passing null here.

@zxqfox Oh, yeah! Thank you! 👍

qfox · 2016-05-12T12:55:16Z

test/modes-mod-test.js

+ test(function() {
+ block('b').mod('m', 'true').tag()('span');
+ },
+ { block: 'b', mods: { m: true } },


@miripiruni Commented the wrong line, sorry. I'm about this one.

@zxqfox updated!

What you think about making true value as a '*'?
true means modifier exists. If it doesn't care about modifier value then semantically it should match to any value, isn't it?

@zxqfox First of all, it’s out of scope of this PR =)
The second: change the semantic of true modVal is a big major change. For what reason we should do it?

It's not, because you decide this is major.

true means modifier exists. If it doesn't care about modifier value then semantically it should match to any value, isn't it?

That's why. Tell me why not now.

Прошу меня извинить за то, что сразу не предоставил аргументов.

Потому что true обозначает наличие модификатора в любом значении.

Сейчас это не так. Пользователи когда пишут block('b').mod('m', true) имеют ввиду не вайлдкард, а конкретное значение — true. В БЭМ modVal равный true никогда не означал вайлдкард, поэтому я не понимаю отчего ты предлагаешь это сделать.

Я понимаю, что это не так. И это не стыкуется с их семантикой. Забудь слово вайлдкард, он здесь не подходит, я привел его для примера обозначения наличия значения (любого).
В БЭМ modVal равный true не существовал до внедрения булевых модификаторов (год 2013-14). Но до сих пор они никак не описаны в методологии, так что спорить надо уходить в другое место.

Извини, видимо, я тороплю события.

Кстати, пользователи пишут .mod('m', true) по другой причине — просто они не могут писать .mod('m').

про булевые («простые») модификаторы всё придумано и описано в i-bem.js — https://ru.bem.info/technology/i-bem/v2/i-bem-js-mods/#Модификаторы — нужно просто сделать аналогично

@veged создал #274

miripiruni · 2016-05-19T10:38:36Z

Review needed.

cc @zxqfox @tadatuta @veged

veged · 2016-05-19T13:23:55Z

lib/bemxjst/match.js

+
+ if (
+ keyVal === undefined ||
+ keyVal === null ||


why you need to check null?

Because null.toString() will prevent execution. But it should not.

why not use String(...) then?

I wish I could say that it is slowly but I can’t. jsperf.com is down :-\

should be the same, actually

@veged updated.

veged · 2016-05-21T22:44:38Z

diff LGTM
what about benchmarks?

miripiruni · 2016-05-22T15:51:40Z

@veged as I wrote in PR’s description: Benchmarks result within the error.

veged · 2016-05-23T22:28:16Z

which error? o_0

miripiruni · 2016-05-23T22:42:37Z

@veged «в пределах погрешности» :)

miripiruni · 2016-06-17T10:46:36Z

@veged Я проверил как устроен MatchNested. Он используется только для elemMods и mods. И будет использован в будущем только для тех подпредикатов, ключ которых явится массивом из двух элементов: 1) имя режима 2) значение ключа модификатора или аналогичное значение ключа этого подпредиката.

veged · 2016-06-17T11:57:58Z

стоит всё-таки сделать как-то, чтобы одно из приведений к строке случалось только один раз

…ings

miripiruni · 2016-06-20T11:59:36Z

Updated. cc @veged

miripiruni · 2016-06-20T12:05:36Z

lib/bemxjst/tree.js

@@ -340,11 +340,11 @@ Tree.prototype.mode = function mode(name) {
 };

 Tree.prototype.mod = function mod(name, value) {
- return this.match(new PropertyMatch([ 'mods', name ], value));
+ return this.match(new PropertyMatch([ 'mods', name ], String(value)));


JFYI: при построении дерева entities эта функция добавит к шаблону подпредикат mod уже в виде { key: [ 'mods', 'modName' ], value: 'modVal' }

veged · 2016-06-20T12:32:14Z

now it's perfect 👍

miripiruni added semver: major status: review type: enhancement scope: semantic labels May 12, 2016

qfox reviewed May 12, 2016
View reviewed changes

miripiruni force-pushed the issue-220__mod-types branch 2 times, most recently from 0f02d84 to 8ff4da7 Compare May 12, 2016 12:52

qfox reviewed May 12, 2016
View reviewed changes

miripiruni force-pushed the issue-220__mod-types branch from 8ff4da7 to 1ed16bc Compare May 12, 2016 13:00

veged reviewed May 19, 2016
View reviewed changes

miripiruni added this to the 7.0.0 milestone May 20, 2016

miripiruni force-pushed the issue-220__mod-types branch from 1ed16bc to f446ac3 Compare May 20, 2016 10:52

Match: check mods/elemMods subpredicates after it’s converting to str…

ab83060

…ings

miripiruni force-pushed the issue-220__mod-types branch from f446ac3 to ab83060 Compare June 20, 2016 10:59

miripiruni reviewed Jun 20, 2016
View reviewed changes

miripiruni merged commit 079a5c8 into master Jun 29, 2016

miripiruni deleted the issue-220__mod-types branch June 29, 2016 17:08

miripiruni removed the status: review label Jun 29, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Match: check mods/elemMods subpredicates after it’s converting to strings #266

Match: check mods/elemMods subpredicates after it’s converting to strings #266

miripiruni commented May 12, 2016 •

edited

Loading

qfox May 12, 2016

miripiruni May 12, 2016

qfox May 12, 2016

miripiruni May 12, 2016

qfox May 12, 2016

miripiruni May 12, 2016

qfox May 12, 2016 •

edited

Loading

miripiruni May 19, 2016 •

edited

Loading

qfox May 19, 2016

qfox May 19, 2016

veged May 19, 2016

miripiruni May 19, 2016

miripiruni commented May 19, 2016

veged May 19, 2016

qfox May 19, 2016

veged May 19, 2016

miripiruni May 19, 2016

veged May 19, 2016

miripiruni May 20, 2016

veged commented May 21, 2016 •

edited

Loading

miripiruni commented May 22, 2016

veged commented May 23, 2016

miripiruni commented May 23, 2016

miripiruni commented Jun 17, 2016 •

edited

Loading

veged commented Jun 17, 2016

miripiruni commented Jun 20, 2016

miripiruni Jun 20, 2016

veged commented Jun 20, 2016

Match: check mods/elemMods subpredicates after it’s converting to strings #266

Match: check mods/elemMods subpredicates after it’s converting to strings #266

Conversation

miripiruni commented May 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qfox May 12, 2016 • edited Loading

Choose a reason for hiding this comment

miripiruni May 19, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

miripiruni commented May 19, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

veged commented May 21, 2016 • edited Loading

miripiruni commented May 22, 2016

veged commented May 23, 2016

miripiruni commented May 23, 2016

miripiruni commented Jun 17, 2016 • edited Loading

veged commented Jun 17, 2016

miripiruni commented Jun 20, 2016

Choose a reason for hiding this comment

veged commented Jun 20, 2016

miripiruni commented May 12, 2016 •

edited

Loading

qfox May 12, 2016 •

edited

Loading

miripiruni May 19, 2016 •

edited

Loading

veged commented May 21, 2016 •

edited

Loading

miripiruni commented Jun 17, 2016 •

edited

Loading