fix: fixed merging of unsupported pseudos by caniuse and browserlist #883

anikethsaha · 2020-02-23T21:02:52Z

This change won't merge rules (having pseudo-classes) which are not either supported by caniuse in browserlist 's browsers.

The bug was even they were not supported, the compatibility was not changed which leads to false merging.

codecov-io · 2020-02-23T21:07:44Z

Codecov Report

Merging #883 into master will decrease coverage by 0.2%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #883      +/-   ##
==========================================
- Coverage   97.27%   97.07%   -0.21%     
==========================================
  Files         118      118              
  Lines        3452     3454       +2     
  Branches     1036     1037       +1     
==========================================
- Hits         3358     3353       -5     
- Misses         86       93       +7     
  Partials        8        8

Impacted Files	Coverage Δ
...postcss-merge-rules/src/lib/ensureCompatibility.js	`100% <100%> (ø)`	⬆️
packages/postcss-merge-rules/src/index.js	`93.67% <0%> (-4.03%)`	⬇️

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 fd7b878...f51afea. Read the comment docs.

alexander-akait

Looks good, what is changed in snapshots?

anikethsaha · 2020-02-25T11:18:42Z

Integration fixtures. there where some of the places in the framework fixtures where it was merging unsupported rules.

Output of yarn run build:integration

alexander-akait · 2020-02-25T11:23:00Z

Can you look on snapshot changes, and write them here (no need all, just couple)

anikethsaha · 2020-02-25T11:43:17Z

for cancise.css

