-
Notifications
You must be signed in to change notification settings - Fork 34
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
Fixed the issue #49 #53
base: main
Are you sure you want to change the base?
Conversation
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.
I still have the same feedback as with the other PR. Is the @angular-devkit/build-angular
dependency really necessary?
It pulls in a LOT of dependencies. Checking the size of that dependency only in pkg-size.dev
, shows 196MB.
In comparison, the @angular/cli
pulls far less. This will impact user experience on the Angular starter quite a bit.
@Nemikolh sorry I missed to add this comment. In the latest versions of Angular (18+), the build system has transitioned to an esbuild-based builder as the default (@angular-devkit/build-angular:application). This change significantly reduces dependency overhead and improves build performance. The previous browser and browser-esbuild builders are now deprecated in favor of this unified application builder. I’ve double-checked the @angular-devkit/build-angular dependency, and it’s still required for leveraging the default Angular CLI build pipeline. Let me know if you’d prefer, we take alternative routes, or if further testing is needed for the current setup. |
I think, I will leave this decision to someone from the Angular team. @alxhub if you have time to check this, that would be really appreciated. To reiterate on my concerns, I'm worried that the dependencies added by this PR will impact:
|
I just realized I dealt with a similar thing recently. Too similar... This underlying issue with the builder which prompted me to make the issue that the previous PR was attempting to fix made me remember... it appears to be the same issue as the bug I reported in the angular.dev playground when using in-component adev(bug): "Learn Angular in the Browser" breaks with styles #28925 fix(@angular/build): use sha256 instead of sha-256 as hash algorithm name #28926 I made a fork of my issue recreation Stackblitz project and bumped it to Angular packages version 19.0.5 like the fix PR tried to do, but without changing away from the TL;DR A fix to the |
Thanks, @michael-small and @Nemikolh, for your insights and the references to the related issues and fixes. The background you provided was invaluable in identifying the root cause and verifying the solution. I've bumped the Angular packages version to 19.0.6, as suggested, and confirmed that the issue is resolved with the updated @angular/build:application builder. The necessary changes have been made in this PR. Could you please review the updates at your convenience? Thanks again for your collaboration! 🙌 |
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.
Nice! Thank you both for investigating 🙏
Just a minor thing and we can merge this!
"@angular/build": "^19.0.7", | ||
"@angular/cli": "^19.0.7", |
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.
Let's keep all the dependencies at the same version 👍
No description provided.