DO NOT MERGE: feat(polyfills): add IE-specific shims to angular2-polyfills.js - #7390

This PR is not ready to be merged but it is here to trigger discussion.

The whole story started with @matsko asking me to add a polyfill for function.name (needed for all IE versions). But I've realized that we've got already function.name in https://github.com/angular/angular/blob/master/modules/angular2/src/testing/testing.ts
At the same time @wardbell was postulating addition of https://github.com/angular/angular/blob/master/modules/angular2/src/testing/testing.ts to angular2-polyfills.js so doing what @wardbell is requesting would also solve @matsko issue.

Now, from what I understand we simply can't stick https://github.com/angular/angular/blob/master/modules/angular2/src/testing/testing.ts into files we distribute due to the licensing issues.

I'm opening this PR so we can make some progress on the issue and decide how do we want to deal with browser-specific shims.

AFAIK Igor and Naomi are the most familiar with the topic so assigning to @IgorMinar

@pkozlowski-opensource those shims aren't required by every user, right? Wouldn't be better to leave angular2-polyfills as is right now and make a new one called angular2-ie-polyfills.js or similar? I use Chrome, personally I don't need any of those.

Well, while it is true they are only needed for IE / android, in practical terms what what means is 'it is needed for one desktop browser and one mobile browser'.

Which in turn is the same as 'you need this if you are doing a desktop browser app or a mobile-first app'. Thus this encompasses the main use cases of Angular 2.

So in that sense, it is not optional, but rather a trap: as a developer, if you fail to include this shim in your app, then you forego IE users and Android users.

Off the top of my head, I don't think there are a lot of apps for which this is acceptable. Perhaps intranet style apps, where the company enforces use of a certain browser that is not IE.

There are licensing issues with some of those IE polyfills. We need to sort those out first.

I also don't like that we push these polyfills onto people that don't need them and we will increase the payload size unnecessarily.

As I've mentioned what @matsko asked me for in the first place was the function.name so we should start by sorting out the licensing issue, if any, for this one. But completely agree with concerns of pushing IE-specific code to everyone (and will play the devil advocate by saying that I also see "removing friction" value in it).

As a side note function.name is part of the https://github.com/paulmillr/es6-shim/blob/master/es6-sham.js

Would it make sense to create 3-4 polyfill bundles and then based on which browser people target they'll need to include on of them?

This way people can download as little or as much as they need.

I'd like to echo @filipesilva and @pkozlowski-opensource 's observation: these polyfills are required in almost all realistic environments. Very few application owners can say "We don't care about IE."

IMO opinion, we don't have a choice. We can't ship code that doesn't work in IE. We could but shouldn't tell people to get the shims they need from some place else.

If necessary we can reimplement these shims ourselves, making sufficient changes to negate licensing concerns.

@IgorMinar suggestion is a good one. Let's give people choice. But just as our samples ship the entire RxJS bundle whether you need it or not, so our samples would ship with the most complete bundle whether you need it or not. Samples are about learning, not byte-shaving.

I may have created some confusion inadvertently.

I don't think we have to blend ALL of the shims together. I didn't mean to imply that. I was saying that we should reduce the TWO angular-labeled shims to one, merging https://github.com/angular/angular/blob/master/modules/angular2/src/testing/shims_for_IE.js into angular2-polyfills.js

I was asking about shims in gitter, hope it's OK to add my comments / input here ...

It's not just IE shims - right now I also need to add Intl shim to make Safari work with some of the number / date pipes (https://github.com/andyearnshaw/Intl.js/). It's not small (33Kb?) and like the IE shims I don't really want to add it to the bundle and increase load times for users with browsers that don't need it. Every small extra piece adds up.

I've been experimenting with the polyfill.io service which seems to me to be the best approach - a CDN sends back the polyfills needed for the browser based on features requested (it can add gated checks as well instead of just using the user-agent although that adds some latency). The benefit is that only the 'substandard' browsers do any extra downloading and shims are kept separate to the Angular 2 / App bundle - as browsers are upgraded then already-cached asset bundles continue to work without change.

