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

Strict mod value #80

Open
blond opened this issue Feb 14, 2017 · 4 comments
Open

Strict mod value #80

blond opened this issue Feb 14, 2017 · 4 comments
Milestone

Comments

@blond
Copy link
Member

blond commented Feb 14, 2017

  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
Copy link
Contributor

qfox commented Feb 14, 2017

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
Copy link
Member

Yeti-or commented Feb 14, 2017

Let's stick to 2-nd variation

@Yeti-or
Copy link
Member

Yeti-or commented Feb 14, 2017

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

@blond blond removed the question label Apr 15, 2017
@blond blond added this to the 2.0 milestone Apr 15, 2017
@blond
Copy link
Member Author

blond commented Apr 15, 2017

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.

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

No branches or pull requests

3 participants