Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use bem decl for merge, subtract and intersect #232

Closed
wants to merge 45 commits into from
Closed

Conversation

skad0
Copy link
Member

@skad0 skad0 commented Sep 18, 2016

Will work when bem-naming mod field support will be enabled

blond and others added 30 commits March 30, 2016 11:21
The `node.getLevelNamingScheme` and the `node.setLevelNamingScheme`
methods will be removed in `enb@2.0.0`.
Got rid of scheme builders
Remove deprecated techs
Remove deprecated options
The `mock-fs` does not support `require` for Node.js 4.

Because of this test failed with error:

```
Cannot find module 'path/to/file.js
```
**Example of new format:**

```
{
    button__text: [
        {
            entity: { block: ‘button’, elem: ‘text’ },
            tech: ‘css’,
            path: ‘path/to/file.ext’,
            level: ‘path/to/level’
        },
        /* ... */
    ],
    /* ... */
}
```

With this introspection format to easily get a list of files named
entity.

**What has been done?**

* Use `bem-walk` to scan levels.
* Use `bem-naming` to get id of BEM entity.
* Add `BundleIntrospection` class to work with introspection of levels
for one bundle.
* Rewrite `levels` tech with `build-flow`.
* Use promises with `node.buildState` to avoid scanning the same levels
multiple times.
* Don't read files for directories (example i18n), it should do `files` tech.
Now the `bem-walk` is used to scan.
Exactly the same benchmarks in `bem-walk` repository
@skad0
Copy link
Member Author

skad0 commented Sep 18, 2016

/cc @blond @zxqfox @tadatuta

Copy link
Member

@tadatuta tadatuta left a comment

Choose a reason for hiding this comment

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

lgtm

@@ -1,3 +1,4 @@
const decl = require('bem-decl');
Copy link
Member

Choose a reason for hiding this comment

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

is it safe to use ES6 here?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.03%) to 95.181% when pulling c69003b on skad0.issue231 into a877bed on master.

@skad0
Copy link
Member Author

skad0 commented Sep 23, 2016

up
@tadatuta @blond

/**
* Coverts declaration modName and modVal to mod and val
*
* @param {Array} deps
Copy link
Member

Choose a reason for hiding this comment

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

looks like it can be an Object as well

* @param {Array} deps
* @return {Array}
*/
_convertFromMod: function (deps) {
Copy link
Member

Choose a reason for hiding this comment

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

it's possible to get rid of copy/paste like this (never tested so be careful):

function _switchModsFormat(conditionKey, modsMap, deps) {
    Array.isArray(deps) || (deps = [deps]);

    return deps.map(function (item) {
        if (!item[conditionKey]) return item;

        return Object.keys(item).reduce(function (acc, key) {
            acc[map[key] || key] = item[key];
            return acc;
        }, {});
    }
}

_convertFromMod: function (deps) {
    return _switchModsFormat('mod', {
        mod: modName,
        val: modVal
    }, deps);
}

_convertToMod: function (deps) {
    return _switchModsFormat('modName', {
        modName: mod,
        modVal: val
    }, deps);
}

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.01%) to 95.135% when pulling e8f96c0 on skad0.issue231 into a877bed on master.

@skad0
Copy link
Member Author

skad0 commented Sep 23, 2016

up
@blond @zxqfox

Copy link
Contributor

@qfox qfox left a comment

Choose a reason for hiding this comment

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

:shipit:

return _switchModsFormat('mod', {
mod: 'modName',
val: 'modVal'
}, deps);
Copy link
Contributor

Choose a reason for hiding this comment

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

@blond whyyyy!?

* @param {Array|Object} deps
* @return {Array}
*/
_convertToMod: function (deps) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Почему не вынести эти функции отдельно? Им же не нужен контекст this

@coveralls
Copy link

Coverage Status

Coverage decreased (-0.004%) to 95.143% when pulling db61590 on skad0.issue231 into a877bed on master.

@skad0
Copy link
Member Author

skad0 commented Oct 12, 2016

@blond @zxqfox @tadatuta
Закрываем его же?
Решено просто избавиться от deps.js и в технологиях напрямую использовать депсы

@blond
Copy link
Member

blond commented Dec 3, 2016

@skad0 какая связь между депсами и операциями над декларациями в разных форматах?

@blond
Copy link
Member

blond commented Dec 3, 2016

@skad0 можешь переоткрыть PR в 3.x ветку?

@skad0 skad0 closed this Sep 5, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants