[wip] Add convert for v1 format #70

skad0 · 2016-10-03T14:57:21Z

tadatuta · 2016-10-03T16:23:25Z

lib/convert/v1.js

+module.exports = function (decl) {
+    Array.isArray(decl) || (decl = [decl]);
+
+    if (decl.length > 0) {


if (!decl.length) return [];

to avoid extra indentation

tadatuta · 2016-10-03T16:24:08Z

lib/convert/v1.js

+
+        decl.forEach(function (dep) {
+            const entity = dep.entity;
+            let item = { name: entity.block };


let -> const

tadatuta · 2016-10-04T00:13:23Z

lib/convert/v1.js

+    if (decl.length > 0) {
+        const res = [];
+
+        decl.forEach(function (dep) {


return decl.reduce(fn, []);

to get rid of res

tadatuta · 2016-10-04T00:38:28Z

lib/convert/v1.js

+            const entity = dep.entity;
+            let item = { name: entity.block };
+
+            if (entity.elem) {


never tested but looks like all this stuff can be rewritten like this:

Array.isArray(decl) || (decl = [decl]); return !decl.length ? [] : decl.reduce(function(acc, dep) { const entity = dep.entity; const item = { name: entity[entity.elem ? 'elem' : 'block'] }; entity.modName && (item.mods = [{ name: entity.modName }]); entity.modVal && (item.mods[0].vals = [{ name: entity.modVal }]); acc.push(entity.elem ? { name: entity.block, elems: [item] } : item); return acc; }, []);

Great, thank you.

qfox

как-то тестов маловато

qfox · 2016-10-04T11:53:54Z

test/convert/v1.test.js

+    const output = [
+        { name: 'block1' },
+        { name: 'block1', mods: [{ name: 'mod1', vals: [{ name: true }] }] },
+        { name: 'block1', mods: [{ name: 'mod1', vals: [{ name: 'val1' }] }] },


может склеивать такое сразу?

skad0 · 2016-10-10T19:06:44Z

up, review needed
@zxqfox @tadatuta @blond

skad0 · 2016-10-12T15:25:01Z

up
/cc @zxqfox @tadatuta @blond

qfox · 2016-10-12T18:23:37Z

test/convert/v1.test.js

+    const output = [
+        { name: 'block1' },
+        {
+            name: 'block1',


А разве 2 объекта должно быть?

Я по наитию ориентировался)

Надеялся, что кто-то про него что-то знает)

https://github.com/bem-sdk/bem-decl/blob/master/test/normalize/elems.test.js#L6-L20 Судя по этому тесту — первый объект лишний.

@blond Интересно, а как таким образом описывать элементы без блока и модификаторы без элемента и блока? Никак штоле?

qfox · 2016-10-12T18:27:34Z

test/convert/v1.test.js

+            name: 'block1',
+            elems: [
+                {
+                    name: 'elem1',


очень странно, что блок мы вынесли в отдельный объект, а элемент нет. Это реально такой хороший формат?

/cc @blond @tadatuta

@zxqfox не понял тебя. Формат реально не хороший :)

skad0 · 2016-10-17T14:46:49Z

up
@zxqfox @blond @tadatuta
@blond @blond @blond ^_^

tadatuta · 2016-12-04T02:25:53Z

нужно отребейзить
нужно исправить мессадж второго коммита (convertation -> conversion)

blond

Не хватает тестов для разных скоупов: например, несколько разных блоков.

blond · 2016-12-10T13:19:47Z

test/convert/v1.test.js

+    t.deepEqual(convert([], { format: 'v1' }), []);
+});
+
+test('must split elems of one block', t => {


split -> group?

blond · 2016-12-10T13:22:51Z

test/convert/v1.test.js

+            name: 'block1',
+            elems: [
+                {
+                    name: 'elem1',


@zxqfox не понял тебя. Формат реально не хороший :)

blond · 2016-12-10T13:24:34Z

test/stringify/v1.test.js

+
+const stringify = require('../../lib/stringify');
+
+test('should stringify enb declaration', t => {


blond · 2016-12-10T13:25:07Z

test/stringify/enb.test.js

@@ -2,10 +2,6 @@ const test = require('ava');

 const stringify = require('../../lib/stringify');

-test('should throws error if no format given', t => {


Почему тест исчез? Вроде годный

skad0 · 2016-12-11T20:20:39Z

🆙
@blond, по поводу тестов, не совсем понял какие кейсы еще не покрыты

blond

Тесты 🆗 .

По коду не нравится много find, лишних обходов и получение объектов через [0].

Написал немного замечаний.

blond · 2016-12-12T09:19:37Z

lib/format/v1.js

+        const item = { name: entity[entity.elem ? 'elem' : 'block'] };
+
+        entity.modName && (item.mods = [{ name: entity.modName }]);
+        entity.modVal && (item.mods[0].vals = [entity.modVal]);


Может { name: entity.modName } вынести в переменную modItem, чтобы не обращаться к mods[0]?

blond · 2016-12-12T09:24:09Z

lib/format/v1.js

+
+        const result = entity.elem ? { name: entity.block, elems: [item] } : item;
+
+        if (result.elems && entity.elem) {


Зачем проверка на entity.elem, если у result выставится elems, только если есть entity.elem.

blond · 2016-12-12T09:25:00Z

lib/format/v1.js

+
+        if (result.elems && entity.elem) {
+            // If exists block with existing elems property
+            const curBlock = acc.find(


Зачем переносы строк?

blond · 2016-12-12T09:36:44Z

lib/format/v1.js

+            if (curBlock) {
+                if (curBlock.elems) {
+                    // check if elem exists
+                    const curElem = curBlock.elems.find(elem => elem.name === entity.elem);


Так как мы уже проходились ранее по элементам, которые добавили, мы можем знать, какие были добавлены, а какие нет.

Чтобы не ходить каждый раз заново по наполняемому массиву можно хранить карту, где ключ это скоуп БЭМ-сущности, а значение ссылка на объект БЭМ-сущности внутри acc.

qfox · 2016-12-23T22:26:38Z

@skad0 Tests were not passed. Check it please?

qfox · 2016-12-23T22:29:06Z

1.js

+            { elem: 'elem', mods: { m2:'v2' } }
+        ]
+    }
+], { format: 'v2' }));


this pr now is in wip status

qfox · 2017-01-23T20:32:12Z

@skad0 Heya! Would be nice to see this in master

skad0 · 2017-01-23T20:35:39Z

@zxqfox will be happy if you can give me some advices tomorrow to complete this

skad0 · 2017-01-23T20:37:04Z

There is a try to create universal transformer based on which all other formatter can work.

skad0 · 2017-02-02T23:19:23Z

#92

skad0 added the status:in progress label Oct 3, 2016

tadatuta reviewed Oct 4, 2016

View reviewed changes

skad0 force-pushed the convert_v1 branch from 2f4d06a to cd79a3b Compare October 4, 2016 08:09

qfox reviewed Oct 4, 2016

View reviewed changes

skad0 force-pushed the convert_v1 branch from 914a777 to 4aa0dcc Compare October 10, 2016 19:06

skad0 force-pushed the convert_v1 branch from 4aa0dcc to 7ff38c5 Compare October 10, 2016 19:07

qfox reviewed Oct 12, 2016

View reviewed changes

skad0 force-pushed the convert_v1 branch from a0e31cd to 89535fc Compare October 17, 2016 14:36

skad0 force-pushed the convert_v1 branch 2 times, most recently from 8150307 to 4504c7f Compare October 19, 2016 09:49

blond requested changes Dec 10, 2016

View reviewed changes

skad0 added 2 commits December 11, 2016 21:10

Add convert for v1 format

0819e18

Add sugar convertation and tests + relative stringify test

c1372a8

skad0 force-pushed the convert_v1 branch from 4504c7f to c1372a8 Compare December 11, 2016 19:02

Short vals in mods for v1 convertation

474c72c

skad0 added 3 commits December 11, 2016 23:21

Convert renamed to format

80bb7b4

format, normalize -> folder/index

0cdd4bb

add tests

b766e43

blond requested changes Dec 12, 2016

View reviewed changes

skad0 added 2 commits December 12, 2016 12:40

w

b7f4afc

as

09487dc

qfox mentioned this pull request Dec 23, 2016

Use bem-cell #89

Merged

qfox reviewed Dec 23, 2016

View reviewed changes

skad0 changed the title ~~Add convert for v1 format~~ [wip] Add convert for v1 format Dec 24, 2016

skad0 closed this Feb 2, 2017


		const stringify = require('../../lib/stringify');

		test('should stringify enb declaration', t => {

		@@ -2,10 +2,6 @@ const test = require('ava');

		const stringify = require('../../lib/stringify');

		test('should throws error if no format given', t => {


		const result = entity.elem ? { name: entity.block, elems: [item] } : item;

		if (result.elems && entity.elem) {

[wip] Add convert for v1 format #70

[wip] Add convert for v1 format #70

Conversation

skad0 commented Oct 3, 2016 • edited Loading

tadatuta Oct 3, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tadatuta Oct 4, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qfox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skad0 commented Oct 10, 2016 • edited Loading

skad0 commented Oct 12, 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

qfox Oct 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skad0 Oct 12, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

skad0 commented Oct 17, 2016

tadatuta commented Dec 4, 2016

blond left a 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

skad0 commented Dec 11, 2016 • edited Loading

blond left a 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 commented Dec 23, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

qfox commented Jan 23, 2017

skad0 commented Jan 23, 2017

skad0 commented Jan 23, 2017

skad0 commented Feb 2, 2017

skad0 commented Oct 3, 2016 •

edited

Loading

tadatuta Oct 3, 2016 •

edited

Loading

tadatuta Oct 4, 2016 •

edited

Loading

skad0 commented Oct 10, 2016 •

edited

Loading

qfox Oct 12, 2016 •

edited

Loading

skad0 Oct 12, 2016 •

edited

Loading

skad0 commented Dec 11, 2016 •

edited

Loading