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

Global dot delimited deps #501

Merged
merged 2 commits into from
Mar 5, 2018
Merged

Conversation

dustyo-O
Copy link
Contributor

@dustyo-O dustyo-O commented Feb 13, 2018

Changes proposed in this pull request

Gets dot-delimited deps from global scope in object notation

Checklist

  • Tests added

@coveralls
Copy link

Coverage Status

Coverage remained the same at 99.389% when pulling 98bc964 on dustyo-O:global-dot-delimited-deps into 117146e on bem:master.

@bem bem deleted a comment from coveralls Feb 13, 2018
@@ -226,6 +266,45 @@ describe('API generate', function() {
return assert.equal(res.fakeReq.getText(), 'globals');
});
});

it('must get dependency from global scope with dot-delimited key' +
Copy link
Contributor

Choose a reason for hiding this comment

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

Я перестал понимать что тестирует этот тест и тест ниже…

Copy link
Contributor

@miripiruni miripiruni Feb 13, 2018

Choose a reason for hiding this comment

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

Дело в том, что в коде, вроде бы, нет никакой ветки, которую он покрывает… А самый первый добавленный тест в этом PR тестирует достаточно и полно.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

все дело в том, что в Ymodules мы все модули заворачиваем в YModules обертку, поэтому все кейсы где YModules - я дополнительно расширил этой ситуацией. можно увидеть, если поискать по deps.global в коде compiler - там три вхождения.

@miripiruni
Copy link
Contributor

miripiruni commented Feb 13, 2018

@tadatuta а зачем это необходимо? Как после этого можно будет пользоваться переменными содержащими точку в имени?

Например, я захочу подключить lodash.get.

@tadatuta
Copy link
Member

@miripiruni

а зачем это необходимо?

у нас достаточно кода, написанного в неймспейсе BEM, например, BEM.I18N. Его нужно уметь передать в шаблоны из глобала.

Как после этого можно будет пользоваться переменными содержащими точку в имени?

или я не понимаю вопрос, или именно этот кейс как раз починится с этим PR.

@miripiruni
Copy link
Contributor

или я не понимаю вопрос, или именно этот кейс как раз починится с этим PR.

@tadatuta сейчас имя содержащее точку будет воспринято как путь https://github.com/bem/bem-xjst/pull/501/files#diff-ee97c5091b89979aace94674818996baR47

@miripiruni
Copy link
Contributor

у нас достаточно кода, написанного в неймспейсе BEM, например, BEM.I18N. Его нужно уметь передать в шаблоны из глобала.

Но ведь нет никакой проблемы передать его как I18N. Зачем передавать путь, который неявным образом будет пореплейшен?

@tadatuta
Copy link
Member

Но ведь нет никакой проблемы передать его как I18N. Зачем передавать путь, который неявным образом будет пореплейшен?

Безотносительно правильности, это в любом случае мажорное изменение. Оставлять так в 8.x нельзя.

@miripiruni
Copy link
Contributor

Похоже тут с моей стороны возникло сильное недопонимание. Прошу прощения. В ходе интеграционного тестирования этого PR-а, а так же PR-а про https://github.com/enb/enb-bemxjst/pull/286/files я понял, что текущий код bem-xjst v8.6.8 и выше содержит деградацию: не поддерживает подключение библиотек вида { requires: { i18n: { globals: 'BEM.I18N' } } }.

Деградация выражена в двух багах:

  1. раньше работало подключение библиотек из глобальной области видимости с помощью указания строки вида 'BEM.I18N'. Моя ошибка была в том, что я думал, что это никогда не работало. Работало. Исправляю.

  2. в enb-bemxjst перестала восприниматься опция requires, хотя её ждут в bem-core. Исправляю.

cc @tadatuta

@miripiruni miripiruni merged commit fc7b19f into bem:master Mar 5, 2018
@miripiruni miripiruni deleted the global-dot-delimited-deps branch March 5, 2018 10:04
@miripiruni miripiruni added this to the 8.9.1 milestone Mar 5, 2018
@miripiruni
Copy link
Contributor

  • bem-xjst@8.9.1

@miripiruni
Copy link
Contributor

bem/bem-core#1558

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.

4 participants