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

Добавлен кастомный postcss плагин postcss-global-data #287

Merged
merged 6 commits into from
Dec 5, 2024

Conversation

VladislavNsk
Copy link
Contributor

@VladislavNsk VladislavNsk commented Nov 9, 2024

Стандартный плагин каждый раз парсил одни и те же переданные файлы в options на каждом css файле.
Плюс в каждый файл, не зависимо надо или не надо, много или мало глобальных переменных, всегда добавлял все глобальные переменные

Сейчас файл будет парситься 1 раз и переменные добавляются только те, которые нужны

[email protected]

Copy link

changeset-bot bot commented Nov 9, 2024

🦋 Changeset detected

Latest commit: 53896eb

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
arui-scripts Minor
example-modules Patch
example Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@@ -28,7 +26,6 @@ export const postcssPlugins = [
'postcss-mixins',
'postcss-for',
'postcss-each',
'@csstools/postcss-global-data',
Copy link
Collaborator

Choose a reason for hiding this comment

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

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

Предлагаю такой вариант:
в функции createPostcssConfig инициализировать все плагины по старому, если же pluginName === '@csstools/postcss-global-data' - использовать кастомный плагин.
На сколько помню можно передавать вместо массива [плагин, опции] сразу инициализированный плагин

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ок, поправлю

Copy link
Contributor Author

@VladislavNsk VladislavNsk Nov 10, 2024

Choose a reason for hiding this comment

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

Если это сделать в createPostcssConfig, то проектам придется все равно делать изменения, тк например такая проверка уже будет не валидна в файле arui-scripts.overrides.ts
if (Array.isArray(plugin) && plugin[0] === '@csstools/postcss-global-data')
тк вместо массива, будет функция создания плагина или объект, если инициализовать на месте плагин

Может лучше сделать это сразу после applyOverrides? Тогда уже будет применен конфиг из проекта и можно заменять на кастомный с использованием переданных новых файлов

Сделал это в последнем комите
Если есть другие мысли или я не так понял, поправлю)

@heymdall-legal
Copy link
Collaborator

Вообще оч круто получилось, проверил сейчас на крупном проекте - вместе с доп оптимизациями из rspack получилось процентов так на 40 время на postcss сократить. типа 86 секунд было, 50 стало. Повнимательнее почитаю в будний день, но вообще оч круто

@VladislavNsk VladislavNsk force-pushed the feature/postcss-perfom branch from e81bdf4 to a8c1949 Compare November 10, 2024 08:30
@VladislavNsk
Copy link
Contributor Author

VladislavNsk commented Nov 10, 2024

Так же вижу например есть проект, который все css файла импортирует из коров
и эти файлы добавляются в этот плагин, в том числе там есть mixin
https://git.moscow.alfaintra.net/projects/MOOS/repos/verify-ui/browse/arui-scripts.overrides.ts#63

Если ранее плагин просто добавлял все переданные файлы в css, то сейчас же он умеет только парсить глобальные переменные и кастомные медиа запросы

Как будто тут не верно сконфигурирован плагин на проекте и файлы с миксинами надо переместить в options плагина postcss-mixins
https://git.moscow.alfaintra.net/projects/MOOS/repos/verify-ui/browse/arui-scripts.overrides.ts#78

Или надо будет сделать поддержку mixin?
Если не делать поддержку, что по мне логично вроде) то проектам кто делает так же, надо будет поправить оверайды корректно

@VladislavNsk VladislavNsk force-pushed the feature/postcss-perfom branch 2 times, most recently from c6afba3 to deea02c Compare November 25, 2024 07:16
@VladislavNsk
Copy link
Contributor Author

Последние изменения:
плагин @csstools/postcss-global-data полностью удален, если кому-то будет нужен, надо будет поставить себе на проект отдельно

Подрефакторил функцию createPostcssConfig, чтобы была возможность переопределять options и для плагинов импортируемых на прямую в массиве postcssPlugins

@VladislavNsk
Copy link
Contributor Author

не понимаю почему тесты не проходят, локально выполняю yarn run test --force и никаких ошибок нет

@VladislavNsk VladislavNsk force-pushed the feature/postcss-perfom branch from 768dbb1 to d3234ee Compare November 29, 2024 07:16
@VladislavNsk
Copy link
Contributor Author

не понимаю почему тесты не проходят, локально выполняю yarn run test --force и никаких ошибок нет

Разобрался, засквошил лишние комиты
Сейчас все вроде гуд

@VladislavNsk
Copy link
Contributor Author

arui-scripts@0.0.0-next-0.0.1-postcss-global-variables-20241129074045

return plugins.map((pluginName) => {
if (pluginName in options) {
return [pluginName, options[pluginName]];
console.log('pluginName: ', pluginName);
Copy link
Collaborator

Choose a reason for hiding this comment

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

console.log лишний

This comment was marked as resolved.

}

return pluginName;
console.log('pluginName.name', pluginName.name);
Copy link
Collaborator

Choose a reason for hiding this comment

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

и тут

Once(root, postcssHelpers): void {
if (!Object.keys(parsedVariables).length) {
options.files.forEach((filePath) => {
console.log('parse file: ', filePath);
Copy link
Collaborator

Choose a reason for hiding this comment

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

и тут log

'arui-scripts': minor
---

Добавлен кастомный плагин postcss-global-data, для оптимизации времени обработки глобальных переменных
Copy link
Collaborator

Choose a reason for hiding this comment

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

Тут конечно возникает вопрос, который в скриптах часто сложно решить - изменение внутренней структуры вроде как не является мажоркой, но тем не менее обновление на новую версию точно сломает что-то командам, которые в оверрайдах использовали postcss-global-data.

Я бы наверное оставил версию как minor, но добавил тут чуть больше информации типа:
Проекты, которые использовали в оверрайдах кастомные настройки для плагина global-data должны у себя поменять название на такое-то (ну или что так они должны сделать)

Обмочевский Владислав Вячеславович and others added 2 commits December 5, 2024 00:47
@VladislavNsk
Copy link
Contributor Author

Поправил, протестировал демку, все ок
arui-scripts@0.0.0-next-0.0.2-postcss-global-variables-20241204180330

return plugins.map((pluginName) => {
if (pluginName in options) {
return [pluginName, options[pluginName]];
console.log('pluginName: ', pluginName);

This comment was marked as resolved.

@VladislavNsk VladislavNsk merged commit 5d15dcf into master Dec 5, 2024
5 checks passed
@core-ds-bot core-ds-bot mentioned this pull request Dec 5, 2024
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.

2 participants