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

the Repo must be maintained #157

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open

Conversation

elawady55
Copy link

@elawady55 elawady55 commented Jun 22, 2021

Hi All,
When I update my projects to Angular 12, I found a really annoying bug that makes my compiled CSS had some syntax error. so I started to dig for this error and found that a lot of issues mentioned the same problem like angular/angular-cli#20975 and an opened one angular/angular-cli#20760 till I found that the problem in @angular-devkit/build-angular and it's dependant on GoogleChromeLabs/critters and I found an Issue opened GoogleChromeLabs/critters#74 but when I tracking the bug in the library it leads me to this project and found an open issue #154

so I found the problem in the Parser in [https://github.com/reworkcss/css/blob/434aa1733f275a67ea700311451b98a14f8cc21a/lib/parse/index.js#L516](this line), the regular expression not suitable for all scenarios especially the strings which had a semicolon inside the value like
@import url('https://fonts.googleapis.com/css2?family=Rubik:wght@300;400;500;600;700');

so I start to get a clone from this repo to fix this problem and I found this repo want really big attention because it follows a 9 years old code for example the test cases file had some bug in reading files so I fix it and add two new test cases for a start.

If this repo will be in a supply chain library for a big framework like Angular, the code must be maintained

@elawady55 elawady55 changed the title Replace All "var"s in test dir, fix cases replace method improving Test cases Jun 24, 2021
@elawady55 elawady55 changed the title improving Test cases the Repo must be maintained Jun 24, 2021
@notbear
Copy link

notbear commented Jun 25, 2021

Agree, the regexp needs some love. For example it would reject the last-in-file at-rule with omitted ending semicolon. It is correct syntax in css.

Regarding urls I'd suggest adding also a test for data URLs eg.
background: url("data:image/svg+xml;charset=UTF-8,%3Csvg%20width%3D%2236%22%20height%3D%2212%22%20viewBox%3D%220%200%2036%2012%22%20xmlns%3D%22http%3A%2F%2Fwww.w3.org%2F2000%2Fsvg%22%3E%0A%20%20%3Ccircle%20cx%3D%226%22%20cy%3D%226%22%20r%3D%223%22%20fill%3D%22rgba(0%2C%200%2C%200%2C%20.2)%22%3E%0A%20%20%20%20%3Canimate%20attributeName%3D%22r%22%20values%3D%223%3B5%3B3%22%20calcMode%3D%22linear%22%20dur%3D%221s%22%20repeatCount%3D%22indefinite%22%20%2F%3E%0A%20%20%3C%2Fcircle%3E%0A%20%20%3Ccircle%20cx%3D%2218%22%20cy%3D%226%22%20r%3D%223%22%20fill%3D%22rgba(0%2C%200%2C%200%2C%20.2)%22%3E%0A%20%20%20%20%3Canimate%20attributeName%3D%22r%22%20values%3D%223%3B5%3B3%22%20calcMode%3D%22linear%22%20begin%3D%22.33s%22%20dur%3D%221s%22%20repeatCount%3D%22indefinite%22%20%2F%3E%0A%20%20%3C%2Fcircle%3E%0A%20%20%3Ccircle%20cx%3D%2230%22%20cy%3D%226%22%20r%3D%223%22%20fill%3D%22rgba(0%2C%200%2C%200%2C%20.2)%22%3E%0A%20%20%20%20%3Canimate%20attributeName%3D%22r%22%20values%3D%223%3B5%3B3%22%20calcMode%3D%22linear%22%20begin%3D%22.66s%22%20dur%3D%221s%22%20repeatCount%3D%22indefinite%22%20%2F%3E%0A%20%20%3C%2Fcircle%3E%0A%3C%2Fsvg%3E%0A");

@holblin
Copy link

holblin commented Jun 9, 2022

Hi, I'm not sure if this repo is still used for the last version of Angular but I created a fork of it and plan to maintain it at: https://github.com/adobe/css-tools
In a few days, the version will be published into npmjs.

@holblin
Copy link

holblin commented Jun 10, 2022

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.

None yet

4 participants