bem-sdk-archive / bem-entity-name

BEM entity name representation. DEPRECATED →
https://github.com/bem/bem-sdk/tree/master/packages/entity-name
Other
5 stars 3 forks source link

Strict mod value #80

Open blond opened 7 years ago

blond commented 7 years ago
  1. We should throw error in constructor if mod value is not a String or true.

Actual:

const entityName1 = new BemEntityName({ block: 'block', mod: { name: 'mod', val: false } });
// BemEntityName { block: 'block', mod: { name: 'mod', val: undefined } }

const entityName2 = new BemEntityName({ block: 'block', mod: { name: 'mod', val: 42 } });
// BemEntityName { block: 'block', mod: { name: 'mod', val: 42 } }

const entityName3 = new BemEntityName({ block: 'block', mod: { name: 'mod' });
// BemEntityName { block: 'block', mod: { name: 'mod', val: true } }

Expected:

const entityName1 = new BemEntityName({ block: 'block', mod: { name: 'mod', val: false } });
// modifier value should be string or `true`

const entityName2 = new BemEntityName({ block: 'block', mod: { name: 'mod', val: 42 } });
// modifier value should be string or `true`

const entityName3 = new BemEntityName({ block: 'block', mod: { name: 'mod' });
// BemEntityName { block: 'block', mod: { name: 'mod', val: true } }
  1. We should support bem-naming logic with mod val false in BemEntityName.create():

Actual:

const entityName = BemEntityName.create({ block: 'block', mod: { name: 'mod', val: false } });
// BemEntityName { block: 'block', mod: { name: 'mod', val: undefined } }

Expected:

const entityName = new BemEntityName({ block: 'block', mod: { name: 'mod', val: false } });
// BemEntityName { block: 'block' }
  1. We should convert other values to string in BemEntityName.create()

Actual:

const entityName1 = BemEntityName.create({ block: 'block', mod: { name: 'mod', val: 42 } });
// BemEntityName { block: 'block', mod: { name: 'mod', val: 42 } }

const entityName2 = BemEntityName.create({ block: 'block', mod: { name: 'mod', val: entityName1 } });
// BemEntityName { block: 'block', mod: { name: 'mod', val: Object } }

Expected:

const entityName1 = BemEntityName.create({ block: 'block', mod: { name: 'mod', val: 42 } });
// BemEntityName { block: 'block', mod: { name: 'mod', val: '42' } }

const entityName2 = BemEntityName.create({ block: 'block', mod: { name: 'mod', val: entityName1 } });
// BemEntityName { block: 'block', mod: { name: 'mod', val: 'block_mod_42' } }
qfox commented 7 years ago

What about:

const entityName3 = BemEntityName.create({ block: entityName1, elem: entityName2, mod: { name: entityName1, val: entityName2 } });
// BemEntityName { block: 'block_mod_42', elem: 'block_mod_block_mod_42', mod: { name: 'block_mod_42', val: 'block_mod_block_mod_42' } }
Yeti-or commented 7 years ago

Let's stick to 2-nd variation

Yeti-or commented 7 years ago

or: BemEntityName.create() → 2 BemEntityName() → 1

blond commented 7 years ago

Coercion to string with toString() method can bring problems with BemEntityName objects.

Example:

const bemNaming = require('@bem/naming');
const BemEntityName = require('@bem/entity-name');

const entityName1 = BemEntityName.create({ block: 'block', mod: { name: 'mod', val: 'val' } });
const entityName2 = BemEntityName.create({ block: 'block', mod: { name: 'mod', val: entityName1 } });
// BemEntityName { block: 'block', mod: { name: 'mod', val: 'block_mod_val' } }

const str = bemNaming.stringify(entityName2); // block_mod_block_mod_val
bemNaming.parse(str); // undefined

But I think we should not handle this case (validate string) because value can be any string.

It may be necessary add validate to bemNaming.stringify method.