example:
<script src="https://cdn.polyfill.io/v2/polyfill.min.js?features=Promise,Intl.~locale.en"></script>

The ideal solution for me would be a single script include that indicated the browser features required and did whatever separate shim loading was required whether this was via a separate polyfill-as-a-service or files I could host myself (really comes down to whether you want any reliance on a 3rd party service).

This should also make example code setup easier too and less of a change when things are bundled which is often confusing (different instructions on which order shim files need to be in). I think adding even more variations on shim combinations would be a negative experience and just lead to more confusion and frustration.

@CaptainCodeman I like where you're going .. if it works. I haven't had much luck with browser sniffing in the past and I worry (a little) about coverage and timing issues. But I'd be happy to hear that this works reliably.

@wardbell @CaptainCodeman but this would mean one more HTTP request + processing time on the mentioned service side. We would have to measure what is actually faster.

I wasn't intending to suggest that particular polyfill service (although it seems to work great) but more the approach and the extra Intl polyfill to factor into any consideration.

When it comes to having an extra HTTP request the only way to prevent that is to bundle ALL the possibly required polyfills and taking the file-size hit. That's where the different polyfill combination options would come in for which browsers you wanted to support although if I was going to bundle them all then I could pick and chose for the browsers anyway so providing them would only be a slight convenience.

They have long cache expiration varied by Accept-Encoding and User-Agent so although it's an extra http request it is very cache friendly (either coming from the browser cache or giving a 304) and you get the required polyfills in one hit. The benefit of using a central service is that you increase the chance that the visitor already has it cached.

They do have an option to instead check for polyfills in the browser itself which would be safer than user-agent sniffing but would add an additional request unless the code for that check was built into the bundle itself - to then request the browser-specific polyfill files suggested earlier.

FWIW, we were linking to the wrong file in the testing area. We meant this one: https://github.com/angular/angular/blob/master/modules/angular2/src/testing/shims_for_IE.js

I corrected earlier comments where I could.

I've done a quick demo to show it working:
http://captain-codeman.appspot.com/

Source code:
https://github.com/CaptainCodeman/angular2-webpack-polyfill

Tested with Chrome, Firefox, Safari, Edge, IE 11 and Android browser.

The src/index.html and src/vendor.ts files are probably the key pieces. The src/ie-fix.ts is from here and seems to be Angular 2 specific (or should it be a generally available polyfill?)

This is using Webpack - I know System.JS has extra issue when it comes to the URL shim and IE which could probably be added to the features if that were being used.

@wardbell @pkozlowski-opensource

A couple of extra details - as most (all?) decent browsers will download scripts in parallel and the polyfill is always much smaller than the Angular 2 / app bundle it doesn't seem to me that it will ever add any additional delay to the page, even with a cold cache:

For repeat visits with cached files there is no download delay and the startup time is all about the script parsing:

I wasn't thinking about performance. I was (am) thinking about the extra noise and complexity. But we appreciate the extra information and commitment to discovery.

I don't think performance should be ignored or only thought about later but I was responding to the specific questions about the performance implications of making the request.

The main benefit is simplicity. Right now the quickstart (which should really be "the easiest simplest version"?) has a long and off-putting list of scripts that have to be included and the combination and order of shims required seem to result in quite a few problems going off gitter and github issues.

The ideal solution IMO is that there is one script entry to load polyfills required to bring the browser up to the feature level that angular requires and then one script to load angular itself (or whatever bundle / module loading the app uses).

I think the suggestion to add even more combinations of polyfill options would not make things simpler.

One script beats 2,3 or 4 ... I grant you.

I hear there are licensing issues. Otherwise, we'd have a single quick-and-dirty script and be done with it.

I think I'd call it that to be utterly clear that said script is not for production :)

Maybe we can fight through those legal issues and get where we need to be. I think they may be the only serious impediment to consolidation.

I repeat: performance absolutely should not be a consideration. Simplicity trump all.

Why? Because the choice today has zero effect on the application tomorrow. Not a line of the app will change because we load one kitchen sink script or a thousand hand picked polyfill scripts. If we ever need to fine-tune our polyfill loading, we do one thing here in index.html and we're done. Further discussion about polyfill script perf is bike shedding

I mean "further discussion ... in the context of quick start".

Of course we need to consider script size and loading time in other contexts. But even then we don't need a one size fits all. Some complexity and choice is expected when our attention turns to performance.

My point is that, unlike more difficult performance dilemmas, we don't have to worry about today's choice having repercussions a year from now.

Definitely agree - make it right, then make it fast. Optimal choices for performance may change anyway as HTTP/2 becomes more common.

As I've become more familiar with Angular 2 I've concluded that the first-use experience gives the impression that things are more complicated than they possibly need to be. I'm sure it can only help to simplify things as much as possible.

Some examples I've discovered while playing around with these shims and webpack bundling:

The provided angular2-polyfills dependency is built from zone.js and reflect-metadata so I don't understand why those sources need to be separately added to the project as well. I see them only needed for advanced users building custom versions from scratch and don't need to be imposed for regular / first-time use.

There's only a couple of points (facade/collections) affecting IE that are preventing the es6-shim being completely removed (and just the polyfill service used instead) which would also be another simplification (I'm new to typescript so don't know if the typings dependency could also go).

I could be completely off base about these of course - I'm still learning and just trying to provide feedback from my own "new user experience + confusion" and attempts to create the simplest setup I can that also supports IE and Safari users. I know Angular 2 is work-in-progress so there are likely lots of things that were done one way for specific reasons which are always evolving.

Not a line of the app will change because we load one kitchen sink script or a thousand hand picked polyfill scripts. If we ever need to fine-tune our polyfill loading, we do one thing here in index.html and we're done.

I agree - that's what I'm trying to get to ... no reference to any polyfills within my Angular 2 app, just a single entry in index.html that will gradually load less and less as browsers evolve.

polymer does a good job of documenting the support here : https://www.polymer-project.org/1.0/resources/compatibility.html - IMO we should do something similar.

Please include #6944 if considering bundling the IE shims.

Our latest thinking is that we should document the polyfills rather than provide a bundle for all. Alternative is to use http://cdn.polyfill.io/v2/docs/features/ which will add the cost for only tho browser which need them.

@mhevery Thanks for clarifying the latest strategy on polyfills. It would be good if the documentation be updated to reflect this. For example, right now the date pipe doesn't have this info, and nor does the quickstart example. As well I think if we decide to go with

http://cdn.polyfill.io/v2/docs/features/

it would be nice if the es6-shim, rxjs and zone.js polyfills could be all supported there as well and we can simply rely on that service to polyfill everything we need. If they are not willing to do that, I would consinder cloning their service to have an angular polyfill service similar to theirs with all our polyfill needs so we don't have to have some hardcoded shims in our code like es6-shim and this service as well. The concept is a good one, but it seems we can expand it further.

@mhevery Upon closer inspection it also seems that

http://cdn.polyfill.io/v2/docs/features/

Does not currently polyfill Function.name so while it might be the current strategy it will not address this issue

#6501

So unless I am mistaken if Function.name hasn't been merged somewhere already it is still not fixed.

These comments reveal the challenging nature of this problem and the rapidly shifting solution set. That is why we are still researching rather than switching immediately.

We just switched to core-js because we had substantial evidence that es6-shim was fatally slow in IE11.

Let's learn more before leap.

@wardbell Thanks for your comments. We also had a look at core-js before, since from the discussion we gather things were moving that way. However the problem with the annotation inheritance described here

#5415 (comment)

made us revert back. In that discussion, after my comments there seem to be an hypothesis as whichever one has the wrong implementation whether it be core-js or reflect metadata. We haven't look in that much depth yet, but one think for sure is that core-js is not a drop in replacement for es6-shim/reflect, it will break some code.

The core-js script I added specifically does not add reflect metadata because non IE browsers need it and should be able to opt out of IE support shim

@wardbell Do you have a config, or a link to that script, somewhere and I can try it later with the plunker see if the inheritance behaviour works or not with it.

Golly jeepers! Material Design 2 isn't supporting IE9. So all this shim talk is useless for me now anyway!
I can't live without Material Design!!!

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}