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

enforce ES5 in all client files, fix usage in ES5 only browsers #1241

Closed
wants to merge 1 commit into from

Conversation

yyx990803
Copy link
Contributor

What kind of change does this PR introduce?

bugfix

Did you add or update the examples/?
No

Summary
There were prior issues regarding the client files, after being upgraded to ES6, not working properly in IE11 (#1084). However, the fix (c9d32f8) left let and const in the client codebase, which works in IE11, but not in other ES5 compliant browsers (IE<11, UC Browser on Android).

Certain regions of the world, in particular China and developing countries, still has a large percentage of browsers that only support ES5 (UC Browser has over 500 million downloads on Google Play), so developers targeting customers in those countries still need to debug their applications in those browsers.

This PR ensures all client files are ES5 only (also enforces this via ESLint parser version).

Does this PR introduce a breaking change?
No.

Other information

@jsf-clabot
Copy link

jsf-clabot commented Dec 21, 2017

CLA assistant check
All committers have signed the CLA.

@codecov
Copy link

codecov bot commented Dec 21, 2017

Codecov Report

Merging #1241 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1241   +/-   ##
=======================================
  Coverage   76.31%   76.31%           
=======================================
  Files           5        5           
  Lines         477      477           
  Branches      154      154           
=======================================
  Hits          364      364           
  Misses        113      113

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a9327e5...8060b25. Read the comment docs.

@shellscape
Copy link
Contributor

@yyx990803 thanks for the PR, I do appreciate the effort. Unfortunately this isn't one we're going to be merging. It's a topic that's been discussed many times in the issues here. The readme outlines when this was done and what users needing to support unsupported platforms should do (use an older version). If a browser doesn't support const and let, webpack-dev-server@2.7.1 should be used. The reasons for not bumping a major are also outlined in the readme, whether or not it was the right call has also been debated. Just as webpack@4 (and subsequent major versions of webpack-family modules) is dropping Node 4 support, older platforms can't be supported forever. We're not leaving those users high-and-dry, they'll just have to lag behind as they're already having to do supporting older platforms.

@shellscape shellscape closed this Dec 21, 2017
@yyx990803
Copy link
Contributor Author

yyx990803 commented Dec 21, 2017

This is extremely irresponsible and overly optimistic about the ES feature set being used around the globe. As I pointed out, UC Browser has 500 million downloads on Google Play which equates to millions of users worldwide. You are essentially shutting down all the developers working in those regions from being able to leverage webpack-dev-server to debug applications in a browser their users choose to use.

To put it in more context, webpack-dev-server is used in the default vue-cli webpack template, and we rely on the latest version to take advantage of the new features, so being stuck on 2.7.1 is not an option.

Using Node 4 as an analogy also doesn't fly - devs have the choice to upgrade their Node versions, but they don't necessarily have control over what browsers their users choose to use.

I don't see any practical downside of using ES5 in the dev-server client code - JavaScript is always backwards compatible, so yes, "older platforms can be supported forever" in this case.

@shellscape
Copy link
Contributor

I understand the argument, and as mentioned, this has been discussed at length in other issues. I would argue it's extremely irresponsible for a browser vendor to continue to distribute a product to 500 million users which is so far beyond modern standards, but there's not much I can say to that effect. These are the same arguments made when China was still predominately on IE6 not long ago. Vendors chose to move forward. At some point the line must be drawn.

If users of WDS are stuck having to support very old (or very behind) browsers, they can use v2.7.1 or use workarounds such as this one.

I also understand the weight your position in the two communities carries, and the water it typically moves. Unfortunately we have a difference of opinion.

@yyx990803
Copy link
Contributor Author

I still don't see a valid argument on why this should not be merged. Can you provide one such argument in prior discussions?

@yoyo837
Copy link
Contributor

yoyo837 commented Dec 21, 2017

I think what they mean is to force the browser and its users to improve, and the browser and its users are always easily turned into negative, backward...

@michael-ciniawsky
Copy link
Member

@shellscape I think this should be revisited since the ES5 compat is a minor change to the code base (mainly let/const => var) and if the user base is big enough (~500 million (claimed)) and folks have issues it's an fair point to consider imho. (e.g can't the client files not be transpiled ?)

@wxsms
Copy link

wxsms commented Dec 21, 2017

Front-end developers can chose Node.js version for themselves, but NOT browsers.

Can't you simply babel your client side ES6 code to ES5? You can still move as fast as you wish without dropping anyone behind!

@shellscape
Copy link
Contributor

shellscape commented Dec 21, 2017

A workaround exists for this issue. The webpack core team has taken up discussion of this. Please use the workaround in the meantime. Locking this thread before it goes bananas.

@webpack webpack locked and limited conversation to collaborators Dec 21, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants