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 enb-beautify for dev environment #100

Closed
wants to merge 1 commit into from
Closed

added enb-beautify for dev environment #100

wants to merge 1 commit into from

Conversation

dab
Copy link

@dab dab commented Mar 30, 2015

No description provided.

]);

nodeConfig.addTargets([/* '?.bemtree.js', */ '?.html', '_?.css', '_?.js']);

// make html beauty if in `dev` environment
isProd ? null : nodeConfig.addTargets(['?.dev.html']);
Copy link
Member

Choose a reason for hiding this comment

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

isProd && nodeConfig.addTargets(['?.dev.html']);

Copy link
Member

Choose a reason for hiding this comment

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

+1

@dab
Copy link
Author

dab commented Mar 30, 2015

🆙

@tadatuta
Copy link
Member

@andrewblond тебе норм?

]);

nodeConfig.addTargets([/* '?.bemtree.js', */ '?.html', '_?.css', '_?.js']);

// make html beauty if in `dev` environment
!isProd && nodeConfig.addTargets(['?.dev.html']);
Copy link
Member

Choose a reason for hiding this comment

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

Думаю, что не нужно делать привязку к режиму, тем более таким способом.

Copy link
Author

Choose a reason for hiding this comment

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

В обсуждении говорили о привязке к dev-режиму. Переменная уже есть, которая знает о режиме. Чем способ странный?

Copy link
Member

Choose a reason for hiding this comment

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

Исправил опечатку.

Copy link
Member

Choose a reason for hiding this comment

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

@andrewblond догадываюсь, что ты про что-то типа

nodeConfig.mode('development', function(nodeConfig) {
    nodeConfig.addTargets(['?.tidy.html']);
});

но если не к режиму привязываться, то к чему?

Copy link
Member

Choose a reason for hiding this comment

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

@tadatuta Тут явно нужно привязываться к настройкам в каждом отдельном проекте или бандле (что суть одно — явно и в make.js).

@blond
Copy link
Member

blond commented Mar 30, 2015

Разделение на ?.dev.html и ?.html не прослеживается для других файлов. Из-за этого будет не очивидно, что ?.dev.html будет не минифицированный.

@qfox
Copy link
Member

qfox commented Mar 30, 2015

@andrewblond предлагаешь собирать в _?.html, а бьютифаером собирать ?.html? ;-)

@blond
Copy link
Member

blond commented Mar 30, 2015

@zxqfox, выделение dev-файлов, на мой взгляд логично при сборке dist'а. Но при сборке обычных бандлов надо в один и тот же файл собирать по разному, при разных режимах сборки.

Но я не знаю какой режим сборки предпологает минифицированный HTML-файл, а какой забьютифаенный.

От рудимента _?.target-name — предлагаю отказаться ;)

@dab
Copy link
Author

dab commented Mar 30, 2015

@andrewblond назвав файл dev — я лишь предположил это обсуждение. Планировал собирать разукрашенный код при разработке.
Если код нужно как-то допиливать руками — это явно не продакшен.

@blond
Copy link
Member

blond commented Mar 30, 2015

Если код нужно как-то допиливать руками — это явно не продакшен.

@tavriaforever с тобой не согласиться ;) Если разработчик верстает, чтобы передать собранные файлы дальше, то для него production — забьютифаенный HTML-файл.

