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 qfox force-pushed the qfox.minus-bem-naming branch from 99a4e7f to c7646b8 Compare May 22, 2017 03:32
@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.

3 participants