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

fix: issue#1112 #1116

Merged
merged 3 commits into from May 24, 2021
Merged

fix: issue#1112 #1116

merged 3 commits into from May 24, 2021

Conversation

codeonquer
Copy link
Contributor

@codeonquer codeonquer commented May 20, 2021

fix #1112

My idea is:

  • find all conflictingProps in firstDecls after prop in intersection
  • find conflictingProps in intersection
  • they should be the same

Copy link
Member

@alexander-akait alexander-akait left a comment

Choose a reason for hiding this comment

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

Can you add test case(s)?

@codeonquer
Copy link
Contributor Author

I have added my test case.

My test:

h1{color:#001;color:#002;color:#003}h2{color:#001;color:#002}

before:

h1{color:#002;color:#003}h1,h2{color:#001}h2{color:#002}

after

h1{color:#001;color:#002;color:#003}h2{color:#001;color:#002}

@codeonquer
Copy link
Contributor Author

What should i do next @alexander-akait

@alexander-akait
Copy link
Member

/cc @ludofischer can you look at this?

@codecov-commenter
Copy link

codecov-commenter commented May 24, 2021

Codecov Report

Merging #1116 (9d72fca) into master (dceafe2) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1116   +/-   ##
=======================================
  Coverage   96.36%   96.36%           
=======================================
  Files         115      115           
  Lines        3575     3576    +1     
  Branches     1051     1051           
=======================================
+ Hits         3445     3446    +1     
  Misses        121      121           
  Partials        9        9           
Impacted Files Coverage Δ
packages/postcss-merge-rules/src/index.js 97.59% <100.00%> (+0.01%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dceafe2...9d72fca. Read the comment docs.

Copy link
Collaborator

@ludofischer ludofischer left a comment

Choose a reason for hiding this comment

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

Although I have never touched this part of the code, I think this is a legitimate fix for a legitimate bug. There are also no changes to the integration test results, so I believe we can safely merge this.

@ludofischer ludofischer merged commit 069c424 into cssnano:master May 24, 2021
@ludofischer
Copy link
Collaborator

@codeonquer Thank you for your PR!

@codeonquer
Copy link
Contributor Author

I am glad to contribute to this project! Thank you all @alexander-akait @ludofischer

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.

[Bug] : PropConflict when using postcss-merge-rules
4 participants