-
Notifications
You must be signed in to change notification settings - Fork 12k
fix(@angular-devkit/build-angular): downlevel libraries based on the browserslist configurations #23185
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
Conversation
2584193
to
5732080
Compare
5732080
to
e6ea05a
Compare
e6ea05a
to
c71832f
Compare
b033dd2
to
3bbc289
Compare
3bbc289
to
93c45fc
Compare
93c45fc
to
86c1e91
Compare
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.
Commit message nit: "fix(@angular-devkit/build-angular): downlevel libraries based on the browserlist configuration"
...s/angular_devkit/build_angular/src/builders/browser/tests/behavior/typescript-target_spec.ts
Outdated
Show resolved
Hide resolved
7feedfa
to
bfe5af8
Compare
…browserslist configurations There is no standard for library authors to ship their library in different ES versions, which can result in vendor libraries to ship ES features which are not supported by one or more browsers that the user's application supports. With this change, we will be downlevelling libraries based on the list of supported browsers which is configured in the browserslist configuration. Previously, we only downlevelled libraries when targeting ES5. The TypeScript target option will not effect how the libraries get downlevelled. Closes angular#23126
bfe5af8
to
5df577b
Compare
I'm a bit confused after this change. I was hoping it would just work, but it doesn't seem like my browser list is being respected. I have it defined in the I previously had the tsconfig set to es5 to output for legacy browsers, and I had a check to make sure the built bundle was es5 to catch any libraries that snuck in which is what this change was meant to address. After upgrading to Angular 14 the migration changed my tsconfig output setting, and since I had previously seen this change I figured it was fine and would just work. However, the es5 check is now complaining and the only way to make sure the build is correct is to set the target back to es5 which triggers the warning that es5 output is deprecated. Is there some configuration option I am missing? I can obviously set the target back to es5 but I'm not sure why there was a migration to change it if this breaks now. Thanks in advance and I can open an issue if needed. |
Indeed ES5 output support is deprecated since Angular no longer supports legacy browsers such as IE 11 which require ES5 syntax. I am not really sure what you mean by Without more information such as a minimal reproduction it's hard wether or not there is an issue. |
@alan-agius4 I may want to just open an issue if more details are needed than this, but the ES5 check I am referring to is using https://github.com/acornjs/acorn after the build step to check if any libraries had slipped in non ES5 features since libraries weren't previously downleveled before this change. Previously our tsconfig output was set to es5, but after updating to Angular 14 and migrating, it changed to es2020. Based on the title of this PR, and what I thought the feature would do, is that the tsconfig output could now be set to es2020 which was previously used internally and all generated output would respect the browserslist configuration. I don't need to strictly check for es5 and I can remove acorn from our build process, but I did notice that the output generated was still using features that were too new for our browserslist configuration and I'm wondering if I'm either not using an option I should be or if I'm misunderstanding what's supposed to happen and I should keep the tsconfig output at es5 until we can remove our need for that downleveling. |
Hi @terencehonles, During the update to version 14 we do indeed update the target to ES2020, this is because all browsers that Angular supports ES2020 features out of the box without the need of any downlevelling or polyfilling features. Prior to this change downlevelling of libraries was only done when targetting ES5. Now downlevelling for libraries always happen. That said this change doesn't effect the TypeScript output when targetting ES2015 or later. Downlevelling of TypeScript sources is only done when targeting ES5 and this is done because various parts of the build only support modern ES syntax. Moving forward in another major version we want to change this and "ignore" the TypeScript target and downlevel ECMA features/syntax based on the configured browserslist configuration even on TypeScript output. |
Ah, ok, so reverting the migration does seem like the correct course of action for us. I thought this was already in effect and why I was commenting.
We unfortunately need ES5 because we currently use wkhtmltopdf to generate PDFs of our pages and it's based on QtWebKit ~= Safari 5. We've not had the time to invest in changing that part of our stack and we're "OK" with everyone seeing ES5 for the time being 😞. We've made progress in cleaning up those pages so that they can be rendered with something like puppeteer instead, but unfortunately all of that work has just been on "spare time" and was never a priority compared to other things we needed to ship. Previously, the v8 differential loading feature was a big factor in why we were "OK" with still having wkhtmltopdf. Our polyfills were probably a bit bloated at that point, but it was something we could live with since we were still in the progress of adopting Angular and our project wasn't that big. Over time more of the application has been converted and we've cleaned up things needed in order to drop super old browsers, but we will still want the ability to target browsers that Angular may not support and that's where we are definitely looking forward to a more comprehensive down leveling step built into the CLI. Thanks for all your responses! |
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
There is no standard for library authors to ship their library in different ES versions, which can result in vendor libraries to ship ES features which are not supported by one or more browsers that the user's application supports.
With this change, we will be downlevelling libraries based on the list of supported browsers which is configured in the browserslist configuration. Previously, we only downlevelled libraries when targeting ES 5.
The TypeScript target option will not effect how the libraries get downlevelled.
Closes #23126