feat(language-service): add perf tracing for #getSemanticDiagnostics() #1
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This commit updates the logic that validates contributors.json data and introduces a new check that verifies that profile images don't exceed specified limit. PR Close angular#41253
…angular#41254) Previously presence of both [class] and [className] bindings on an element was treated as compiler error (implemented in angular@6f203c9). Later, the situation was improved to actually allow both bindings to co-exist (see angular@a153b61), however the compiler check was not removed completely. The only situation where the error is thrown at this moment is when static (but with interpolation) and bound `class` attributes are present on an element, for ex.: ``` <div class="{{ one }}" [class]="'two'"></div> ``` In the current situation the error is acually misleading (as it refers to `[className]`). This commit removes the mentioned compiler check as obsolete and makes the `class` and `style` attribute processing logically the same (the last occurrence is used to compute the value). PR Close angular#41254
…lectors (angular#41261) When there was more than one rule in a single style string, only the first rule was having its `:host` selector processed correctly. Now subsequent rules will also be processed accurately. Fixes angular#41237 PR Close angular#41261
Specify a date format for the reviewed tag and provided an example. PR Close angular#41269
…1274) With this change we valid breaking changes descriptions as per our contribution guidelines See: https://github.com/angular/angular/blob/88fbc066775ab1a2f6a8c75f933375b46d8fa9a4/CONTRIBUTING.md#commit-message-footer PR Close angular#41274
Removing commit message builder and wizard as they are unused and unneeded. PR Close angular#41280
alxhub
force-pushed
the
ngtsc/perf/tracing
branch
from
a3cb9b4 to
f862827
Compare
In angular#41253 the size of contributor images was limited, but some images were already too large. So an exclusion list was added. These images have now been reduced, so the exclusion list is no longer needed. The files were reduced by a combination of running them through the https://tinyjpg.com/ online service and manually setting their size to 168px wide or tall using the MacOS Image Preview app. PR Close angular#41292
The terminology use in the comments did not match the actual types being referred to. PR Close angular#41119
…ias to `unknown` (angular#41119) These types are only used in the generated typings files to provide information to the Angular compiler in order that it can compile code in downstream libraries and applications. This commit aliases these types to `unknown` to avoid exposing the previous alias types such as `ɵɵDirectiveDef`, which are internal to the compiler. PR Close angular#41119
…lar#41119) Th `ɵɵFactoryDef` type will appear in published libraries, via their typings files, to describe what type dependencies a DI factory has. The parameters on this type are used by tooling such as the Language Service to understand the DI dependencies of the class being created by the factory. This commit moves the type to the `public_definitions.ts` file alongside the other types that have a similar role, and it renames it to `ɵɵFactoryDeclaration` to align it with the other declaration types such as `ɵɵDirectiveDeclaration` and so on. PR Close angular#41119
…orDef` in compiled output (angular#41119) The `ɵɵInjectorDef` interface is internal and should not be published publicly as part of libraries. This commit updates the compiler to render an opaque type, `ɵɵInjectorDeclaration`, for this instead, which appears in the typings for compiled libraries. PR Close angular#41119
ActivatedRoute.fragment was typed as Observable<string> but could emit both null and undefined due to incorrect non-null assertion. These non-null assertions have been removed and fragment has been retyped to string | null. BREAKING CHANGE: Strict null checks will report on fragment potentially being null. Migration path: add null check. Fixes angular#23894, fixes angular#34197. PR Close angular#37336
…ngular#41059) Currently, when importing `BrowserAnimationsModule`, Angular uses `AnimationRenderer` as the renderer. When the root view is removed, the `AnimationRenderer` defers the actual work to the `TransitionAnimationEngine` to do this, and the `TransitionAnimationEngine` doesn't actually remove the DOM node, but just calls `markElementAsRemoved()`. The actual DOM node is not removed until `TransitionAnimationEngine` "flushes". Unfortunately, though, that "flush" will never happen, since the root view is being destroyed and there will be no more flushes. This commit adds `flush()` call when the root view is being destroyed. BREAKING CHANGE: DOM elements are now correctly removed when the root view is removed. If you are using SSR and use the app's HTML for rendering, you will need to ensure that you save the HTML to a variable before destorying the app. It is also possible that tests could be accidentally relying on the old behavior by trying to find an element that was not removed in a previous test. If this is the case, the failing tests should be updated to ensure they have proper setup code which initializes elements they rely on. PR Close angular#41059
This commit removes some contributor images that are no longer referenced in `contributors.json` (i.e. they belong to contributors that have since been removed). BTW, removing these unused images saves ~720KB off the total size of the assets that are deployed along with the app. PR Close angular#41290
When a contributor was removed from `contributors.json`, the corresponding image should also be removed from `aio/content/images/bios/`. However, this was often overlooked, resulting in unused images remaining in `aio/content/images/bios/`. This commit adds a check to ensure that all images in `aio/content/images/bios/` are referenced in `contributors.json`. PR Close angular#41290
…mal (angular#41072) The Ivy Language Service uses the compiler's template type-checking engine, which honors the configuration in the user's tsconfig.json. We recommend that users upgrade to `strictTemplates` mode in their projects to take advantage of the best possible type inference, and thus to have the best experience in Language Service. If a project is not using `strictTemplates`, then the compiler will not leverage certain type inference options it has. One case where this is very noticeable is the inference of let- variables for structural directives that provide a template context guard (such as NgFor). Without `strictTemplates`, these guards will not be applied and such variables will be inferred as 'any', degrading the user experience within Language Service. This is working as designed, since the Language Service _should_ reflect types exactly as the compiler sees them. However, the View Engine Language Service used its own type system that _would_ infer these types even when the compiler did not. As a result, it's confusing to some users why the Ivy Language Service has "worse" type inference. To address this confusion, this commit implements a suggestion diagnostic which is shown in the Language Service for variables which could have been narrowed via a context guard, but the type checking configuration didn't allow it. This should make the reason why variables receive the 'any' type as well as the action needed to improve the typings much more obvious, improving the Language Service experience. Fixes angular/vscode-ng-language-service#1155 Closes angular#41042 PR Close angular#41072
…lar#41092) Adds a migration that casts the value of `ActivatedRouteSnapshot.fragment` to be non-nullable. Also moves some code from the `AbstractControl.parent` migration so that it can be reused. Relates to angular#37336. PR Close angular#41092
add a link from github doc on how to fork a repo. this will enhance user experience for users new to github by putting the information on how to fork a repo at their finger tips without them having to do additional work to search for it. PR Close angular#41266
… using TS 4.2 (angular#41305) TypeScript 4.2 has changed its emitted syntax for synthetic constructors when using `downlevelIteration`, which affects ES5 bundles that have been downleveled from ES2015 bundles. This is typically the case for UMD bundles in the APF spec, as they are generated by downleveling the ESM2015 bundle into ES5. ngcc needs to detect the new syntax in order to correctly identify synthesized constructor functions in ES5 bundles. Fixes angular#41298 PR Close angular#41305
… using TS 4.2 (angular#41305) TypeScript 4.2 has changed its emitted syntax for synthetic constructors when using `downlevelIteration`, which affects ES5 bundles that have been downleveled from ES2015 bundles. This is typically the case for UMD bundles in the APF spec, as they are generated by downleveling the ESM2015 bundle into ES5. The reflection capabilities in the runtime need to recognize this new form to correctly deal with synthesized constructors, as otherwise JIT compilation could generate invalid factory functions. Fixes angular#41298 PR Close angular#41305
Format the HTML used in browser-support.md such that the tables render correctly under GitHub markdown. PR Close angular#41122
…ng (angular#41286) Use conventional-commits-parser for parsing commits for validation, this is being done in anticipation of relying on this parser for release note creation. Unifying how commits are parsed will provide the most consistency in our tooling. PR Close angular#41286
Some `elements` tests rely on `window.customElements` being available. On browsers where this was not present, the tests were skipped. This commit includes the `@webcomponents/custom-elements` polyfill in order to be able to run all `elements` tests on older browsers, which do not natively support Custom Elements. This, also, fixes the [saucelabs_ivy][1] and [saucelabs_view_engine][2] CI jobs (part of the `monitoring` workflow), which have been failing recently on IE 11 (probably due to the update to TS 4.2.3). [1]: https://circleci.com/gh/angular/angular/944291 [2]: https://circleci.com/gh/angular/angular/944289 PR Close angular#41324
alxhub
force-pushed
the
ngtsc/perf/tracing
branch
2 times, most recently
from
be71f52 to
314f36a
Compare
Add an entry for Zach Arend to the contributors.json file. PR Close angular#41325
…ngular#41125) ngtsc has an internal performance tracing package, which previously has not really seen much use. It used to track performance statistics on a very granular basis (microseconds per actual class analysis, for example). This had two problems: * it produced voluminous amounts of data, complicating the analysis of such results and providing dubious value. * it added nontrivial overhead to compilation when used (which also affected the very performance of the operations being measured). This commit replaces the old system with a streamlined performance tracing setup which is lightweight and designed to be always-on. The new system tracks 3 metrics: * time taken by various phases and operations within the compiler * events (counters) which measure the shape and size of the compilation * memory usage measured at various points of the compilation process If the compiler option `tracePerformance` is set, the compiler will serialize these metrics to a JSON file at that location after compilation is complete. PR Close angular#41125
A previous commit implemented a streamlined performance metric reporting system for the compiler-cli, controlled via the compiler option `tracePerformance`. This commit adds a custom Bazel flag rule //packages/compiler-cli:ng_perf to the repository, and wires it through to the `ng_module` implementation such that if the flag is set, `ng_module` will produce perf results as part of the build. The underlying mechanism of `//:ng_perf` is not exported from `@angular/bazel` as a public rule that consumers can use, so there is little risk of accidental dependency on the contents of these perf traces. An alias is added so that `--ng_perf` is a Bazel flag which works in our repository. PR Close angular#41125
zarend
force-pushed
the
perf-in-logs
branch
3 times, most recently
from
4360ae5 to
977b9b0
Compare
Adds perf tracing for the public methods in LanguageService. If the log level is verbose or higher, trace performance results to the tsServer logger. This logger is implemented on the extension side in angular/vscode-ng-language-service.
alxhub
pushed a commit
that referenced
this pull request
Oct 31, 2023
…ngular#52414) With the directive-based control flow users were able to conditionally project content using the `*` syntax. E.g. `<div *ngIf="expr" projectMe></div>` will be projected into `<ng-content select="[projectMe]"/>`, because the attributes and tag name from the `div` are copied to the template via the template creation instruction. With `@if` and `@for` that is not the case, because the conditional is placed *around* elements, rather than *on* them. The result is that content projection won't work in the same way if a user converts from `*ngIf` to `@if`. These changes aim to cover the most common case by doing the same copying when a control flow node has *one and only one* root element or template node. This approach comes with some caveats: 1. As soon as any other node is added to the root, the copying behavior won't work anymore. A diagnostic will be added to flag cases like this and to explain how to work around it. 2. If `preserveWhitespaces` is enabled, it's very likely that indentation will break this workaround, because it'll include an additional text node as the first child. We can work around it here, but in a discussion it was decided not to, because the user explicitly opted into preserving the whitespace and we would have to drop it from the generated code. The diagnostic mentioned point #1 will flag such cases to users. Fixes angular#52277. PR Close angular#52414
alxhub
pushed a commit
that referenced
this pull request
Nov 2, 2023
…ngular#52414) With the directive-based control flow users were able to conditionally project content using the `*` syntax. E.g. `<div *ngIf="expr" projectMe></div>` will be projected into `<ng-content select="[projectMe]"/>`, because the attributes and tag name from the `div` are copied to the template via the template creation instruction. With `@if` and `@for` that is not the case, because the conditional is placed *around* elements, rather than *on* them. The result is that content projection won't work in the same way if a user converts from `*ngIf` to `@if`. These changes aim to cover the most common case by doing the same copying when a control flow node has *one and only one* root element or template node. This approach comes with some caveats: 1. As soon as any other node is added to the root, the copying behavior won't work anymore. A diagnostic will be added to flag cases like this and to explain how to work around it. 2. If `preserveWhitespaces` is enabled, it's very likely that indentation will break this workaround, because it'll include an additional text node as the first child. We can work around it here, but in a discussion it was decided not to, because the user explicitly opted into preserving the whitespace and we would have to drop it from the generated code. The diagnostic mentioned point #1 will flag such cases to users. Fixes angular#52277. PR Close angular#52414
alxhub
pushed a commit
that referenced
this pull request
May 16, 2024
…lar#55747) The first test asserts that bubbling does not work right now. The second asserts that stopPropagation works, which should pass when test #1 passes too. The third test asserts properties about the events passed to the event handler. THe fourth test asserts that mouse events do not translate to jsaction nor help emit the jsaction binary. This required a change in code to make this pass. PR Close angular#55747
alxhub
pushed a commit
that referenced
this pull request
Jun 12, 2024
…lar#55747) The first test asserts that bubbling does not work right now. The second asserts that stopPropagation works, which should pass when test #1 passes too. The third test asserts properties about the events passed to the event handler. THe fourth test asserts that mouse events do not translate to jsaction nor help emit the jsaction binary. This required a change in code to make this pass. PR Close angular#55747
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This branch is a work-in-progress yet, and it is not ready to merge. Opening a draft PR for the diff and to run the tests...
Adding perf tracing for the language service. The language service logs perf tracing to the angular server log when the log level is set to verbose.
Here's how I've been testing it:
As you can see, the logs can easily be grep'd for the perf tracing.