Skip to content
This repository has been archived by the owner on Feb 4, 2018. It is now read-only.

We should not use bem-naming just for id getter #122

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

qfox
Copy link
Contributor

@qfox qfox commented May 18, 2017

fix: stop using bem-naming for generating id

cc @blond @tadatuta

@tadatuta
Copy link
Member

нужно тесты обновить, а по смыслу — like

@qfox
Copy link
Contributor Author

qfox commented May 19, 2017

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

Типа block−elem−mod−val, block−−mod−val, block−elem−−, block−elem−mod−, etc. (это непростой минус, хоть он и похож на простой, но он лучше, чем \x01, потому что будет проще читать дампы при необходимости).

id: e => [
    e.block,
    e.elem || '',
    e.mod ? e.mod.name : '',
    (e.mod || e.mod.val !== true) && e.mod.val || ''
].join('−'),
parseId: s => {
    const x = s.split(//g);
    return new BemEntityName({
        block: x[0],
        elem: x[1] || null,
        mod: x[2] && { name: x[2], val: x[3] || true }
    });
},

@Yeti-or
Copy link
Member

Yeti-or commented May 19, 2017

я не очень понимаю зачем тебе parseId
и также мне кажется лучше придержаться стандартного нейминга в данном месте, не?
Довод:
кто-то мог на это завязаться хотя он сам виноват в данном случае

@qfox
Copy link
Contributor Author

qfox commented May 22, 2017

Нравится мне иметь parseid как обратную операцию к id.

@qfox
Copy link
Contributor Author

qfox commented May 22, 2017

Тесты поправил. Вернул старое поведение

/ping

@@ -1,6 +1,4 @@
import test from 'ava';
import sinon from 'sinon';
import proxyquire from 'proxyquire';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

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

Successfully merging this pull request may close these issues.

None yet

3 participants