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

Added support for YModules with backward compatibility #606

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from

Conversation

rakchaev
Copy link

Added possibility to use YModules in js files from own bem libraries based on bem-bl.

@qfox
Copy link
Member

qfox commented Feb 12, 2015

Great thing! 👍 I'm already want to get it ;-)

@arikon
Copy link
Member

arikon commented Feb 12, 2015

@rakchaev How and when to use it?

@rakchaev
Copy link
Author

@arikon
We use the next bem libs stack:

  • own bem libs
  • islands libs
  • bem-bl

We write js files from own bem levels with YModules.
It gives

  • benefits of module system;
  • possibility of easy migration to the bem-core library.

@arikon
Copy link
Member

arikon commented Feb 12, 2015

@rakchaev But how do you use it? You just write own project blocks using ym and that's it?

@@ -0,0 +1,19 @@
// Realization from bem-core#2.5.0
Copy link
Member

Choose a reason for hiding this comment

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

Realization -> Implementation

It will be more correct (here and in the other files)

@rakchaev
Copy link
Author

@arikon Yes. We just write own project blocks using ym. Like blocks of bem-core or bem-components.
Thank you for your notes.

@arikon
Copy link
Member

arikon commented Feb 12, 2015

I think it's not a good idea just because it's a 1-to-1 copy from bem-core. If these lines will change in bem-core it will be easier to move diff here.

@zxqfox The rule is simple: this code is harder to understand for the ordinal reader than simple if block. So it should be rewritten.

I don't support some of the decisions made for bem-core code style. In my opinion, they are for robots, not for humans.


/**
* @module i-bem__dom_init
*/
Copy link
Member

Choose a reason for hiding this comment

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

These style @module comments are not used in bem-bl

Copy link
Author

Choose a reason for hiding this comment

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

I've found usage of @module comments in bem-bl:

./blocks-common/i-jquery/__debounce/i-jquery__debounce.js: * @module i-jquery__debounce
./blocks-common/i-jquery/__decodeuri/i-jquery__decodeuri.js: * @module i-jquery__decodeuri
./blocks-common/i-jquery/__observable/i-jquery__observable.js: * @module i-jquery__observable
./blocks-common/i-geolocation/i-geolocation.js: * @module i-geolocation
./blocks-common/i-ecma/__json/i-ecma__json.js: * @module i-ecma__json
./blocks-common/i-ecma/__function/i-ecma__function.js: * @module i-ecma__function
./blocks-common/i-ecma/__string/i-ecma__string.js: * @module i-ecma__trim
./blocks-common/i-ecma/__object/i-ecma__object.js: * @module i-ecma__object
./blocks-common/i-ecma/__array/i-ecma__array.js: * @module i-ecma__array
./blocks-common/i-menu/i-menu.js: * @module i-menu
./blocks-common/i-bem/i-bem.js: * @module i-bem
./blocks-common/i-bem/__internal/i-bem__internal.js: * @module  i-bem__internal
./blocks-common/i-bem/__dom/_init/i-bem__dom_init.js: * @module i-bem__dom_init
./blocks-common/i-bem/__dom/i-bem__dom.js: * @module i-bem__dom
./blocks-common/b-menu-vert/__trigger/b-menu-vert__trigger.js: * @module b-menu-vert
./blocks-common/i-system/i-system.js: * @module i-system

@arikon
Copy link
Member

arikon commented Feb 12, 2015

@rakchaev What do you think of next-tick dependency?

@rakchaev
Copy link
Author

@arikon Yes, we can replace it.

@qfox
Copy link
Member

qfox commented Feb 13, 2015

@arikon I'm for wrapping that block to if in bem-core but it would be harder to sync. Anyway, I'm agree with ya. ;-)

@arikon
Copy link
Member

arikon commented Feb 13, 2015

bem-bl and bem-core are separate code bases. I think there is no need to focus on future syncronization (that would not happen at all).

13.02.2015, 19:50, "Alexej Yaroshevich" notifications@github.com:

@arikon I'm for wrapping that block to if in bem-core but it would be harder to sync. Anyway, I'm agree with ya. ;-)


Reply to this email directly or view it on GitHub.

Отправлено из мобильной Яндекс.Почты: http://m.ya.ru/ymail

@deeonis
Copy link
Contributor

deeonis commented Feb 18, 2015

@dfilatov could you review plz? there are doubts that we can accept this pr as is

@dfilatov
Copy link
Member

