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

Got Chrome 4 as earliest supported version #472

Closed
max-mykhailenko opened this issue Apr 12, 2020 · 20 comments · Fixed by #572
Closed

Got Chrome 4 as earliest supported version #472

max-mykhailenko opened this issue Apr 12, 2020 · 20 comments · Fixed by #572

Comments

@max-mykhailenko
Copy link

Commands and results
npx browserslist '> 0.5% in my stats'

Result

and_chr 80
android 80
chrome 80
chrome 70
firefox 74
ios_saf 13.4
ios_saf 13.3
ios_saf 13.0-13.1
ios_saf 12.2-12.4
safari 13

Looks like it's exactly what I have in my browserslist-stats.json.

But when I use babel/preset-env and pass the same string/same config got next (debug mode on)

Using targets:
{
  "android": "80",
  "chrome": "4",
  "firefox": "74",
  "ios": "12.2",
  "safari": "13"
}

When I pass all browsers' version into babel-env directly I don't have problems with build.

Versions
"@babel/core": "7.9.0"

@ai
Copy link
Member

ai commented Apr 12, 2020

Can you show results of npx browserslist … and npx broeserslist --mobile-to-desktop.

It will allow us to be sure, that it is not Babel problem.

@max-mykhailenko
Copy link
Author

@ai looks like "@babel/core": "7.9.0" installs browserslist version which doesn't have --mobile-to-desktop arg

@max-mykhailenko
Copy link
Author

Babel-env installs browserslist version 4.10.0

@max-mykhailenko
Copy link
Author

npx browserslist --mobile-to-desktop '> 0.5% in my stats'
browserslist: Unknown arguments --mobile-to-desktop.

Usage:
  npx browserslist
  npx browserslist "QUERIES"
  npx browserslist --json "QUERIES"
  npx browserslist --config="path/to/browserlist/file"
  npx browserslist --coverage "QUERIES"
  npx browserslist --coverage=US "QUERIES"
  npx browserslist --coverage=US,RU,global "QUERIES"
  npx browserslist --env="environment name defined in config"
  npx browserslist --stats="path/to/browserlist/stats/file"
  npx browserslist --update-db

@ai
Copy link
Member

ai commented Apr 12, 2020

Start the new project without any dependency expect browserslist to avoid the problem of having old dependencies in node_modules.

@ai
Copy link
Member

ai commented Apr 12, 2020

You may use different versions of Browserslist in npx and Babel. Your problem may be already fixed in the latest version.

@max-mykhailenko
Copy link
Author

max-mykhailenko commented Apr 12, 2020

@ai thanks for advise. I keep only browserslist-stats.json

npx browserslist --mobile-to-desktop '> 0.5% in my stats'
npx: installed 11 in 2.251s
and_chr 4
android 80
chrome 80
chrome 70
firefox 74
ios_saf 13.4
ios_saf 13.3
ios_saf 13.0-13.1
ios_saf 12.2-12.4
safari 13


npx browserslist '> 0.5% in my stats'
npx: installed 11 in 2.154s
and_chr 80
android 80
chrome 80
chrome 70
firefox 74
ios_saf 13.4
ios_saf 13.3
ios_saf 13.0-13.1
ios_saf 12.2-12.4
safari 13

@ai
Copy link
Member

ai commented Apr 13, 2020

Thanks for the reproduction stand. I will not have a time next week, because of different open source projects.

Can I ask you to review this code in your test stand? https://github.com/browserslist/browserslist/blob/master/index.js#L248-L260

@max-mykhailenko
Copy link
Author

I'll review during this week. May I ask your advise about condition like '> 0.5% in my stats but chrome > 70'. Is it possible somehow with the current codebase?

@ai
Copy link
Member

ai commented Apr 13, 2020

May I ask your advise about condition like '> 0.5% in my stats but chrome > 70'. Is it possible somehow with the current codebase?

> 0.5% in my stats, not chrome < 70

@aronwoost
Copy link
Contributor

I'm seeing the same issue.

How can I assist to debug this issue?

When doing npx browserslist --mobile-to-desktop '> 0.5% in my stats' I'm also seeing and_chr 4 which is causing "chrome": "4", right?

@aronwoost
Copy link
Contributor

I digged into the code a bit.

The problem is normalizeStats() together with mobileToDesktop being true.

Since both arguments in normalizeStats(data, stats) have one version for and_chr, the output is {and_chr: { '0': 1 }} (check this condition). Is assume this is correct and by design (I'm not fully aware of the architecture).

Then, since mobileToDesktop is true, chrome is cloned with all the chrome entries and {and_chr: { '0': 1 }} results in picking the first chrome version, which is 4.

I'm willing to work on this issue, but I'm unsure where to tackle it. I don't want to make normalizeStats() aware of mobileToDesktop. We could quick-fix it in byName() by adding another if. But I'm not sure about it.

@ai
Copy link
Member

ai commented Dec 30, 2020

Can you send PR? It is hard to understand your fix in text description.

@aronwoost
Copy link
Contributor

@ai True, absolutely impossible to understand! 😂😂😂

I'm willing to fix, but I'm unsure where.

You were correct above 👆🏼, byName() does not work as expected.

I provided a failing test in #572. Does this help?

@ai
Copy link
Member

ai commented Dec 30, 2020

@aronwoost can you check this PR? Maybe we already fixed the problem

#511

@aronwoost
Copy link
Contributor

@ai

Same result in the PR

    Expected substring: "and_chr 87"
    Received string:    "and_chr 4"

@Semigradsky
Copy link
Contributor

@aronwoost your test can be fixed by changing

var normal = Object.keys(data[i].versions)[0]

to

      var normal = data[i].versions[0]

Could you try this change?

@ai As I see data[i].versions is array of strings. Maybe earlier it could have been an object?

@aronwoost
Copy link
Contributor

@Semigradsky The change does indeed fixes the test. But it does not fixes the browserslist --mobile-to-desktop '> 0.5% in my stats' issue.

Sorry for pointing in the wrong direction. 😕

Let me see if I can create a better test.

@aronwoost
Copy link
Contributor

I doubled checked and the fix works as expected.

I was running npm browserslist --mobile-to-desktop '> 0.5% in my stats'

When running node cli.js --mobile-to-desktop '> 0.5% in my stats' it does no longer reports and_chr 4 but and_chr 87. 🎉

I updated the PR.

@ai ai closed this as completed in #572 Jan 6, 2021
@ai
Copy link
Member

ai commented Jan 6, 2021

The fix released in 4.16.1.

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.

4 participants