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

NpmScript - build #3

Closed
dyuvzhenko opened this issue Nov 13, 2017 · 45 comments
Closed

NpmScript - build #3

dyuvzhenko opened this issue Nov 13, 2017 · 45 comments
Assignees

Comments

@dyuvzhenko
Copy link
Owner

No description provided.

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 13, 2017

@dhilt Начал делать build - 41df64c.
И сразу вопрос насчет последовательности из Gruntfile.js:

    'jshint:tests',
    'jshint:sources',
    'clean:temp',
    'webpack:compressed',
    'karma:compressed',
    'webpack:default',
    'copy:sources',
    'copy:jqLiteExtrasFake'

Нам нужна каждая команда из списка, так ведь?
Если да, выходит следующими шагами должно быть размещение внутри webpack'а jshint'а и его двух конфигураций из списка, зачистка temp. 4 пункт выполнен в коммите.

А вот как должен выглядеть код далее... Возможно, последние четыре пункта из списка должны все писаться в package.json'е, примерно так:

...
"build": "webpack --config webpack/config.js && \"npm run dev-test\" && \"npm run dev-build\" && \"node run ./copy-method.js\"",
...

(где copy-method.js - это файл с кодом, занимающийся копированием нужных файлов из /temp в /dist. Последние два пункта из Gruntfile)

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 13, 2017

@dhilt Ну и теперь ./webpack/config.js после коммита полностью копирует ./webpack.config.js. Возможно, стоит удалить последний и на его место поставить первый файл.
Хотя если у нас появится отдельный файл с копированием из /temp в /dist, может лучше эти дела оставить в отдельной папке ./webpack.

@dyuvzhenko
Copy link
Owner Author

@dhilt Добавил удаление папки /temp - a149be1.

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 13, 2017

Пока был в поисках информации насчет jshint, приметил что есть у нас clean-webpack-plugin и переписал удаление /temp вместе с ним - 74acb55.

Я так понимаю, плагин с копированием можно не применять, ибо копирование должно идти после двух сборок проекта, production и development. Буду делать отдельным файлом.

@dhilt
Copy link
Collaborator

dhilt commented Nov 13, 2017

Я исправил несколько важнейших моментов. Prod работает в папке ./dist. Prod собирает как сжатые, так и несжатые файлы. Prod не наблюдает за изменениями.

@dyuvzhenko
Copy link
Owner Author

@dhilt Так-так, в Gruntfile как я понял prod свои файлы оставлял в ./temp, и далее последние две из восьми grunt-задачи копировали результаты в ./dist. Если я все правильно понял, то если теперь prod оставляет все файлы в ./dist, то копирование не нужно?

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 13, 2017

@dhilt По сравнению с исходным репозиторием не хватает двух файлов *jqlite*.js, похоже все-таки нужно. Хотя в этих файлах собственно ничего и нет...

@dhilt
Copy link
Collaborator

dhilt commented Nov 13, 2017

@bitden Да, но нам важен результат, а не путь, которым мы к нему приходим. Таким образом мы сделаем путь короче... Не будем копировать вслед за Грантом дистрибутив, Вебпак сам кладет его сразу куда надо. Но на счет заглушки для jqlite надо подумать, можно ли обойтись буз копирования. Я думаю да, через еще один entry-point в вебпаке. Еще есть проблема, что Banner-комментарии режутся в сжатых версиях.

@dhilt
Copy link
Collaborator

dhilt commented Nov 13, 2017

@bitden Я кажется понял на счет Баннера, попробуй поменять местами порядок плагинов:


    new CleanWebpackPlugin(configEnv.outputFolder, {
      root: path.join(__dirname, '..')
    }),
    ...configEnv.plugins,
    new webpack.BannerPlugin(getBanner(configEnv.compressing))

Там важна последовательность. И тогда не очень удобно пока получается с configEnv.plugins, но это мы еще подумаем.

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 13, 2017

screenshot

И еще кажется появились некие изменения в собранных файлах. Или это может какие-то исключительно внутренние системные webpack-заморочки, не отражающиеся на самой сути кода?

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 13, 2017

@dhilt Попробовал, в *.min.js баннер нормально пишется построчно, после предложенной смены порядка кода. Исключая файлы *.js.map

@dhilt
Copy link
Collaborator

dhilt commented Nov 13, 2017

@bitden Возможно это связано с тем, что я поднял мажорную версию Вебпака до 3. В .map файлах не должно быть никаких банеров.

@dhilt
Copy link
Collaborator

dhilt commented Nov 13, 2017

@bitden Попробуй https://github.com/webpack-contrib/jshint-loader для замены

    'jshint:tests',
    'jshint:sources',

@dyuvzhenko
Copy link
Owner Author

@dhilt Да, именно эту библиотеку пока и изучаю, сейчас буду пробовать ее применить. А в остальном, нам остается только придумать что-то с jqlite-файлами и подкорректировать команду build в package.json, добавив ей ... && \"npm run dev-test\"?

@dhilt
Copy link
Collaborator

