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

[Bug] Using "last X versions or not dead", nearly not more prefixing. #476

Closed
Yivan opened this issue Apr 19, 2020 · 10 comments · Fixed by #478
Closed

[Bug] Using "last X versions or not dead", nearly not more prefixing. #476

Yivan opened this issue Apr 19, 2020 · 10 comments · Fixed by #478

Comments

@Yivan
Copy link
Contributor

Yivan commented Apr 19, 2020

Hello,

I spend long time on this, but i arrive to the conclusion that there is a bug when "or not dead" condition used (settings option or .rc file same).

Bug:
Adding "or not dead" doesn't prefix the file as intended.

To reproduce:
Run a first time with just "last 10 versions": you will get a well prefixed files with many prefix.
Run a second time with "last 10 versions or not dead": you get no more prefixers (or very few), and file size is a lot lighter so. It should not as it is a "or" condition.

What is strange is that the list of browsers detected is right... so it is like there is something special during prefixing for this condition in the prefixing code itself... if it can help to fix the bug, maybe there is something here.

This bugs doesn't happen with condition like "or not ie <= 10", in this case it works fine.

I hope it could be fixed, because if some devolpers and a condition like this, they break all the prefixing...

Thanks!

@Yivan
Copy link
Contributor Author

Yivan commented Apr 19, 2020

For indication I opened firstly an issue here postcss/autoprefixer#1304

@ai
Copy link
Member

ai commented Apr 19, 2020

Here is current behavior:

$ npx browserslist "last 1 version"
and_chr 80
and_ff 68
and_qq 1.2
and_uc 12.12
android 80
baidu 7.12
bb 10
chrome 80
edge 80
firefox 74
ie 11
ie_mob 11
ios_saf 13.3
kaios 2.5
op_mini all
op_mob 46
opera 66
safari 13
samsung 11.1
$ npx browserslist "last 1 version or not dead"
and_chr 80
and_ff 68
and_qq 1.2
and_uc 12.12
android 80
baidu 7.12
chrome 80
edge 80
firefox 74
ie 11
ios_saf 13.3
kaios 2.5
op_mini all
op_mob 46
opera 66
safari 13
samsung 11.1

What output do you expect?

@Yivan
Copy link
Contributor Author

Yivan commented Apr 19, 2020

Thanks for your fast return on this.
The ouput is right, and I got the same on my side too.
Yes I know that list returned is fine (as i stated in my orginal message What is strange is that the list of browsers detected is right...).

Like i said this is autoprefixer the problem:
Case 1: last 10 versions => the renderer css is 200ko for instance (many prefix)
Case 2: last 10 version or not dead => the renderer css is 120ko for instance (just a few prefix)
Making a diff of them show that Case 1 has many more prefix added, but Case 2 should have at least the same prefix... and he has not, here is the problem.

Fell free to ask any information which can help tracking the problem.
I use the latest versions of the packages.

@ai
Copy link
Member

ai commented Apr 19, 2020

Case 2 should have at least the same prefix...

Docs say that or not X will remove browsers as well. So everything is as expected.

https://github.com/browserslist/browserslist#query-composition

I agree that it could be non-obvious. But It is a rare case, I do not think we should we should spend our time here.

Just use , not dead always.

@ai ai closed this as completed Apr 19, 2020
@Dan503
Copy link

Dan503 commented Apr 20, 2020

I noticed that ie_mob 11 is in the $ npx browserslist "last 1 version" output but not the $ npx browserslist "last 1 version or not dead" output.

That doesn't sound right. "or" should mean that all of the "last 1 version" entries should appear in the "last 1 version or not dead" list as well.

@ai
Copy link
Member

ai commented Apr 20, 2020

or === ,

last 1 version, not dead was part of public API and we can’t change it. This is why X, not Y == X or not Y == X and not Y.

Yeap, it is not really correct. But the correct form will force strange construction like (last 1 version or > 1%) and not ie 11 and will break public API.

@Yivan
Copy link
Contributor Author

Yivan commented Apr 20, 2020

Thanks for all this explainations. I understand now.
So all is normal.
I suggest docs could be improved on this as it is not so easy to spot this use case.

Maybe udpate:

Obviously you can not start with a not combiner, since there is no left-hand side query to combine it with.

With:

Obviously you can not start with a not combiner, since there is no left-hand side query to combine it with. It must be noted that the handside is always relsolved as AND logic operator even if OR is used (this is an API implementation specificity).

And in this snippet:

> .5% and not last 2 versions
> .5% or not last 2 versions
> .5%, not last 2 versions

change to somethings like:

All those 3 cases are equivalent too `.5% and not last 2 versions` as using OR NOT is transformed in AND NOT at the API level
> .5% and not last 2 versions
> .5% or not last 2 versions
> .5%, not last 2 versions

You get the idea ; )
Actually using "not" without knowing this is really prone to silently removing browsers check, as any developer could think (and they are right) "or not" is "or not". But as "or not" is transformed during the process in "and not" it should be indicated more clearly I think.

Thanks again !

@ai
Copy link
Member

ai commented Apr 20, 2020

I always like idea to update docs. Can you send PR?

@Yivan
Copy link
Contributor Author

Yivan commented Apr 20, 2020

Sure !
I would like to add a precedance example too (between and and or operators).
You confim me that AND has precedance on OR ?

So something like:
'> 0.5%', 'last 2 versions and not Safari', 'Firefox ESR', 'not ie <= 10', 'not ie_mob <= 10'

Will give:
(> 0.5% or (last 2 versions and not Safari) or Firefox ESR) and not ie <= 10 and not ie_mob <= 10

Am I right ?
I will add this precedance example too, so it make it more clear for developpers.

@Yivan
Copy link
Contributor Author

Yivan commented Apr 20, 2020

PR suggested.
Thanks.

tats-u pushed a commit to tats-u/browserslist that referenced this issue Apr 8, 2023
* Add hash links support

* Add query to hash redirect

* Fix back to empty query

* Fix back to empty query

* Add longer cache to server

* Clean up server code

* Add server based redirect

* Fix history

* Clean up server code

* Fix test concurrency

* Re-use URL
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 a pull request may close this issue.

3 participants