build: update size test tool to work with Angular v13 #23811
Conversation
Adds a little comment explaining why`.mjs` is processed in the Angular esbuild rule.
Updates the size test tool to properly run with the Angular v13 compilation pipeline, matching conceptually with what the Angular CLI performs. i.e. running the Angular linker, running the non-deprecated build-optimizer variant (i.e. the Babel plugins).
devversion
added
merge safe
target: minor
google-cla
bot
added
the
cla: yes
|
|
||
| /** | ||
| * ESBuild plugin configuring various optimization Babel plugins. The Babel plugins | ||
| * configured as part of this plugin run in the Angular CLI compilation pipeline as well. |
There was a problem hiding this comment.
I think that this is going to go out of sync with the CLI eventually. Wouldn't it be better to run an actual CLI build?
There was a problem hiding this comment.
It's true that this can become outdated in some cases, but usually the JS optimization pipeline in the CLI does not change too often and we just need to update this in some cases (like for v13).
The framework also runs tests like that without the CLI because a CLI project is much harder to integrate with size trackings in Bazel, and it wouldn't allow us to have such fine-grained and isolated scenario tests (notice how we have various scenarios like drag drop basic, advanced etc.)
There was a problem hiding this comment.
Subjectively I also think that a pipeline like that allows for easier debugging. In the CLI it is a little more difficult to follow what caused a problem, while here it's a rather simple script allowing for easier/better reasoning and manual edits for debugging (extremely helpful when I compared MDC sizes)
There was a problem hiding this comment.
Is the ease of debugging really relevant when we can't trust that the setup is the same as for real CLI users?
I don't see why we wouldn't be able to have the same granularity in the test scenarios either. We could get away with defining a single CLI workspace and then each test scenario would be a project within the same angular.json. That would allow us to build everything with a single command and we might not even need to go through Bazel.
Also I believe that the FW repo has CLI-based size tests that run against AIO.
There was a problem hiding this comment.
Is the ease of debugging really relevant when we can't trust that the setup is the same as for real CLI users?
It definitely is, at least for me. I added these size checks initially to debug the lightweight token Ivy issue, and also used it to compare MDC sizes. For that, it was significantly easier inspecting the non-Webpack convoluted bundle, while I could also disable certain optimizations in a fine-grained way. i.e. disabling mangling, beautifying output, keeping pure calls etc. Some of this is also do-able with the CLI but only with some manual edits of the node modules (some things can also be controlled with unknown/internal environment variables though)
We could get away with defining a single CLI workspace and then each test scenario would be a project within the same angular.json
Yeah, but this is significantly more code to maintain, starting with the generation of all the other unused parts of a CLI application, and having to maintain the angular.json file..
and we might not even need to go through Bazel.
We heavily invested in running everything in Bazel, especially in the FW repo. Going through Bazel makes it easier to run these things consistently on all platforms (in a hermetic way). The major benefit is also that the required NPM package output can be added as dependency from Bazel. This removes the need of copying package output outside of Bazel + enables caching, and remote execution.
Also I believe that the FW repo has CLI-based size tests that run against AIO.
yeah, they have, but it's significantly more difficult to maintain these within Bazel (there are a few CLI Bazel size tests in FW). A large problem would also be that all size tests run whenever something changes, while with the current approach only affected tests re-run. Lastly, it would require even more work to capture sizes if there are multi-projects (for all the scenarios) in a single CLI workspace.
Happy to discuss this further. also cc. @josephperrott for this
Generally I'd also prefer to rely as much as possible on the CLI here, but we kind of do leverage as much as possible with my current approach, without all the downsides of using the CLI directly
There was a problem hiding this comment.
Anyway, I think the discussion of whether this should run with the CLI or not is not in-scope for this PR. We can/should discuss this more though.
For now, this is low-maintenace and makes our size tests work properly for v13.
|
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. |
See individual commits.
This also brought the form-field size increase down again. The new sizes are now matching closely with what a CLI app would see. They vary slightly due to some of the Webpack chunk generation.