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

feat: avoid fatal error #137

Merged
merged 1 commit into from
Dec 29, 2021
Merged

feat: avoid fatal error #137

merged 1 commit into from
Dec 29, 2021

Conversation

alexander-akait
Copy link
Collaborator

@alexander-akait alexander-akait commented May 28, 2021

/cc @ludofischer @Semigradsky

The main idea is avoid fatal error when we can't parse and keep value as it, should we use warning or error? Should we have options to change warning on error and versa vice.

@ludofischer
Copy link
Collaborator

The main idea is avoid fatal error when we can't parse and keep value as it, should we use warning or error? Should we have options to change warning on error and versa vice.

In the context of cssnano I think it should be a warning, if the code reaches minification we assume the CSS is already tested and validated. There's already a way to turn PostCSS warnings into errors, and I don't think adding the option to one plugin at a time is a common use case (why would you want to only error when there are calc() warnings but not on other warnings?)

@alexander-akait
Copy link
Collaborator Author

why would you want to only error when there are calc() warnings but not on other warnings?

Because here he handle only calc 😄

Right now many developers get fatal errors due this using cssnano, we should fix it

@ludofischer
Copy link
Collaborator

I think this is fine, people can use a linter to prevent them from writing invalid CSS.

@alexander-akait
Copy link
Collaborator Author

@ludofischer anyway we should use postcss API for errors, no just throw an error

@ludofischer
Copy link
Collaborator

@ludofischer anyway we should use postcss API for errors, no just throw an error

Yes, this should solve cssnano/cssnano#910

@ludofischer
Copy link
Collaborator

Is this ready? I've received permissions on this repository so I could merge it.

@alexander-akait
Copy link
Collaborator Author

Ready, just want to ensure you review my changes

@ludofischer
Copy link
Collaborator

Looks fine, we can fix the quote formatting in a different PR as right now it's inconsistent throughout the code base.

@ludofischer ludofischer merged commit 125be6a into master Dec 29, 2021
@ludofischer ludofischer deleted the avoid-fatal-error branch December 29, 2021 18:58
@alexander-akait
Copy link
Collaborator Author

do you have access for publish?

@ludofischer
Copy link
Collaborator

do you have access for publish?

No.

@alexander-akait
Copy link
Collaborator Author

alexander-akait commented Jan 3, 2022

@ludofischer Sorry for delay, added permission for postcss-calc, now you can publish

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