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

Add initial .browserslistrc file #2602

Merged
merged 2 commits into from
Oct 14, 2019
Merged

Add initial .browserslistrc file #2602

merged 2 commits into from
Oct 14, 2019

Conversation

XhmikosR
Copy link
Contributor

@XhmikosR XhmikosR commented Sep 25, 2019

This is a very basic patch, mostly opened for discussion.

To visualize what's supported with this patch please check: https://browsersl.ist/?q=%3E%3D+0.2%25%2C+last+2+major+versions%2C+not+dead%2C+Android+%3E%3D+6%2C+Chrome+%3E%3D+45%2C+Edge+%3E%3D+12%2C+Explorer+%3E%3D+10%2C+Firefox+%3E%3D+38%2C+iOS+%3E%3D+9%2C+Safari+%3E%3D+9

and this is with the defaults: https://browsersl.ist/?q=defaults

Notes:

  • for Firefox we should probably use Firefox ESR
  • for Chrome, I'm not sure what the minimum version should be
  • for iOS/Safari I'm not sure either

Refs #2464

@XhmikosR
Copy link
Contributor Author

Alright, I changed the patch to be defaults + IE >= 10, see https://browsersl.ist/?q=defaults%2C+Explorer+%3E%3D+10

This should be the safest approach since without a .browserslistrc file the defaults are used.

@XhmikosR XhmikosR marked this pull request as ready for review September 30, 2019 19:12
@XhmikosR
Copy link
Contributor Author

XhmikosR commented Oct 13, 2019

Can we get a review about this? The solution I went with is as close as it gets to the current master.

IE9 does not work due to requestAnimationFrame, although the content is fine. Maybe I should add IE9 too?

This is the diff with IE9 added vs the current patch (which is defaults + IE>=10): https://gist.github.com/XhmikosR/0ffdac274b042e3e89f2c8c45d0943dd/revisions

Using browserslist defaults along with IE >= 10.
@XhmikosR
Copy link
Contributor Author

BTW if we decide to sort of support IE9, we should tweak the scroll to top script to not use requestAnimationFrame if it's not available. Everything else works AFAICT.

@Trott
Copy link
Member

Trott commented Oct 13, 2019

/ping @nodejs/website

@lpinca
Copy link
Member

lpinca commented Oct 13, 2019

I think this is fine and can be revisited later if it turns out to be too restrictive.

@nschonni
Copy link
Member

Is this used by any other tools besides the .use(autoprefixer()) line?

@XhmikosR
Copy link
Contributor Author

Currently no AFAICT, but it can be used by other tools.

@XhmikosR
Copy link
Contributor Author

So, how about IE 9?

@nschonni
Copy link
Member

Not sure if it matters much if this setup will be eventually replaced with https://github.com/nodejs/nodejs.dev/. Whatever browsers it's working with today is probably OK

@XhmikosR
Copy link
Contributor Author

Well, IE9 isn't currently specified in master. But apart from the JS error due to requestAnimationFrame, which we could work around, the display is good enough on IE9 AFAICT.

@MylesBorins
Copy link
Contributor

@XhmikosR I think we can move forward with this as is right now and in a follow up PR get a solution to fix IE9 and update the support table. Thoughts?

Copy link
Contributor

@MylesBorins MylesBorins left a comment

Choose a reason for hiding this comment

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

LGTM

@XhmikosR
Copy link
Contributor Author

SGTM. I'll leave #2464 open for discussion

@XhmikosR XhmikosR merged commit 3f1bf0a into nodejs:master Oct 14, 2019
@XhmikosR XhmikosR deleted the master-xmr-browserslist branch October 14, 2019 17:12
@XhmikosR XhmikosR mentioned this pull request Dec 26, 2019
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 this pull request may close these issues.

5 participants