Кроме того при добавлении пробельных символов в HTML вёрстка может разъехаться из-за inline-стилей. Т.е. при переключении из одного режима в другой у нас может быть разный результат. Я не знаю как решать эту проблему правильно :(

@tadatuta
Copy link
Member

@andrewblond

при добавлении пробельных символов в HTML вёрстка может разъехаться из-за inline-стилей

предлагаю оставить это на откуп разработчику. разве что можно в ридми явно описать вместе с рассказом о том, как собрать «красивый» html.

@dab
Copy link
Author

dab commented Mar 30, 2015

@tavriaforever да, я читал его комментарий. Имхо сделать еще один «разпрекрасный» файл — выход из положения.

@qfox
Copy link
Member

qfox commented Mar 30, 2015

@andrewblond Продакшн тут может быть только в случае сборки готовой статики. Если дальше идет разбивка по интерполируемым шаблонам — это и не продакшн, и не dev, это что-то между, и этому желательно быть отформатированным. Еще один наиболее частый кейс — это сам процесс верстки — и здесь тоже желательно иметь красивый файл, но не обязательно. Других кейсов я в принципе не вижу.

@qfox
Copy link
Member

qfox commented Mar 30, 2015

Т.е. при переключении из одного режима в другой у нас может быть разный результат. Я не знаю как решать эту проблему правильно :(

Оборачивать пробелы в html-комменты? типа <!--[\n\r\t]+-->

@blond
Copy link
Member

blond commented Mar 30, 2015

Оборачивать пробелы в html-комменты? типа

Что это даст? Это же нечитаемый код :)

@qfox
Copy link
Member

qfox commented Mar 30, 2015

@andrewblond Это решит проблему с inline стилями. Разве нет?

@blond
Copy link
Member

blond commented Mar 30, 2015

Это решит проблему с inline стилями. Разве нет?

Забьютифаенный код, нужен, чтобы его читали. Если каждый пробел превратиться в <!--[\n\r\t]+--> код станет нечитаемый, и тогда нет разницы между таким способом и минифицированным файлом.

Может быть сделаем новый режим, раз это и не dev и не production по смыслу?

@tadatuta
Copy link
Member

кажется, я придумал, что нужно сделать )
нужно переадресовать этот PR из этого репозитория в генератор и там задавать вопрос: «хотите оптимизированный html или красивый?»
и прямо там же в скобках предупреждать, что красивый он получается за счет бьютифаера и пробельные символы могут ломать инлайн-блоки. а в project-stub ограничиться ссылкой на enb-beautify.

за/против?

@qfox
Copy link
Member

qfox commented Mar 30, 2015

@andrewblond Можеть быть, но это один из кейсов. См. мой коммент выше — dev для процесса разработки (и он, видимо, будет ломать инлайн), production для публикации страницы в статике, и этот третий вариант — для передачи верстки дальше для интеграции (опять же, будет ломать инлайн стили).

dev и третий режимы хоть и близки, но немного про разное. В dev у нас предпросмотр результата, в третьем — файл для разбивки на шаблоны.

И на мой взгляд без решения задачи с инлайн-стилями (это значит, что бьютифаер должен много всего знать про css страницы, и про базовые display дом нод) — будет невозможно сделать универсальное решение.

@tadatuta это все равно не решает задачи с инлайном, но +1. В project-stub его можно сделать закомменченным (по аналогии со сборкой bemhtml на клиенте) — часто спрашивают.

@qfox
Copy link
Member

qfox commented Mar 30, 2015

Например, файл для разбивки может иметь комменты с началом и концом разных блоков — как бы намекая бэкенд разработчику, где его правильнее всего пилить. Для процесса разработки эти комментарии не нужны. Еще он может иметь лишние сбросы строк, чтобы проще было разбивать руками. В пределе он может собираться в готовые шаблоны, в т.ч. и haml, и twig, и что угодно еще. Это не задача бьютифаера, конечно, но я про режим в целом, в котором бьютифаер может быть лишь частью.

@dab
Copy link
Author

dab commented Mar 31, 2015

🆙 добавил закоментированный ?.tidy.html. Убрал зависимость от окружения. Можем добавить строку про это в README файл.

@qfox
Copy link
Member

qfox commented Mar 31, 2015

LGTM

@blond blond added the review label Jun 21, 2015
@tadatuta
Copy link
Member

close in favour of bem-archive/generator-bem-stub#124

@tadatuta tadatuta closed this Jun 30, 2015
@tadatuta tadatuta removed the review label Jun 30, 2015
@tadatuta tadatuta deleted the enb-beautify branch March 27, 2016 01:51
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

6 participants