I think you shouldn't want this ) It's just increasing the entropy, nothing more

@qfox
Copy link
Member

qfox commented Feb 19, 2015

@dfilatov Is it blocker?

@arikon Would it fine for you if we add a comment or rewrite code to if statement?

Any chance to see this in the next release?

@dfilatov
Copy link
Member

@zxqfox blocker for what?

@qfox
Copy link
Member

qfox commented Feb 19, 2015

@dfilatov For mergin'

@dfilatov
Copy link
Member

If I were maintainer I wouldn't merge

@qfox
Copy link
Member

qfox commented Feb 19, 2015

@dfilatov Thank god you are not ;-)

upd Ловлю себя на мысли, что я бы тоже не влил ;-) Что если унести на отдельный уровень эти правки и подключать уровень только при необходимости? В отдельную библиотеку уносить тоже вариант, но в bem-bl такое не принято.

@deeonis
Copy link
Contributor

deeonis commented Feb 19, 2015

@arikon what do you think?

@arikon
Copy link
Member

arikon commented Feb 19, 2015

@dfilatov Дима, прокомментируешь реализацию?

19.02.2015, 16:47, "Filatov Dmitry" notifications@github.com:

I think you shouldn't want this ) It's just increasing the entropy, nothing more


Reply to this email directly or view it on GitHub.

Отправлено из мобильной Яндекс.Почты: http://m.ya.ru/ymail

@qfox
Copy link
Member

qfox commented Mar 10, 2015

polite ping @dfilatov

@dfilatov
Copy link
Member

@zxqfox I can't add to what I said anything else.

@arikon
Copy link
Member

arikon commented Mar 25, 2015

@dfilatov Дима, причины идеологические или технологические?

@corpix
Copy link
Contributor

corpix commented Mar 26, 2015

I like this feature. +1 for merge.

@andre487
Copy link
Contributor

+1

@narqo
Copy link
Member

narqo commented Mar 26, 2015

@rakchaev @corpix @Andre-487 вы можете описать, про какие именно «преимущества модульной системы» идет речь, с учетом того, что:

  1. объект BEM остается в глобальной области видимости;
  2. никакие другие блоки из библиотек заворачивать в модульную систему не планируют;
  3. jquery (и его плагины) тоже лежат в global.

Что мешает начать, учетом перечисленного выше, начать использовать ym у себя на проекте и оборачивать в modules.define свои блоки?

