Skip to content
This repository has been archived by the owner on May 11, 2018. It is now read-only.

Explicitly resolve lowest browser version #121

Merged
merged 1 commit into from
Jan 6, 2017

Conversation

brokenmass
Copy link
Contributor

stop relying on browserlist returning a list sorted by browser version
fix #119

@existentialism
Copy link
Member

Nit, but can we dasherize the fixture names (ios-6, ios-10, etc)?

@brokenmass
Copy link
Contributor Author

@existentialism : oops my bad, fixed.

import "core-js/modules/es7.object.get-own-property-descriptors";
import "core-js/modules/web.timers";
import "core-js/modules/web.immediate";
import "core-js/modules/web.dom.iterable";
Copy link
Member

@yavorsky yavorsky Jan 5, 2017

Choose a reason for hiding this comment

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

These 3 builtIns is included by default in any cases because of lack of info about it's support #111 . Let's use something simpler, like arrow functions:

const a = () => 1;

@hzoo hzoo added the i: bug label Jan 6, 2017
stop relying on browserlist returning a list sorted by browser version
fix babel#119
@brokenmass
Copy link
Contributor Author

modified as requested

@yavorsky
Copy link
Member

yavorsky commented Jan 6, 2017

@brokenmass Looks good! Also the tests will fail if we add some shims in the future. But we haven't any features < ios 9, so using builtIns seems to be the only way if we want to test ios 6.
Thanks 👍

@hzoo
Copy link
Member

hzoo commented Jan 6, 2017

We should have a test with multiple browserlist entries like the original issue right? I don't see how the tests cover the changes in code atm since it seems to just test iOS (which is good too)

@brokenmass
Copy link
Contributor Author

brokenmass commented Jan 6, 2017

As the problem was caused by the getLowestVersions algorithm the query ios >= 6 is enough to 'generate' the issue, as it previously resolved as {ios: 10} because browserlist returned an unordered list.

Copy link
Member

@existentialism existentialism left a comment

Choose a reason for hiding this comment

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

LGTM

@hzoo hzoo merged commit 84d10f3 into babel:master Jan 6, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants