Skip to content

[Meta] Replace CleanCSS by LightningCSS #2429

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

Merged
merged 1 commit into from
Dec 7, 2024

Conversation

Kocal
Copy link
Member

@Kocal Kocal commented Dec 5, 2024

Q A
Bug fix? no
New feature? no
Issues Fix #...
License MIT

One more step to modernize our build stack, by replacing https://github.com/clean-css/clean-css by https://lightningcss.dev/.

TBH it does not bring a lot of value since we only have 4 very small CSS files, buuuut that's for a better future :) (#rolldown)

@carsonbot carsonbot added the Status: Needs Review Needs to be reviewed label Dec 5, 2024
Copy link

github-actions bot commented Dec 5, 2024

📊 Packages dist files size difference

Thanks for the PR! Here is the difference in size of the packages dist files between the base branch and the PR.
Please review the changes and make sure they are expected.

FileBefore (Size / Gzip)After (Size / Gzip)
Dropzone
style.min.css 949 B / 475 B 944 B-1% 📉 / 468 B-1% 📉
LiveComponent
live.min.css 79 B / 82 B 78 B-1% 📉 / 81 B-1% 📉
TogglePassword
style.min.css 338 B / 228 B 311 B-8% 📉 / 216 B-5% 📉

@Kocal Kocal force-pushed the meta-replace-clean-css-by-lightning-css branch from b9dcde3 to 3f2e4f4 Compare December 5, 2024 16:06
@Kocal Kocal changed the title [Meta] Replace CleanCSS by Lightning CSS [Meta] Replace CleanCSS by LightningCSS Dec 5, 2024
@Kocal Kocal requested review from kbond, smnandre and WebMamba December 5, 2024 16:16
@Kocal Kocal merged commit 49449f2 into symfony:2.x Dec 7, 2024
60 checks passed
@Kocal Kocal deleted the meta-replace-clean-css-by-lightning-css branch December 7, 2024 10:01
[data-loading=""],[data-loading=show],[data-loading=delay\|show]{display:none}
Copy link
Member

Choose a reason for hiding this comment

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

@Kocal Not sure about that... and we may want to discuss this kind of thing together 😅

.toggle-password-container{position:relative}.toggle-password-icon{width:1rem;height:1rem}.toggle-password-button{background-color:#0000;border:none;flex-direction:row;place-items:center;column-gap:.25rem;height:1rem;font-size:.875rem;line-height:1.25rem;display:flex;position:absolute;top:-1.25rem;right:.5rem}
Copy link
Member

Choose a reason for hiding this comment

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

1% less compatibility ;)

@@ -22,7 +22,7 @@
"@rollup/plugin-typescript": "^11.1.6",
"@symfony/stimulus-testing": "^2.0.1",
"@vitest/browser": "^2.1.1",
"clean-css": "^5.3.3",
"lightningcss": "^1.28.2",
Copy link
Member

Choose a reason for hiding this comment

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

+1 for the change
-1 for the hurry 😅

filename: path.basename(inputStyleFile, '.css'),
code: Buffer.from(css),
minify: true,
sourceMap: false, // TODO: Maybe we can add source maps later? :)
Copy link
Member

Choose a reason for hiding this comment

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

Source Maps have more interest when there is multiple source files, variables, etc.. not the case here.

But if we have a component offering CSS, why not

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Needs Review Needs to be reviewed
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants