-
-
Notifications
You must be signed in to change notification settings - Fork 78.9k
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
Revisit our browserslist config #30986
Conversation
Hmm, makes me think we could drop legacy Edge (17 and 18) given the difference there in those %s. |
bec3bed
to
b22052d
Compare
Is this okay to review and get merged @XhmikosR? |
I'm still not sure if it's the right time to drop Legacy Edge. The percentage, which might seem small, does not reflect the real number. I'm just thinking out loud here so that we make the right choice for our users and the project. Also, I wanted to make another change, I'll do it later. |
f7efefa
to
61197dc
Compare
Despite the fact that I'd love to use logical properties for RTL, I'm not sure about dropping Legacy Edge. The New Edge is only available for every Windows for a few weeks, isn't it? |
Yeah, it hasn't shipped to all supported Windows 10 versions yet AFAICT and I don't know where they track this. Also, there might be more stuff we can remove if we go with the Edge removal and ideally I'd like all the related changes to be in the same PR (not this one). EDIT: Also, do note that the Legacy Edge removal saves us a significant amount of bytes from the JS dist files. CSS doesn't save a lot. So, I'm not sure how to proceed. |
6e23ef7
to
a12ee41
Compare
Chatted with @XhmikosR about this in Slack and wanted to reiterate here. I'm in favor of dropping this knowing that it'll be a couple months to stable v5 and we're going to be working on this for at least a year, hopefully a little more. Having some foresight into what things will look like at that point is what I'm hoping to rally around. if that makes sense. |
Also slating this for alpha 2 now to make sure we're all aligned on it :). |
57ee6c1
to
66cee3b
Compare
1d5f4e3
to
736d1ce
Compare
So, here's the current situation:
The only "issue" I see is iOS 9.3 being included, and some other browsers we might not care about. I think if we switch to >= 0.25% and last 3 major versions it might be better?
/CC @twbs/team |
Also, do note that with this patch as is the generated JS files use EDIT: scratch that, it was due to another local change I had ( |
Let's roll with the second option I think @XhmikosR. We can drop iOS 9.3 IMO. |
I need to revisit this thoroughly because 1) iOS 10 tests fail and 2) I see |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pre-approving for when this comes up for review.
423783c
to
42a9bad
Compare
Use browserslist defaults which is `> 0.5%, last 2 versions, Firefox ESR, not dead`
Let's land this as is; it doesn't really change anything in the dist files as is. Due to the issue we are having when removing Android, I say we revisit this later. |
fcb8247
to
0804c93
Compare
Closes #29314.
I kept the Edge patch separate because ATM the update hasn't shipped on all supported Windows versions: https://support.microsoft.com/en-us/help/4541302/the-new-microsoft-edge-is-available (see also my last point)
master
branch:This branch:
This branch without the Edge patch:
Notes:
defaults
change, but IMO we should change what we were using because1 last version
isn't properPreview: https://deploy-preview-30986--twbs-bootstrap.netlify.app/
EDIT: actually dropping Opera Mini should allow us to use
const
/let
. I will experiment with that later.