It didnt merge the ::-webkit-slider-thumb selector (not supported from caniuse in browserlist 's default env)

gumpy.css

:visited not supported by us. not in the list here

alexander-akait · 2020-02-25T11:48:33Z

Can we fix visited here?

anikethsaha · 2020-02-25T11:56:16Z

Can we fix visited here?

I will surely fix it , but I dont know the name for caniuse to search it.

I tried
:visited
visited
css-visited

but for all of these, caniuse api is return error with cannot find name ...

Do you know the feature name for this. ?

alexander-akait · 2020-02-25T12:02:58Z

https://caniuse.com/#feat=mdn-css_selectors_visited, think we should try css_selectors_visited

anikethsaha · 2020-02-25T12:05:09Z

no luck

packages/postcss-merge-rules/src/lib/ensureCompatibility.js

alexander-akait · 2020-02-25T12:13:49Z

I will look on visited in near future

anikethsaha · 2020-03-02T13:07:36Z

cc @evilebottnawi

codecov-commenter · 2020-06-07T15:19:14Z

Codecov Report

Merging #883 into master will decrease coverage by 0.15%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #883      +/-   ##
==========================================
- Coverage   97.28%   97.12%   -0.16%     
==========================================
  Files         118      119       +1     
  Lines        3458     3479      +21     
  Branches     1040     1048       +8     
==========================================
+ Hits         3364     3379      +15     
- Misses         86       92       +6     
  Partials        8        8

Impacted Files	Coverage Δ
...postcss-merge-rules/src/lib/ensureCompatibility.js	`100.00% <100.00%> (ø)`
packages/postcss-merge-rules/src/index.js	`93.67% <0.00%> (-4.03%)`	⬇️
packages/postcss-ordered-values/src/index.js	`100.00% <0.00%> (ø)`
...ages/postcss-ordered-values/src/rules/listStyle.js	`94.73% <0.00%> (ø)`
packages/cssnano/src/__tests__/_processCss.js	`100.00% <0.00%> (+33.33%)`	⬆️

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 f326b7f...7c68ce6. Read the comment docs.

alexander-akait · 2020-06-08T14:15:06Z

packages/postcss-merge-rules/src/lib/ensureCompatibility.js

+  '::grammar-error': 'grammar-error',
+
+  '::part()': '::part',
+  '::slotted()': '::slotted',


Why some have () parenthesis, some doesn't have? It is bug?

let me check

Ok these should not have () but these are level 4 CSS Selector except for the visited.

And I think level 4 is not supported by caniuse-lite , so this all will be removed

anikethsaha · 2020-06-09T15:39:30Z

I think, visited is at selector level 3 ?

alexander-akait · 2020-06-09T17:32:59Z

@anikethsaha Yes 👍

anikethsaha · 2020-06-09T18:35:43Z

there were level 4 CSS selectors in early commits of this PR, I had to remove them as caniuse-lite doesnt support them i guess

alexander-akait · 2020-06-10T11:09:55Z

@anikethsaha So we do not merge selector for 4 level?

anikethsaha · 2020-06-10T11:12:24Z

currently no!

Cause we dont know their browser support so that may lead to wrong minification.
Once caniuse-lite supports that, I will add them.

alexander-akait · 2020-06-10T11:16:50Z

We have some of them https://caniuse.com/#search=selector%204

anikethsaha · 2020-06-10T11:24:16Z

I guess caniuse-lite API doesn't support that.

I couldn't locate the selectors here.

(may be I missed something, let me know)

alexander-akait · 2020-06-10T11:26:18Z

https://github.com/ben-eb/caniuse-lite/blob/master/data/features/css-not-sel-list.js
https://github.com/ben-eb/caniuse-lite/blob/master/data/features/css-any-link.js

anikethsaha · 2020-06-10T11:30:01Z

https://github.com/ben-eb/caniuse-lite/blob/master/data/features/css-not-sel-list.js
https://github.com/ben-eb/caniuse-lite/blob/master/data/features/css-any-link.js

ohh great, I thought they might have packed like level 2,3 are

I will add those which are supported.

packages/postcss-merge-rules/src/__tests__/index.js

alexander-akait · 2020-06-10T17:39:22Z

packages/postcss-merge-rules/src/lib/ensureCompatibility.js

+  ':visited': cssSel3,
+  ':any-link': 'css-any-link',
+  ':read-only': 'css-read-only-write',
+  ':read-write': 'css-read-only-write',


I think we can have more pseudo here

I guess I all covered the level 4. let me check if I missed others.

Can we create script to generate this stuff from caniuse-api? It is allow to validate we nothing is missed

i will check

Thanks, I am afraid we can missing something, so we need script to generate and validate it

what I am thinking is to simply query using the code name like for :react-write , query using react-write. but two issue can come

there may be more that one result, and we dont know which is correct

there may be no result as the query can be different

Maybe we can parse all existing pseudo from css-whatwg?

alexander-akait · 2020-06-10T17:51:03Z

:blank
:defined
:first
:fullscreen
:host
:host-context
:invalid
:matches
:any
:left
:link
:required
:right
:scope
:valid
:where

::cue
::cue-region
::grammar-error
::part
::slotted
::spelling-error

::-moz-progress-bar
::-moz-range-progress
::-moz-range-thumb
::-moz-range-track
::-ms-browse
::-ms-check
::-ms-clear
::-ms-expand
::-ms-fill
::-ms-fill-lower
::-ms-fill-upper
::-ms-reveal
::-ms-thumb
::-ms-ticks-after
::-ms-ticks-before
::-ms-tooltip
::-ms-track
::-ms-value
::-webkit-progress-bar
::-webkit-progress-value
::-webkit-slider-runnable-track
::-webkit-slider-thumb

Need to check each

anikethsaha · 2020-06-10T18:21:54Z

🚧: I will implement this here
⬆️ : same as above
🤔 : dont know the caniuse-lite feature name of the selector, tried searching here, let me know

pseudo selector	status
`:blank`	it is level 4, not sure its `caniuse-lite` feature name
`:defined`	not sure its `caniuse-lite` feature name
`:first`	✔️
`:fullscreen`	⚠️ , experimental, not sure about its caniuse-lite feature name
`:host`	it is level 1, 🤔
`:host-context`	🤔
`:invalid`	level 4, 🤔
`:matches`	renamed to `:is`, level 4, 🤔
`:any`	I think you meant `:any-link` ? I couldn't find `:any` in the mdn docs, Let me know
`:left`	✔️
`:link`	✔️
`:required`	level 4, 🤔
`:right`	✔️
`:scope`	level 4, 🤔
`:valid`	level 4, 🤔
`:where`	level 4, 🤔
`::cue`	IDK the feature name,
`::cue-region`	🤔
`::grammar-error`	🤔
`::part`	🤔
`::slotted`	🤔
`::spelling-error`	🤔

anikethsaha · 2020-06-10T18:32:10Z

Also, those verdor prefixed selectors will be un-prefixed using the postcss api here.
also, I couldn't find their feature name for caniuse-lite as well.

ludofischer · 2021-04-28T13:42:03Z

I think continuing this work could also fix #999

anikethsaha force-pushed the no-merge-for-unknown-pseudo branch from cc47223 to 8fb8163 Compare February 24, 2020 06:49

anikethsaha requested a review from alexander-akait February 24, 2020 11:00

alexander-akait reviewed Feb 25, 2020

View reviewed changes

packages/postcss-merge-rules/src/lib/ensureCompatibility.js Outdated Show resolved Hide resolved

anikethsaha requested a review from alexander-akait March 2, 2020 12:05

alexander-akait reviewed Jun 8, 2020

View reviewed changes

fix: merging of un-supported pseudo classes

448455a

anikethsaha force-pushed the no-merge-for-unknown-pseudo branch from 7f349fe to 448455a Compare June 9, 2020 17:31

test: fixed test

2a8eb7d

anikethsaha requested a review from alexander-akait June 9, 2020 18:34

alexander-akait previously approved these changes Jun 10, 2020

View reviewed changes

chore: added some more selectors of level 4

ad32f05

anikethsaha dismissed alexander-akait’s stale review via ad32f05 June 10, 2020 12:00

anikethsaha requested a review from alexander-akait June 10, 2020 12:16

alexander-akait reviewed Jun 10, 2020

View reviewed changes

packages/postcss-merge-rules/src/__tests__/index.js Show resolved Hide resolved

test: added test for merging

5c8c075

alexander-akait reviewed Jun 10, 2020

View reviewed changes

chore: added some more pseudo sel

7c68ce6

ludofischer added this to To do in merge-rules bugs Apr 28, 2021

ludofischer mentioned this pull request May 12, 2021

fix(postcss-merge-rules): add some missing known pseudo classes. #1099

Merged

fix: fixed merging of unsupported pseudos by caniuse and browserlist #883

Are you sure you want to change the base?

fix: fixed merging of unsupported pseudos by caniuse and browserlist #883

Conversation

anikethsaha commented Feb 23, 2020

codecov-io commented Feb 23, 2020 • edited

Codecov Report

alexander-akait left a comment

Choose a reason for hiding this comment

anikethsaha commented Feb 25, 2020 • edited

alexander-akait commented Feb 25, 2020

anikethsaha commented Feb 25, 2020 • edited

alexander-akait commented Feb 25, 2020

anikethsaha commented Feb 25, 2020

alexander-akait commented Feb 25, 2020

anikethsaha commented Feb 25, 2020

alexander-akait commented Feb 25, 2020

anikethsaha commented Mar 2, 2020

codecov-commenter commented Jun 7, 2020 • edited

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

anikethsaha commented Jun 9, 2020

alexander-akait commented Jun 9, 2020

anikethsaha commented Jun 9, 2020

alexander-akait commented Jun 10, 2020

anikethsaha commented Jun 10, 2020

alexander-akait commented Jun 10, 2020

anikethsaha commented Jun 10, 2020

alexander-akait commented Jun 10, 2020

anikethsaha commented Jun 10, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexander-akait commented Jun 10, 2020

anikethsaha commented Jun 10, 2020 • edited

anikethsaha commented Jun 10, 2020

ludofischer commented Apr 28, 2021

codecov-io commented Feb 23, 2020 •

edited

anikethsaha commented Feb 25, 2020 •

edited

anikethsaha commented Feb 25, 2020 •

edited

codecov-commenter commented Jun 7, 2020 •

edited

anikethsaha commented Jun 10, 2020 •

edited