angular / angular-cli

CLI tool for Angular
https://cli.angular.io
MIT License
26.77k stars 11.97k forks source link

NG 8 IE 11 & Differential Loading #16653

Closed vespertilian closed 3 years ago

vespertilian commented 4 years ago

🐞 bug report

Affected Package

The issue is caused by package @angular/ ... not sure cli? it's a build issue

Description

So there are a few things to track here:

Specifically in IE11

If I build the linked project with the tsconfig.json target "es5" everything works. (baseline).

If I build the project with tsconfig.json target "es2015" a few things do not work as I expect:

  1. It builds both es2015 and es5 bundles. Strangely IE11 will load both the es2015 and the es5 bundles, when I expected it to just load the es5 bundle. Chrome correctly loads just the es2015 bundle.
ie11-loading-both-bundles
  1. The es5 bundle created when differential loading is enabled is larger than the es5 bundle created when ts.config is set to es5 and differential loading is off. In my small sample app the differences are not that large but in our production app the difference is substantial. I would have thought that the es5 differential loading bundle would be no bigger than the non differential loading use case.

Differential loading

ng8-ie11-app-diff-loading

No differential loading

ng8-ie11-no-d-loading

From the above images you can see that the main bundle for the base ES5 scenario is 282 but for the differential loading its 298. Again this is a small sample app, in our production app this is more pronounced.

  1. The bundle created produces errors when running in IE11, loading ESRI maps. Again I find this strange as I would have assumed the es5 differential loading bundle would be equivalent to the bundle created when setting ts.config to es5, which works fine.
ie11-error

πŸ”¬ Minimal Reproduction

https://github.com/vespertilian/ng8-ie11

build with npm run build serve the dist build with ng run serve-dist (might need to install http-server)

change ts.config from es5 to es2015 to build the different packages.

πŸ”₯ Exception or Error

Just for the third case


ERROR Error: Uncaught (in promise): TypeError: Object expected

🌍 Your Environment

Angular Version:



Angular CLI: 8.3.22
Node: 12.14.0
OS: darwin x64
Angular: 8.2.14
... animations, common, compiler, compiler-cli, core, forms
... language-service, platform-browser, platform-browser-dynamic
... router

Package                           Version
-----------------------------------------------------------
@angular-devkit/architect         0.803.21
@angular-devkit/build-angular     0.803.21
@angular-devkit/build-optimizer   0.803.21
@angular-devkit/build-webpack     0.803.21
@angular-devkit/core              8.3.21
@angular-devkit/schematics        8.3.22
@angular/cli                      8.3.22
@ngtools/webpack                  8.3.21
@schematics/angular               8.3.22
@schematics/update                0.803.22
rxjs                              6.4.0
typescript                        3.5.3
webpack                           4.39.2

Anything else relevant?

destus90 commented 4 years ago
  1. Check out the official docs (alert number 2) or this issue.
  2. Is there any difference between the images? Seems like you have uploaded the same image twice.
vespertilian commented 4 years ago

@destus90 Thank you for your reply.

2- Yes I uploaded the same image twice :grin: (fixed)

1- I see from your link on the angular site it does mention:

"Some legacy browsers still download both bundles, but only execute the appropriate scripts based on the attributes mentioned above."

That's a little disappointing, we just moved to Angular 8 and differential loading seemed to be one of the killer features, reading @StephenFluin blog post I was excited. The thing is that most of our users are running modern browsers, it's just IE11 that's the issue for corporate clients and I feel like this is where differential loading would shine. However it looks like for our clients this is the reality:

ie-bundle-size

Could we not also just wrap the ES2015 bundle in something like:

`

    <!--<![endif]-->

`

I feel like this is a solvable problem for the whole Angular community, I also feel like IE11 should be the focus of differential loading as most of the other browsers are evergreen and can all use the 2015 feature set.

3 -

Still the biggest issue for me is that the es5 bundle (non differential loading works) but the es5 bundle (when differential loading is turned on) does not work. But they should be the same es5 bundle, it's just that we have a second bundle for modern browsers, right?

The most upvoted "How do I make Angular 8 compatible with IE11?" answer on stack overflow is to disable differential loading, but it should work with IE11 and differential loading, so I feel like others have this issue maybe I am just the first one to raise it.

zero-1 commented 4 years ago

linked the wrong issue id in prev edit... the build breaking in IE11 -- please check https://github.com/angular/angular-cli/issues/16366

alan-agius4 commented 4 years ago

Can you try to update @angular-devkit/build-angular to 0.803.22 please?

With regards to the conditional html comments ie: <!--[if !IE]--> these are not supported from IE 10 onwards.

zero-1 commented 4 years ago

Tried the attached demo project with @angular-devkit/architect 0.803.22 Still able to reproduce the problem....

Downgrading to ... @angular-devkit/build-angular": "0.803.19 fixes the issue......

This still leaves the bigger issue of both packages being downloaded on IE 11 It seems the choices are ... 1) go with ES2015. Reduce the download size for modern browser while doubling(almost) it for IE 11. 2) Or... go with ES5

vespertilian commented 4 years ago

@alan-agius4

Maybe we could use something like:

https://stackoverflow.com/questions/29987969/how-to-load-a-script-only-in-ie

<script type="text/javascript">
    if(/MSIE \d|Trident.*rv:/.test(navigator.userAgent))
        document.write('<script src="../nolng/js/ex1IE.js"><\/script>
        <script src="../nolng/js/ex2IE.js"><\/script>');
    else
        document.write('<script src="../nolng/js/ex1.js"><\/script>
       <script src="../nolng/js/ex2.js"><\/script>');
</script>

to prevent downloading both bundles in IE11.

clydin commented 4 years ago

The current method used for execution bifurcation was chosen after research into a variety of methods. There are indeed tradeoffs for some older browsers (IE in particular). However, as best explained in this comment from one of the Chrome team members, the real world affect of the double download is generally minimal to non-existent. This is in contrast to the effect of the method in the above comment (as well as others), which can have large performance issues for modern browsers especially on mobile platforms.

If the user base of a particular application is predominantly using IE, targeting ES5 may be the best option for the application. It is also possible to post process the generated index to alter the differential loading method if testing reveals non-ideal performance metrics for a given application's usage environment. Another option would be using a server generated index per request and opens up potential additional avenues regarding attempts at browser detection guided script selection.

vespertilian commented 4 years ago

@clydin thanks for the link. Web development = complexity!

Your argument about desktop vs mobile does make sense. For us, currently, I don't think we can choose almost a double initial bundle for IE and other older browsers to get faster performance in modern browsers, as our bundle is embarrassingly large :grin: . Hopefully, this will change in the future.

I feel like the marketing for this feature missed the mark a bit.

The statement "When users load your application, they’ll automatically get the bundle they need." from Version 8 of Angular β€” Smaller bundles, CLI APIs, and alignment with the ecosystem is not super accurate. As you pointed out it's really a compromise, and while modern browsers get better smaller bundles and parse performance older browsers initial bundle size is almost doubled, I know the docs do make a cursory reference to this but it might be worthwhile to make this tradeoff more prominent, for anyone else with embarrassingly large bundles, as I thought I could just drop this in and win!

I don't think we will be using this feature for the next couple of months until our initial bundle is not embarrassingly large (working on it), but we should still work to fix issues 2 and 3 for others :smiley: .

Thanks again.

maxisam commented 4 years ago

well, tried 0.803.19 and .25

It throws error like Object doesn't support property or method 'includes'

michaelz commented 4 years ago

@maxisam The polyfills are automatically loaded now for all the Angular code. If your own code contains some additional code that is unsupported by IE, you still need to add the polyfill.

In my case, i had to add the following imports (using core-js 3). You need to find out which ones you need.

import 'core-js/features/array/includes';
import 'core-js/features/array/values';
import 'core-js/features/array/entries';
import 'core-js/features/object/values';
import 'core-js/features/object/entries';
maxisam commented 4 years ago

@michaelz

Thanks! I just noticed that as well. And I also noticed that if we import extra code like import 'core-js/features/array/includes';

It shows up in both es2015 and es5 polyfills.

I think Angular should put what polyfills it includes in the document somewhere.

Here is the es5 polyfills used in Angular

https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/build_angular/src/angular-cli-files/models/es5-polyfills.js

michaelz commented 4 years ago

@maxisam , Because it shows up in both es2015 and es5 polyfills, I created a new custom polyfills-ie.ts file that I manually bundle with webpack and manually insert in the html with the "nomodule" mention. Users with a recent browser shouldn't pay for the ineptitude of IE users/managers to update.

Rockstar311 commented 4 years ago

I have the same problem with loading ESRI maps on es5 working ok but on es2015 I get TypeError

rplotkin commented 4 years ago

I'm reporting here that this is an issue that impacted the functionality of our app. IE11 was loading both bundles, and a login button would not become enabled (no idea why that button was impacted, but it was an immediate and obvious error). I'm serving via Express, so upon server start I have express create a version of index.html without es2015 references, and then with the (mostly reliable) express-useragent npm module I serve that up as index to IE11. Problem went away. NOTE: this was ng9. For ng8 we were doing es5 builds, and I wanted to stop that.

dgp1130 commented 4 years ago

Regarding the three points raised in the original post:

  1. @clydin pointed out here that some IE builds will double download both versions, but only execute one of them.
  2. Changing tsconfig.json to ES5 only applies for application code. Library code may include ES2015, and altering tsconfig.json won't downlevel that. When differential loading is enabled, we downlevel the entire build to ES5, so the bundle size increases as a result. This is why there is often a bundle size difference between ES5 without differential loading and ES5 with differential loading.
  3. Because library code is not downleveled from modifying tsconfig.json (per (2)), library code with ES2015 features may break on IE. I suspect this is the root cause for your error, since you mention it seems to work when using differential loading. We typically recommend that you use browserslist to control differential loading behavior, rather than modifying tsconfig.json for this reason (our docs have more info on this, but are not very clear about this specific foot-gun and we're working on refactoring them to be more helpful in this regard).

I'm not able to confirm (3) as I don't have IE on-hand atm. If that is accurate, then it seems like this is all WAI, just some confusion around the target attribute in tsconfig.json and how ES5 with differential loading differs from ES5 without differential loading. Please let me know if there is something else I'm missing here.

/cc @aikidave as FYI for future improvements to docs (points (2) and (3) in particular don't seem to be covered anywhere).

rplotkin commented 4 years ago

@dpg1130

  1. @clydin pointed out here that some IE builds will double download both versions, but only execute one of them.

Something I don't want to see lost, though, is that, with the default ng9 configs, IE11 was failing to run the application correctly when the es2015 bundles were being downloaded. Even though it was also downloading the es5 bundles. This isn't just a performance hit I don't want to take, but there's something going on that's more hinky than IE11 just ignoring the es2015 contents (and suppressing those files for IE11 fixed the issue).

clydin commented 4 years ago

@rplotkin Can you provide the content of the index.html file that was failing (that contained both bundle sets), or even better, a reproduction? It might also be easier to investigate if a separate issue was opened (please include full version information as well).

A browser will not execute a script if the type attribute is an unsupported value. For differential loading, all ES2015 scripts have a type set to module. This method of execution bifurcation is used extensively for this use case; including in other web frameworks (Vue.js modern mode, for instance).

ckrasi-old commented 3 years ago

@maxisam , Because it shows up in both es2015 and es5 polyfills, I created a new custom polyfills-ie.ts file that I manually bundle with webpack and manually insert in the html with the "nomodule" mention. Users with a recent browser shouldn't pay for the ineptitude of IE users/managers to update.

Can you share how did you do it? I have similar approach - need to support IE11, but care only about es2015 bundle size.

michaelz commented 3 years ago

@maxisam , Because it shows up in both es2015 and es5 polyfills, I created a new custom polyfills-ie.ts file that I manually bundle with webpack and manually insert in the html with the "nomodule" mention. Users with a recent browser shouldn't pay for the ineptitude of IE users/managers to update.

Can you share how did you do it? I have similar approach - need to support IE11, but care only about es2015 bundle size.

Sure. First I wrote a polyfills-ie.ts that contains some imports from core-js, in /src. Then I found a small webpack configuration file like this somewhere on the internet which I adapted for my needs:

const path = require('path');

module.exports = {
    mode: 'production',
    entry: './src/polyfills-ie.ts',
    output: {
        path: path.resolve(__dirname, 'src'),
        filename: './polyfills-ie.[hash].bundle.js'
    },
    resolve: {
        // Add `.ts` and `.tsx` as a resolvable extension.
        extensions: ['.ts', '.tsx', '.js']
    },
    module: {
        rules: [
            // all files with a `.ts` or `.tsx` extension will be handled by `ts-loader`
            {
                test: /\.ts$/, loader: 'ts-loader', options: {
                    configFile: 'tsconfig.es5.json'
                }
            }
        ]
    }
};

Then I generate my js file like this:

webpack --config small-webpack-config.js

Then I get the js file somewhere with a hash, say, polyfills-ie.hashhash.js that I set as an asset in angular.json so that it gets copied, then I load it in the index.html file like that:

<head>
...
    <script nomodule="" src="/polyfills-ie.hashhash.bundle.js"></script><!-- IE ONLY -->
</head>

The hash is useful for versionning and caching.

ckrasi-old commented 3 years ago

@michaelz thx for quick response! I thought that this can be incorporated into angular build process, but having it prepared before is also doable. I am happy as long as it work as intended.

michaelz commented 3 years ago

@ckrasi-acx I didn't do it because this file doesn't change that much, and it takes a really long time to compile for some reason... but it's doable ! Our jenkins pipeline already a lot of time and I didn't want to add a couple of minutes to it.

alan-agius4 commented 3 years ago

This issue is now obsolete since differential loading will be removed in the next major version. See: https://github.com/angular/angular-cli/pull/21467

angular-automatic-lock-bot[bot] commented 3 years ago

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.