dhilt commented Nov 13, 2017

@bitden ТЕсты нужно запускать на минифиципрванном дистрибутиве (&& "npm run prod-test" ?).
ui-scroll-jqlite.js нужно просто скопировать из ./src в ./dist (в ./temp этого не надо), и там же должен оказаться не менее фейковый ui-scroll-jqlite.min.js. Минифицировать можно по-настоящему, а можно простым переименованием копии, как это делал Грант.

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 13, 2017

@dhilt Так тесты выпадают из данной задачи или надо как-то умудриться их запустить после сборки min-версий файлов, а затем после этого и остальные файлы собрать?

А насчет ui-scroll-jqlite думаю лучше использовать copy-webpack-plugin. Иначе с минифицированием можно подумать что этот файл как-то задействован и имеет какую-то роль и тем самым создается путаница, как мне кажется. Кстати говоря, а какая у него роль?)

@dhilt
Copy link
Collaborator

dhilt commented Nov 13, 2017

@bitden Раньше ui-scroll-jqlite существовал в виде отдельного модуля. Проблемы, предшествовавшие объединению его с модулем ui-scroll, описаны здесь angular-ui#72. Теперь это всего лишь заглушка для обратной совместимости. В принципе, рано или поздно она должна уйти. Следуя semver, это можно будет сделать при переходе на 2.x. Мы не вполне точно следуем semver, поэтому гипотетически его можно было бы убрать уже сейчас...

Минифицированные и неминифицированные файлы в новой версии собираются одновременно, соответственно тесты нужно запускать строго после отработки сборщика. И синтаксис && в package.json подходит для этого.

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 13, 2017

@dhilt Синтаксис && конечно же подходит, но у нас ведь после следующего изменения - https://github.com/BitDen/ui-scroll/blob/master/webpack/config.js#L81, npm run build и npm run dev-build содержат неминифицированные файлы, следовательно после оператора && в package.json тесты пойдут по неминифицированным файлам. Наверное, я пока что чего-то не понимаю.

Тем временем, применил jshint-loader - 6904779. Корявенько так применил. Сколько не искал, нигде не мог найти как бы .jshintrc применить как задумано, в итоге пока что сделал его .json-файлом. И тут вопрос есть насчет 'jshint:tests' и 'jshint:sources', кто из этих двух относится к нашим нынешним production и development?

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 13, 2017

@dhilt Я пока что склоняюсь к ответу, что development режим должен содержать .jshinrc, а production .jshintrc плюс дополнительно описанные правила в 'jshint:tests':

