Support CommonJS #28
Support CommonJS #28
Conversation
|
@blond у меня сильные сомнения по поводу теста https://github.com/enb-make/enb-diverse-js/pull/28/files#diff-b57fdf30ce49f0637363fc0c68699588R129 |
| bundled = this._bundled || ''; | ||
|
|
||
| if (bundled) { | ||
| var files = sourceFiles.map(function (file) { |
There was a problem hiding this comment.
относительные пути нужны при любом значении bundled, так что их получение стоит вынести перед if
There was a problem hiding this comment.
@tadatuta почему? Мы же их используем только в контексте этой опции.
UPD: все, поняла, вынесу.
UPD2: как я могу их вынести, если у меня в каждом map используется file.fullname?
|
@blond апнула PR c учетом вчерашних и сегодняшних правок. |
| @@ -29,6 +29,8 @@ | |||
| "enb": ">= 0.8.22" | |||
| }, | |||
| "devDependencies": { | |||
| "chai": "^3.2.0", | |||
| }).should.throw(/fake/); | ||
| }) | ||
| .always(function () { | ||
| dropRequireCache(require, require.resolve('fake')); |
There was a problem hiding this comment.
Путь до модуля надо строить перед удалением папки node_modules.
|
@blond исправила замечания |
| @@ -26,7 +30,7 @@ describe('browser-js', function () { | |||
|
|
|||
| return build(blocks) | |||
| .then(function (content) { | |||
| content[0].must.be(reference); | |||
| content[0].should.be.eql(reference); | |||
There was a problem hiding this comment.
should.be.eql -> should.be.equal
Это разные операции сравнения. equal — идентичность (===), eql — глубокая эквивалентность (deepEqual).
Было must.be — идентичность (===).
|
Написал ещё пару замечаний, в остальном 🆗. |
|
@blond Спасибо, все исправила, все сосквошила. |
| globals.must.include('node-js'); | ||
| globals.must.have.length(1); | ||
| global.__fake.should.be.equal('fake'); | ||
| bundle.should.not.contain('module.exports = "fake";'); |
There was a problem hiding this comment.
Такая проверка ничего не гаранитрует. Да и проверять 2 кейса в одном тесте не ок.
Предлагаю убрать эту проверку из этого теста, переименовать его в must use CommonJS module, а последний тест must fail if node_modules does not exist переименовать в must not reveal node_modules.
There was a problem hiding this comment.
@blond спасибо за терпение, переделала, обновила, сосквошила.
|
@gela-d теперь тесты падают из-за кодстайла =) |
|
@blond перезапустила, все ок сейчас |
|
👍 |
Close #17