modules.define('my-block', function(provide) {
  provide(BEM.DOM.decl('my-block', {···});
});

На мой взгляд, ничего кроме увеличения сложности и запутанности кода, этот pr (речь про пользователей внутренних библиотек) не привносит.

Пример про «запутанность»:

// islands/blocks/button.js
BEM.DOM.decl('button', {
  doSomething() {
    ···
  }
});

// prj/blocks/my-block1.js
modules.define('my-block1', ['i-bem__dom'], function(provide, BemDom) {

provide(BemDom.decl('my-block1', {
  doSomethingMore() {
    //! этот код может нормально работать — 
    //  блок `button` задекларирован вне ym и код его `decl`'а уже выполнился к этому моменту!
    this.findBlockInside('button').doSomething();
  }
}));

});

// prj/blocks/my-block2.js
modules.define('my-block2', ['i-bem__dom'], function(provide, BemDom) {

provide(BemDom.decl('my-block2', {
  doSomethingMore() {
    //! а вот тут никто `my-block1` в массив зависимостей (2й аргумент define) не добавил — 
    //  вообще не факт, что этот код будет работать
    this.findBlockInside('my-block1').doSomethingMore();
  }
}));

});

@qfox
Copy link
Member

qfox commented Mar 26, 2015

Я не знаю о каких преимуществах идет речь, но этот фикс позволяет начинать писать блоки в рамках ymodules, и постепенно переводить с i-bem из bem-bl на i-bem из bem-core.
Т.к. bem-bl развиваться, явно, не собирается — то этот шаг упростить работу по переезду проектов в ymodules.

Про внутренних пользователей я ничего не знаю, но как-то странно это обсуждать в публичном репе.

upd Пример хорош для «порезался об ножницы — нужны царапки», имхо.

@andre487
Copy link
Contributor

Да, действительно смысл в облегчении миграции с bem-bl на bem-core.

@narqo
Copy link
Member

narqo commented Mar 26, 2015

upd Пример хорош для «порезался об ножницы — нужны царапки», имхо.

Я готов поспорить, что количество людей которые про это даже не задумаются (не то, чтобы попытаются понять), будет соразмерима с количеством людей которые не понимают зачем писать mustDeps от i-bem__html (возможно это NDA, но да, их много)

[..] этот шаг упростить работу по переезду проектов в ymodules

Возможно я не понимаю, но в чем упрощение? Проекту прямо сейчас можно начать использовать ym:

modules.define('my-button', function(provide) {
  provide(BEM.DOM.decl('my-button', {···});
});

Чем драматически помогает наличие BEMDOM в аргументах provide-функции? Можно примеров?

@qfox
Copy link
Member

qfox commented Mar 26, 2015

Я готов поспорить, что количество людей которые про это даже не задумаются (не то, чтобы попытаются понять), будет соразмерима с количеством людей которые не понимают зачем писать mustDeps от i-bem__html

Да нет смысла об этом спорить. Просто есть люди, которым надо переезжать, а есть те, которых и так все устраивает.
Фикс-то полезный, но возможно просто не для основной ветки. Думаю, об этом и стоит говорить.

Чем драматически помогает наличие BEMDOM в аргументах provide-функции? Можно примеров?

Тем, что когда все блоки будут описаны таким образом, можно будет, грубо говоря, сделать -bem-bl, +bem-core в зависимостях.

@tadatuta
Copy link
Member

Тем, что когда все блоки будут описаны таким образом, можно будет, грубо говоря, сделать -bem-bl, +bem-core в зависимостях.

@zxqfox тебе ли не знать, что заворачивание блоков в модульную систему — это самая простая задача из тех, с которыми придется столкнуться при миграции с bem-bl на bem-core?

@qfox
Copy link
Member

qfox commented Mar 26, 2015

тебе ли не знать, что заворачивание блоков в модульную систему — это самая простая задача из тех

Да это понятно, просто без таких правок (даже самых простых) переезжать постепенно становится невозможно. Можно, впрочем, вдобавок сделать в bemhtml: data-bem="каксейчас" onclick="return JSON.parse(this.getAttribute('data-bem')), сделать once = onFirst, и т.д.
С моей колокольни этого, думаю, на 95% хватит, чтобы начать переезд.

@qfox
Copy link
Member

qfox commented Mar 26, 2015

Просто я не думаю, что можно взять и просто заменить bem-bl на bem-core. Нужны какие-то переходные версии, которые позволят сгладить переезд. Есть и другой вариант — сделать версию bem-core совместимую со старым API. Но ведь это сложнее, правда?

@tadatuta
Copy link
Member

@zxqfox

  • bem-bl поддерживает data-bem="каксейчас" и переключение полностью обратносовместимо
  • bem-core 1.0 поддерживал название методов из bem-bl в статусе deprecated, так что такая промежуточная версия есть

@rakchaev
Copy link
Author

@narqo

вы можете описать, про какие именно «преимущества модульной системы» идет речь

Привет!
Да, попробую.

Что мешает начать, учетом перечисленного выше, начать использовать ym у себя на проекте и оборачивать в modules.define свои блоки?

modules.define('my-block', function(provide) {
  provide(BEM.DOM.decl('my-block', {···});
});

Инициализация блоков (запуск BEM.DOM.init из i-bem__dom_init_auto) может выполниться до того момента, когда будет задекларирован такой блок, пока зависимости его модуля еще не зарезолвены.
Поэтому в таком использовании никакого смысла нет.

На мой взгляд, ничего кроме увеличения сложности и запутанности кода, этот pr (речь про пользователей внутренних библиотек) не привносит.

Привносит пользователям библотеки:

  • benefits of module system;
  • possibility of easy migration to the bem-core library.

Пример про «запутанность»:

//! а вот тут никто `my-block1` в массив зависимостей (2й аргумент define) не добавил — 
//  вообще не факт, что этот код будет работать

Инициализация всех блоков выполнится только после резолва всех объявленных модулей, у которых в зависимостях есть i-bem__dom (см. i-bem__dom.js в PR или в bem-core).
Поэтому все будет работать.

@rakchaev
Copy link
Author

rakchaev commented Apr 6, 2015

В итоге, чтобы использовать на своем проекте модульную систему ym для реализации блоков на фреймворке i-bem, нужно чтобы ядро об этом знало и выполняло инициализацию всех блоков только после того, как будут определены все модули, в которых происходит декларация блоков.
Про это этот PR.

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

Successfully merging this pull request may close these issues.

9 participants