{
          node: true,
          globals: {
            angular: false,
            inject: false,
            jQuery: false,
            jasmine: false,
            afterEach: false,
            beforeEach: false,
            ddescribe: false,
            describe: false,
            expect: false,
            iit: false,
            it: false,
            spyOn: false,
            xdescribe: false,
            xit: false
          }

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 13, 2017

@dhilt Добавил копирование двух *jqlite.js-файлов - baa18e5. Правда библиотека оказалась с багом, и вроде как я не должен писать options с {copyUnmodified: true}.
И еще оставив ссылку на этот баг тут, в библиотеке сopy-webpack-plugin появилась ссылка (на 156 issue) на наш issue, ну и дела.

@dhilt
Copy link
Collaborator

dhilt commented Nov 13, 2017

@bitden Я сделал комит с приблизительным результатом по npm run build. Я согласен, что есть отклонения, но посмотри, как оно работает, Webpack кладет прод-сборку в ./dist, а Karma запускает тесты поверх ./dist/min.

@dyuvzhenko
Copy link
Owner Author

@dhilt Выходит осталось только с jshint по этой задаче определиться?

@dhilt
Copy link
Collaborator

dhilt commented Nov 13, 2017

@bitden Да, и надо бы сделать так, чтобы Webpack заваливал сборку, если jshint не проходит.

@dyuvzhenko
Copy link
Owner Author

@dhilt А какие там отклонения? Конечный результат вроде бы в точности как у Gruntfile, исключая по jshint.

@dyuvzhenko
Copy link
Owner Author

@dhilt Повторю тогда вопрос насчет jshint - 'jshint:tests' и 'jshint:sources', кто из этих двух относится к нашим нынешним production и development?

@dhilt
Copy link
Collaborator

dhilt commented Nov 13, 2017

@bitden Давай напустим jshint на исходники (./src) одинаково для обоих окружений. А для dev-окружения еще и на тесты (./test). Обрати внимание на файлы .jshintrc – они задают частные правила для папок.

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 14, 2017

@dhilt Пока что перенес jshint-конфиги для prod и dev сборок, в точности как в Gruntfile - a1ae6f7.
Осталось только вспомнить как регулярки писать. И все должно запуститься вроде бы.
Ну и да, пришлось второй .jshintrc переименовать в .jshintrc.json.

@dyuvzhenko
Copy link
Owner Author

@dhilt Написал вроде бы верные регулярки - e708e0f.
Теперь надо бы как-нибудь ошибку спровоцировать, или как там с jshint устроено.

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 14, 2017

И вдогоноку переписал немного регулярки - fab01d4.
И похоже ничего не работает пока что.

@dhilt
Copy link
Collaborator

dhilt commented Nov 14, 2017

@bitden Я заставил jshint-loader и babel-loader работать вместе. Теперь надо разобраться с .json и jshint для тестов.

UPD: .jshintrc работает их коробки. Причем можно перекрывать правила, раскидывая .jshintrc файлы по проекту, без дополнительных ссылок. Еще я удалил ./src/.jshintrc. Остались тесты.

@dhilt
Copy link
Collaborator

dhilt commented Nov 14, 2017

@bitden Остались тесты, я имел в виду jshint на тесты.

@dhilt dhilt reopened this Nov 14, 2017
@dyuvzhenko
Copy link
Owner Author

@dhilt Вроде готово: 85f435e.

@dyuvzhenko
Copy link
Owner Author

@dhilt Вру, еще не готово.

@dhilt
Copy link
Collaborator

dhilt commented Nov 14, 2017

@bitden Да, вижу. Проверить просто: замени в globals

describe2: false,

и jshint должен выругаться. Еще надо будет удалить верхние комментарии из спек вида

  /*global describe, beforeEach, module, it, expect, runTest */

И потом вынести правила в .test/.jshintrc

@dhilt
Copy link
Collaborator

dhilt commented Nov 14, 2017

@bitden Плагины для дев потеряли смысл, я сделал рефакторинг конфига, надеюсь, тебе это не помешает. Сейчас одна проблема есть с плагинами, она уже давно, насколько я могу судить, это метка compressed/uncompressed в баннере. Она не работает, т.к. оба варианта собираются в окружении prod.

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 14, 2017

@dhilt Ничего не помешало, комментарии убрал, правила перенес в отдельный .jshintrc.

Но, все еще ничего не работает. Уже отчаялся и полез в node_modules в библиотеку jshint-loader, далее в rcloader и rcfinder, расставлял там console.log'и чтобы увидеть доходят ли с webpack-конфига две команды, два адреса для .jshintrc - и нет, доходит только одна. Пробовал и по отдельности, и массивом в include впихнуть - тоже ничего. А еще есть похоже синтаксическая ошибочка с [...configEnv.rules, ], почему-то таким образом данные не отображаются как задумывается - но проблема не в этом.

@dhilt
Copy link
Collaborator

dhilt commented Nov 14, 2017

@bitden Надо поискать решение в интернете, не закапывайся слишком глубоко. Скорее всего webpack не может работать с исходниками вне деревьей, растущих из entry points. Задай вопрос на Stackoverflow! как запустить jshint на файлах, не участвующих в сборке? может быть это можно сделать через Karma?

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 14, 2017

@dhilt Такс, вопрос задал - https://stackoverflow.com/questions/47291494/webpack-jshint-loader-jshint-doesnt-watch-out-for-two-various-directories. Не знаю насколько удачно, но задал)
И к сожалению нельзя было создать тег jshint-loader, опять же нужна репутация, на сей раз 1500.

@dhilt
Copy link
Collaborator

dhilt commented Nov 14, 2017

@bitden Добро пожаловать на StackOverflow. 7 просмотров у твоего вопроса, мой 8, без слез не взглянешь, удивительная непопулярность)) Я отредактировал его. Пока же можно продолжить поиски и/или заняться npm start.

@dyuvzhenko
Copy link
Owner Author

@dhilt Ого, можно оказывается чужие вопросы корректировать, интересно как)

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

@dhilt
Copy link
Collaborator

dhilt commented Nov 15, 2017

@bitden 👍 На SO работает система привилегий. По-моему начиная с 1000 очков репутации можно вносить правки в чужие вопросы/ответы без модерации.

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 15, 2017

@dhilt Кажется мне, без включения файлов в entry point не обойтись. Пока что добавил *Spec.js-файлы в entry point по крайней мере, так все работает славно. За исключением того, что теперь куча файлов создается в корневой папке. Коммит - bdb65e9.

Хотя выходит таким образом они тоже проходят сборку и это довольно сильный побочный эффект выходит.

@dhilt
Copy link
Collaborator

dhilt commented Nov 15, 2017

@bitden Да, это непорядок и нужно найти способ избежать этого. А может быть создать отдельный процесс npm run hint-tests? Который чисто через npm прогоняет jshint? И добавить его в начало npm test через &&?

@dyuvzhenko
Copy link
Owner Author

dyuvzhenko commented Nov 15, 2017

@dhilt Сделал, работает - 1bd7bf6.
Только наверное надо было через concurrently запускать, чтобы отслеживание изменений в *Spec.js-файлах было.

@dyuvzhenko
Copy link
Owner Author

@dhilt Заметил что npm-script build все еще отличается от того, что есть в Gruntfile.js в одноименной задаче. Не хватает же jshint-прохода по директории ./test. Добавил в общем - aa429bf.

@dhilt dhilt closed this as completed Nov 19, 2017
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

No branches or pull requests

2 participants