fix https://github.com/angular/components/issues/21674
When setting `ngZoneRunCoalescing` to true, `onStable` is not emitted correctly.
The reason is before this commit, the code looks like this
```
// Application code call `ngZone.run()`
ngZone.run(() => {}); // step 1
// Inside NgZone, in the OnInvoke hook, NgZone try to delay the checkStable()
function delayChangeDetectionForEvents(zone: NgZonePrivate) {
if (zone.lastRequestAnimationFrameId !== -1) { // step 9
return;
}
zone.lastRequestAnimationFrameId = zone.nativeRequestAnimationFrame.call(global, () => { // step 2
if (!zone.fakeTopEventTask) {
zone.fakeTopEventTask = Zone.root.scheduleEventTask('fakeTopEventTask', () => {
zone.lastRequestAnimationFrameId = -1; // step 3
updateMicroTaskStatus(zone); // step 4
checkStable(zone); // step 6
}, undefined, () => {}, () => {});
}
zone.fakeTopEventTask.invoke();
});
updateMicroTaskStatus(zone);
}
function updateMicroTaskStatus(zone: NgZonePrivate, ignoreCheckRAFId = false) {
if (zone._hasPendingMicrotasks ||
((zone.shouldCoalesceEventChangeDetection || zone.shouldCoalesceRunChangeDetection) &&
zone.lastRequestAnimationFrameId !== -1)) { // step 5
zone.hasPendingMicrotasks = true;
} else {
zone.hasPendingMicrotasks = false;
}
}
function checkStable(zone: NgZonePrivate) {
if (zone._nesting == 0 && !zone.hasPendingMicrotasks && !zone.isStable) { // step 7
try {
zone._nesting++;
zone.onMicrotaskEmpty.emit(null);
...
}
// application ref subscribe onMicroTaskEmpty
ngZone.onMicroTaskEmpty.subscribe(() => {
ngZone.run(() => { // step 8
tick();
});
});
```
And the process is:
1. step 1: application call ngZone.run()
2. step 2: NgZone delay the checkStable() call in a requestAnimationFrame, and also set
zone.lastRequestAnimationFrameId
3. step 3: Inside the requestAnimationFrame callback, reset zone.lastRequestAnimationFrameId first
4. step 4: update microTask status
5, step 5: if zone.lastRequestAnimationFrameId is -1, that means no microTask pending.
6. step 6: checkStable and trigger onMicrotaskEmpty emitter.
7. step 7: ApplicationRef subscribed onMicrotaskEmpty, so it will call another `ngZone.run()` to process
tick()
8. step 8: And this new `ngZone.run()` will try to check `zone.lastRequestAnimationFrameId` in `step 9`
when trying to delay the checkStable(), and since the zone.lastRequestAnimationFrameId is already reset
to -1 in step 3, so this ngZone.run() will run into step 2 again.
9. And become a infinite loop..., so onStable is never emit
In this commit, the `zone.lastRequestAnimationFrameId` reset is moved after `checkStable()` call.
PR Close#40540
They aim to improve code readability.
Since they are defined by `const enum` they have zero runtime performance impact
over just using constant literals.
Fixes#23543
PR Close#23548
When parsing interpolations, if we encounter an empty interpolation
(`{{}}`), the current code uses a "pretend" value of `$implicit` for the
name as if the interplotion were really `{{$implicit}}`. This is
problematic because the spans are then incorrect downstream since they
are based off of the `$implicit` text.
This commit changes the interpretation of empty interpolations so that
the text is simply an empty string.
Fixes https://github.com/angular/vscode-ng-language-service/issues/1077
Fixes https://github.com/angular/vscode-ng-language-service/issues/1078
PR Close#40583
The codebase currently contains several `EMPTY_ARRAY` constants,
and they can end up in the bundle of an application.
A recent commit 6fbe219 tipped us off
as it introduced several `noop` occurrences in the golden symbol files.
After investigating with @petebacondarwin,
we decided to remove the duplicated symbols.
This probably shaves only a few bytes,
but this commit removes the duplicated functions,
by always using the one in `core/src/utils/empty`.
PR Close#40587
If the template parse option `leadingTriviaChars` is configured to
consider whitespace as trivia, any trailing whitespace of an element
would be considered as leading trivia of the subsequent element, such
that its `start` span would start _after_ the whitespace. This means
that the start span cannot be used to mark the end of the current
element, as its trailing whitespace would then be included in its span.
Instead, the full start of the subsequent element should be used.
To harden the tests that for the Ivy parser, the test utility `parseR3`
has been adjusted to use the same configuration for `leadingTriviaChars`
as would be the case in its production counterpart `parseTemplate`. This
uncovered another bug in offset handling of the interpolation parser,
where the absolute offset was computed from the start source span
(which excludes leading trivia) whereas the interpolation expression
would include the leading trivia. As such, the absolute offset now also
uses the full start span.
Fixes#39148
PR Close#40513
This commit adds a new `IncrementalResourceCompilationTicket` which reuses
an existing `NgCompiler` instance and updates it to optimally process
template-only and style-only changes. Performing this update involves both
instructing `DecoratorHandler`s to react to the resource changes, as well as
invalidating `TemplateTypeChecker` state for the component(s) in question.
That way, querying the `TemplateTypeChecker` will trigger new TCB generation
for the changed template(s).
PR Close#40561
To prepare for the optimization of template-only changes, this commit
refactors the `ComponentDecoratorHandler`'s handling of template parsing.
Previously, templates were extracted from the raw decorator metadata and
parsed in a single operation.
To better handle incremental template updates, this commit splits this
operation into a "declaration" step where the template info is extracted
from the decorator metadata, and a "parsing" step where the declared
template is read and parsed. This allows for re-reading and re-parsing of
the declared template at a future point, using the same template declaration
extracted from the decorator.
PR Close#40561
Previously, the incremental flow for NgCompiler was simple: when creating a
new NgCompiler instance, the consumer could pass state from a previous
compilation, which would cause the new compilation to be performed
incrementally. "Local" information about TypeScript files which had not
changed would be passed from the old compilation to the new and reused,
while "global" information would always be recalculated.
However, this flow could be made more efficient in certain cases, such as
when no TypeScript files are changed in a new compilation. In this case,
_all_ information extracted during the first compilation is reusable. Doing
this involves reusing the previous `NgCompiler` instance (the container for
such global information) and updating it, instead of creating a new one for
the next compilation. This approach works cleanly, but complicates the
lifecycle of `NgCompiler`.
To prevent consumers from having to deal with the mechanics of reuse vs
incremental steps of `NgCompiler`, a new `CompilationTicket` mechanism is
added in this commit. Consumers obtain a `CompilationTicket` via one of
several code paths depending on the nature of the incoming compilation, and
use the `CompilationTicket` to obtain an `NgCompiler` instance. This
instance may be a fresh compilation, a new `NgCompiler` for an incremental
compilation, or an existing `NgCompiler` that's been updated to optimally
process a resource-only change. Consumers can use the new `NgCompiler`
without knowledge of its provenance.
PR Close#40561
The Language Service uses the source span of AST nodes to recognize which
node a user has selected, given their cursor position in a template. This is
used to trigger autocompletion.
The previous source span of BindingPipe nodes created a problem when:
1) the pipe binding had no identifier (incomplete or in-progress expression)
2) the user typed trailing whitespace after the pipe character ('|')
For example, the expression `{{foo | }}`. If the cursor preceded the '}' in
that expression, the Language Service was unable to detect that the user was
autocompleting the BindingPipe expression, since the span of the BindingPipe
ended after the '|'.
This commit changes the expression parser to expand the span of BindingPipe
expressions with a missing identifier, to include any trailing whitespace.
This allows the Language Service to correctly recognize this case as
targeting the BindingPipe and complete it successfully. The `nameSpan` of
the BindingPipe is also moved to be right-aligned with the end of any
whitespace present in the pipe binding expression.
This change allows for the disabled test in the Language Service for pipe
completion in this case to be re-enabled.
PR Close#40346
The `LanguageServiceAdapter` must implement `realpath` in order to resolve
symlinks in `node_modules`.
Local libraries are often symlinked in `node_modules` by adding a local
dependency in `package.json`.
Fix https://github.com/angular/vscode-ng-language-service/issues/1083
PR Close#40593
Previously, in `_mismatch()`, the `DefaultIterableDiffer` first checks
`_linkedRecords` for `itemTrackBy`, then checks `_unlinkedRecords`.
This cause the `DefaultIterableDiffer` to move "later" items that match the
`itemTrackBy` from the old collection, rather than using the "earlier" one.
Now we check `_unlinkedRecords` first, so that the `DefaultIterableDiffer`
can give a more stable and reasonable result after diffing. For example,
rather than (`a1` and `a2` have same trackById)
```
a1 b c a2 => b a2 c a1
```
we get
```
a1 b c a2 => b a1 c a2
```
where a1 and a2 retain their original order despite both
having the same track by value.
Fixes#23815
PR Close#23941
Updates to rules_nodejs@2.3.3 to take advantage of windows specific fixes.
rules_nodejs@2.3.3 was created as a patch specifically with a fix for
the issues we found updating to rules_nodejs@2.2.2.
PR Close#40581
Update to the latest version of bazel.
`4.0.0` introduced a breaking change on unnecessary backslashes and these
instance are corrected in this change.
PR Close#40579
This is a follow up fix for
894286dd0c.
It turns out that comments can be closed in several ways:
- `<!-->`
- `<!-- -->`
- `<!-- --!>`
All of the above are valid ways to close comment per:
https://html.spec.whatwg.org/multipage/syntax.html#comments
The new fix surrounds `<` and `>` with zero width space so that it
renders in the same way, but it prevents the comment to be closed eagerly.
PR Close#40525
Previously, we were naïvely checking whether a function name was a partial linker
declaration call by testing the map of linkers with `linkers[name]`. Since
`linkers` was a plain object, it also matched function names like `toString`!
This has been refactored as a `Map` to avoid the problem.
PR Close#40563
This PR adds a way for the language server to retrieve compiler options
diagnostics via `languageService.getCompilerOptionsDiagnostics()`.
This will be used by the language server to show a prompt in the editor if
users don't have `strict` or `fullTemplateTypeCheck` turned on.
Ref https://github.com/angular/vscode-ng-language-service/issues/1053
PR Close#40423
Prior to this commit, the `patchValue()` of the `FormGroup` and `FormArray` classes used to throw an exception
when the `value` argument contained a data structure that has `null` or `undefined` as a value for a field
that represents an instance of `FormGroup` or `FormArray` (for `FormControl` it's not a problem, since it
doesn't have nested controls), since the `patchValue()` method tried to iterate over provided values to
match current data structure.
This commit updates the `patchValue()` logic in `FormGroup` and `FormArray` classes to just ignore `null` and
`undefined` values (without any changes to corresponding `FormGroup` and `FormArray` instances). This
behavior looks inline with the `patchValue()` method goal of "doing its best to match the values to the
correct controls" (quote from docs).
Fixes#36672.
Fixes#21021.
PR Close#40534
PR #39235 introduced additional cleanup logic for form controls and directives. The cleanup logic relies
on the presence of ControlValueAccessor instances on FormControlName and FormControl directives. In general
these fields are present and there are also checks to make sure that the mentioned directive instances are
created with CVAs. However some scenarios (primarily tests) may invoke the logic in a way that the directive
instance would not be fully initialized, thus causing CVA to be absent. As a result, the cleanup logic fails
while trying to call some methods on associated CVA instances.
This commit updates the cleanup logic to take into account the situation when CVA is not present.
Fixes#40521.
PR Close#40526
The ARB format doesn't have a dedicated field for message meaning so these changes include it
as a customize attribute called `x-meaning`.
Fixes#40506.
PR Close#40546
The compliance test runner has various macros that process the
expectation files before actually checking their contents. Among those
macros are i18n helpers, which uses a global message counter to be able
to uniquely identify ICU variables.
Because of the global nature of this message index, it was susceptible
to ordering issues which could result in flaky tests, although it failed
very infrequently.
This commit resets the global message counter before applying the macros.
As a result of this change an expectation file had to be updated; this
is actually a bug fix as said test used to fail if run in isolation (if
`focusTest: true` was set for that particular testcase).
PR Close#40529
When migrating zone.js from gulp to bazel, some legacy build config files are still there,
we have `rollup-es5.config.js` and `rollup-es5_global-es2015.config.js`, since in gulp build
system, build `es5` or `esm` files are set in the config file, but in the bazel world,
the output format is not config in the config.js file, but is required by the downstream
bazel target. So we don't really need the two rollup config files any longer.
Another difference is in `rollup-es5.config.js`, the `external` and `global` libraries names
are also config there, and these settings are also valid for `es2015` build, these settings
are not in the `es2015.config.js` for some legacy reasons. So we don't need to keep this
difference either.
PR Close#40481
This commit reverts commit [_fix(service-worker): handle error with
ErrorHandler_](https://github.com/angular/angular/pull/39990/commits/552419d).
With Angular v11.0.4 and commit [_fix(service-worker): handle error with
ErrorHandler_](https://github.com/angular/angular/pull/39990/commits/552419d)
Angular start to send all service worker registration errors to the Angular
standard `ErrorHandler#handleError()` interface, instead of logging them in the
console.
But users existing `ErrorHandler#handleError()` implementations are not adapted
to service worker registration errors and it might result in broken apps or
bad UI.
Passing to `ErrorHandler` is desirable for some and undesirable for others and
the same is true for passing to `console.error()`.
But `console.error()` was used for a long time and thus it is preferable to keep
it as long as a good solution is not found with `ErrorHandler`.
Right now it's hard to define a good solution for `ErrorHandler` because:
1. Given the nature of the SW registration errors (usually outside the control
of the developer, different error messages on each browser/version, often
quite generic error messages, etc.), passing them to the `ErrorHandler` is
not particularly helpful.
2. While `ErrorHandler#handleError()` accepts an argument of type `any` (so
theoretically we could pass any object without changing the public API), most
apps expect an `Error` instance, so many apps could break if we changed the
shape.
3. Ideally, the Angular community want to re-think the `ErrorHandler` API
and add support for being able to pass additional metadata for each error
(such as the source of the error or some identifier, etc.). This change,
however, could potentially affect many apps out there, so the community must
put some thought into it and design it in a way that accounts for the needs
of all packages (not just the SW).
4. Given that we want to more holistically revisit the `ErrorHandler` API, any
changes we make in the short term to address the issue just for the SW will
make it more difficult/breaky for people to move to a new API in the future.
To see the whole explanation see GitHub PR #40236.
PR Close#40236
This is a follow up fix for
894286dd0c.
It turns out that comments can be closed in several ways:
- `<!-->`
- `<!-- -->`
- `<!-- --!>`
All of the above are valid ways to close comment per:
https://html.spec.whatwg.org/multipage/syntax.html#comments
The new fix surrounds `<` and `>` with zero width space so that it
renders in the same way, but it prevents the comment to be closed eagerly.
PR Close#40525
In `ViewEncapsulation.Emulated` mode, the compiler must generate additional
combinations of selectors to handle the `:host-context()` pseudo-class function.
Previously, when there is was more than one `:host-context()` selector in a
rule, the compiler was generating invalid selectors.
This commit generates all possible combinations of selectors needed to
match the same elements as the native `:host-context()` selector.
Fixes#19199
PR Close#40494
When a source-map has an inline source, any source-map linked from
that source should only be loaded if itself is also inline; it should not
attempt to load a source-map from the file-system. Otherwise we can
find ourselves with inadvertent infinite cyclic dependencies.
For example, if a transpiler takes a file (e.g. index.js) and generates
a new file overwriting the original file - capturing the original
source inline in the new source-map (index.js.map) - the source
file loader might read the inline original file (also index.js) and
then try to load the `index.js.map` file from disk - ad infinitum.
Note that the first call to `loadSourceFile()` is special, since you can
pass in the source-file and source-map contents directly as in-memory
strrngs. This is common if the transpiler has just generated these and has
not yet written them to disk.
When the contents are passed into `loadSourceFile()` directly, they are
not treated as "inline" for the purposes described above since there is
no chance of these "in-memory" source and source-map contents being caught
up in a cyclic dependency.
Fixes#40408
PR Close#40435
This patch adds an API to retrieve the template typecheck block for a
template (if any) at a file location, and a selection of the TS node
in the TCB corresponding to the template node at which the request for
a TCB was made (if any).
Probably not something we want to land soon, but a useful debugging tool
for folks working with TCBs.
PR Close#39974
rxjs was only used within one location within the static queries migration to workaround
a previous limitation that schematics could not directly use a promise. However, promise
support has been available since 8.0. This change removes the observable promise wrapping.
It also removes an any cast that was previously needed to workaround rxjs version mismatches
during compilation.
PR Close#38657
Adds an `appendAll()` method to `HttpParams` that can construct the HTTP
request/response body from an object of parameters and values.
This avoids calling `append()` multiple times when multiple parameters
need to be added.
Fixes#20798
PR Close#20930
For the Google Cloud Console within Google we observed errors in the
shallowEqual function for users in IE and Edge. This patch was made within
Google and the errors went away. This commit upstreams the change into Angular.
PR Close#40488
This commits adds additional expectations to verify that the bloom
filter is able to correctly handle token IDs that exceed the size of
the bloom filter (which is currently 256 bits).
PR Close#40489
The injector system uses a bloom filter to determine if a token is
possibly defined in the node injector tree, which is stored across
multiple bloom buckets that each represent 32 bits of the full 256-bit
wide bloom hash. This means that a computation is required to determine
the exact bloom bucket which is responsible for storing any given 32-bit
interval, which was previously computed using three bitmask operations
and three branches to derive the bloom bucket offset.
This commit exploits the observation that all bits beyond the low 5 bits
of the bloom hash are an accurate representation for the bucket offset,
if shifted right such that those bits become the least significant bits.
This reduces the three bitmask operations and three branches with a
single shift operation, while additionally offering a code size
improvement.
PR Close#40489
Rather than mutating the span on the template when renaming literal strings,
this commit updates the logic to mutate the `TextSpan` equivalent that
is used by the Language Service.
PR Close#40484
Fix a case where matrix parameters weren't stringified when they are passed as a first command
when creating a url tree. Fix return type in parseMatrixParams method
because it always returns {[key: string]: string}
Closes#23165
PR Close#25095
Many `ts.LanguageService` APIs accept a filename, for example
```ts
getQuickInfoAtPosition(fileName: string, position: number)
```
The requirement is that `fileName` is agnostic to the platform (Linux, Mac,
Windows, etc), and is always normalized to TypeScript's internal
`NormalizedPath`.
This is evident from the way these APIs are called from the language server:
```ts
private onHover(params: lsp.TextDocumentPositionParams) {
const lsInfo = this.getLSAndScriptInfo(params.textDocument);
if (lsInfo === undefined) {
return;
}
const {languageService, scriptInfo} = lsInfo;
const offset = lspPositionToTsPosition(scriptInfo, params.position);
const info = languageService.getQuickInfoAtPosition(scriptInfo.fileName, offset);
// ...
}
```
9fca9c6651/server/src/session.ts (L594)
Here `scriptInfo.fileName` is always a `ts.server.NormalizedPath`.
However, https://github.com/angular/angular/pull/39917 accidentally leaked
the platform-specific paths, and caused a mismatch between the incoming paths
and the paths stored in the internal data structure `fileToComponent`.
This PR fixes the bug by always normalizing the paths, and updating the
type to reflect the format of the underlying data.
Fixes https://github.com/angular/vscode-ng-language-service/issues/1063
PR Close#40492
The initial implementation assumed that the consuming editors would
de-duplicate rename locations. In fact, vscode treats overlapping rename
locations as distinct and errors when trying to preview the renames.
This commit updates the language service to de-duplicate exact file+span
matches before returning rename and reference locations.
While vscode _does_ de-duplicate reference results, it still makes sense
to de-duplicate them on our side when possible to make tests more
understandable. If a template has 3 instances of a variable, it makes
sense to get get 3 reference results rather than 4+ with some duplicates.
PR Close#40454
The current "go to definition" is broken for template variables and
references when a template is overridden. This is because we get the
file url from the source span, which uses the overridden name
'override.html'. Instead, we can retrieve the template file from the
compiler in the same manner that is done for references.
Another way to fix this would have been to use the real template file path when
overriding a template, but this was the more straightforward fix since
the strategy was already used in find references and rename locations.
fixes https://github.com/angular/vscode-ng-language-service/issues/1054
PR Close#40455
When a form is reset, it goes through `_forEachChild` to call `reset` on each of its children.
The problem is that if a control is removed while the loop is running (e.g. by a subscription),
the form will throw an error, because it built up the list of available control before the loop
started.
These changes fix the issue by adding a null check before invoing the callback.
Fixes#33401.
PR Close#40462
Close#40215
`fesm2015/zone.js` is built to `esm` bundle with rollup, so the 'use strict';
statement is not generated in the bundle, even we put the 'use strict' in the src code,
rollup removes the code in the final bundle.
So if we load the `fesm2015/zone.js` as a module, such as `ng serve`, in the index.html
```
<script src="polyfills.js" type="module"></script>
```
Everything works fine, since polyfills.js is loaded as `module`, so it is always `strict`.
But in `ng test`, webpack concat the `zone.js` and loaded into the karma html. For other app and
test code, they are still `strict` since they are `module` because they have `export/import`
statement, but `zone.js` is a bundle without `export`, it is a `side effect` bundle, so after
loaded by webpack, it becomes non-strict. Which causes some issues, such as #40215,
the root cause is the `this` context should be `undefined` but treated as `Window` in `non-strict` mode.
```
Object.prototype.toString.apply(undefined);
// should be [object undefined], but it is [object Window] in non-strict mode.
// zone.js patched version of toString
Object.prototype.toString = function() {
...
// in non-strict mode, this is Window
return originalObjectPrototypeToString.call(this);
}
```
So in this commit, `'use strict';` is always added to the `esm` bundles.
PR Close#40456
The `getRenameInfo` action is used by consumers to
1. Determine if a location is a candidate for renames
2. Determine what text to use as the starting point for the rename
PR Close#40439
We should provide the completion when the cursor is in the attribute
name after the `@` and `animate-`, but now the `KeySpan` starts from the
`@` or `animate-`. For example, the animation event `(@name.done)="v"`,
we can know where the cursor is by the `KeySpan` of `name.done` exactly,
it's in the event name or in the phase name.
PR Close#40347
Reasons for change:
- css_parser, css_ast, and css_lexer are not used anywhere and there are
no entry points from compiler.ts
- tested by building Angular and building/running aio with build-local
PR Close#37463
When loading a module that doesn't provide `RouterModule.forChild()` preloader will get stuck
in an infinite loop and throw `ERROR Error: Maximum call stack size exceeded.`
The issue is that child module's `Injector` will look to its parent `Injector` when it doesn't
find any `ROUTES` so it will return routes for it's parent module instead. This will load the
child again that returns its parent's routes and so on.
Closes#29164
PR Close#36605
This commit lays the groundwork for potentially providing rename
locations from the Ivy native LS. The approach is very similar to what
was done with the feature to find references. One difference, however,
is that we did not require the references to be fully "correct". That
is, the exact text spans did not matter so much, as long as we provide a
location that logically includes the referenced item.
An example of a necessary difference between rename locations and references is
directives. The entire element in the template is a "reference" of the
directive's class. However, it's not a valid location to be renamed. The
same goes for aliased inputs/outputs. The locations in the template
directly map to the class property, which is correct for references, but
would not be correct for rename locations, which should instead map to
the string node fo the alias.
As an initial approach to address the aforementioned issues with rename
locations, we check that all the rename location nodes have the same text. If
_any_ node has text that differs from the request, we do not return any
rename locations. This works as a way to prevent renames that could
break the the program by missing some required nodes in the rename action, but
allowing other nodes to be renamed.
PR Close#40140
Because the query now has `flags` which specify the mode, the static query
instruction can now be remove. It is simply normal query with `static` flag.
PR Close#40091
Previous implementation would fire changes `QueryList.changes.subscribe`
whenever the `QueryList` was recomputed. This resulted in artificially
high number of change notifications, as it is possible that recomputing
`QueryList` results in the same list. When the `QueryList` gets recomputed
is an implementation detail and it should not be the thing which determines
how often change event should fire.
This change introduces a new `emitDistinctChangesOnly` option for
`ContentChildren` and `ViewChildren`.
```
export class QueryCompWithStrictChangeEmitParent {
@ContentChildren('foo', {
// This option will become the default in the future
emitDistinctChangesOnly: true,
})
foos!: QueryList<any>;
}
```
PR Close#40091
When using the `NewEntryPointWriter`, we must copy over all files from the
entry-point bundle to the new entry-point. But since we are going to
write out the modified files directly, there is no need to copy those.
This commit skips copying the files that have been modified.
PR Close#40429
When using the `NewEntryPointWriter` we copy unmodified files over to the new
entry-point in addition to writing out the source files that are processed by ngcc.
But we were not copying over associated source-map files for these unmodified
source files, leading to warnings in downstream tooling.
Now we will also copy over source-maps that reside as siblings of unmodified
source files. We have to make sure that the sources of the source-map point
to the correct files, so we also update the `sourceRoot` property of the copied
source-map.
Fixes#40358
PR Close#40429
This commit updates the logic that calculates `useFactory` function arguments to avoid relying on `instanceof`
checks (thus always retaining symbols) and relies on flags that DI decorators contain (as a monkey-patched property).
Another perf benefit is having less megamorphic reads while calculating args for the `useFactory` call: we used to
check whether a token has `ngMetadataName` property 4 times (in worst case), now we have just 1 megamorphic read in
all cases.
Closes#40143.
PR Close#40145
Report non-template diagnotics when calling `getDiagnotics` function of
the language service we only returned template diagnotics. This change
causes it to return all diagnotics, not just diagnostics from the
template type checker.
PR Close#40331
This commit fixes a bug in the **View Engine** implementation of
`getSemanticDiagnostics` and `getDefinitionAndBoundSpan` for node in the
decorator metadata that represents an external URL
(`templateUrl` or `styleUrls`).
The URL could be either relative or absolute, but the latter was not taken
into account.
Fix https://github.com/angular/vscode-ng-language-service/issues/1055
PR Close#40406
The `template` and `isInline` fields were previously stored in a nested
object, which was initially done to accommodate for additional template
information to support accurate source maps for external templates. In
the meantime the source mapping has been accomplished in a different
way, and I feel this flattened structure is simpler and smaller so is
preferable over the nested object. This change also makes the `isInline`
property optional with a default value of `false`.
PR Close#40383
The parser has a list of tag definitions that it uses when parsing the template. Each tag has a
`contentType` which tells the parser what kind of content the tag should contain. The problem is
that the browser has two separate `title` tags (`HTMLTitleElement` and `SVGTitleElement`) and each
of them has to have a different `contentType`, otherwise the parser will throw an error further down
the pipeline.
These changes update the tag definitions so that each tag name can have multiple content types
associated with it and the correct one can be returned based on the element's prefix.
Fixes#31503.
PR Close#40259
Do not warn that navigation was triggered outside Angular zone if the
Router was created outside Angular zone in the first place.
Closes#25837
PR Close#25839
Previously, the SW would wait to become idle before executing scheduled
tasks (including checks for newer app versions). It was considered idle
when it hadn't received any request for at least 5 seconds. As a result,
if the app performed polling (i.e. sent requests to the server) in a
shorter than 5 seconds interval, the SW would never detect and update to
a newer app version.
Related issue: #40207
This commit fixes this by adding a max delay to `IdleScheduler` to
ensure that no scheduled task will remain pending for longer than the
specified max delay.
PR Close#40234
This commit refactors `Driver#deleteAllCaches()` to use `Array#map()`
instead of `Array#reduce()` for running async operations in parallel.
This allows avoiding having to recursively wrap Promises with
`Promise.all()`.
PR Close#40234
Previously, clients were notified about updates sequentially. This
wasn't necessary.
This commit changes the `Driver#notifyClientsAboutUpdate()` method to
notify the clients in parallel (by switching from `Array#reduce()` to
`Array#map()` and `Promise.all()`).
This also aligns the `notifyClientsAboutUpdate()` method with the
`notifyClientsAboutUnrecoverableState()` method.
PR Close#40234
Previously, the `Driver#notifyClientsAboutUnrecoverableState()` method
would not wait for the completion of the promises created to notify the
clients. Theoretically, this could result in the SW instance's getting
destroyed by the browser before all clients have been notified. This is
extremely unlikely to happen in practice, since the async operations are
very quick, but it _is_ theoretically possible.
This commit ensures that the SW instance will remain alive while
notifying the clients by making `notifyClientsAboutUnrecoverableState()`
await the notification promises.
PR Close#40234
The decorator downleveling transform patches `ts.EmitResolver.isReferencedAliasDeclaration`
to prevent elision of value imports that occur only in a type-position, which would
inadvertently install the patch repeatedly for each source file in the program.
This could potentially result in a stack overflow when a very large number of files is
present in the program.
This commit fixes the issue by ensuring that the patch is only applied once.
This is also a slight performance improvement, as `isReferencedAliasDeclaration`
is no longer repeatedly calling into all prior installed patch functions.
Fixes#40276
PR Close#40374
Previously, if there were path-mapped entry-points, where one contaied the
string of another - for example `worker-client` and `worker` - then the
base paths were incorrectly computed resulting in the wrong package path
for the longer entry-point. This was because, when searching for a matching
base path, the strings were tested using `startsWith()`, whereas we should
only match if the path was contained in a directory from a file-system
point of view.
Now we not only check whether the target path "starts with" the base path
but then also whether the target path is actually contained in the base path
using `fs.relative()`.
Fixes#40352Fixes#40357
PR Close#40376
`Object.values` is not supported in IE11 without a polyfill. The quickest,
most straightfoward fix for this is to simply use `Object.keys` instead.
We may want to consider including the polyfill in the CLI in the future
or just wait until IE11 support is dropped before using
`Object.values`.
PR Close#40370
Currently the language service has to force `compileNonExportedClasses` to
`true` to handle inline NgModules in tests, regardless of the value in user's
tsconfig.json.
However, the override is not reinstated after the compiler option changes
(triggered by a change in tsconfig.json).
This commit fixes the bug.
PR Close#40364
This commit documents how to add a helper function which combines all the params
in the router state tree into a single object. It provides a starting point for
developers to reference if they require a more fine-tuned approach.
Fixes#11023
PR Close#40306
In #37182 the in-memory-web-api module was moved into this repo.
Copy the reamde into this repo with the following changes:
* Removed Travis badges
* Updated github links to point to Angular repo
* Removed 'running tests' as it is no longer relevant
Fixes#40190
PR Close#40203
When we attach a `ViewRef` to a `ViewContainerRef`, we save a reference to the container
onto the `ViewRef` so that we can remove it when the ref is destroyed. The problem is
that if the container's `hostView` is destroyed first, the `ViewRef` has no way of knowing
that it should stop referencing the container.
These changes remove the leak by not saving a reference at all. Instead, when a `ViewRef`
is destroyed, we clean it up through the `LContainer` directly. We don't need to worry
about the case where the container is destroyed before the view, because containers
automatically clean up all of their views upon destruction.
Fixes#38648.
PR Close#40219
The `NgControlStatusGroup` directive is shared between template-driven and reactive form modules. In cases when
only reactive forms module is present, the `NgControlStatusGroup` directive is still activated on all `<form>`
elements, but if there is no other reactive directive applied (such as `formGroup`), corresponding `ControlContainer`
token is missing, thus causing exceptions (since `NgControlStatusGroup` directive relies on it to determine the
status). This commit updates the logic to handle the case when no `ControlContainer` is present (effectively making
directive logic a noop in this case).
Alternative approach (more risky) worth considering in the future is to split the `NgControlStatusGroup` into
2 directives with different set of selectors and include them into template-driven and reactive modules separately.
The downside is that these directives might be activated simultaneously on the same element (e.g. `<form>`),
effectively doing the work twice.
Resolves#38391.
PR Close#40344
This class is refactored to extend the new `NodeJSReadonlyFileSystem`
which itself extends `NodeJSPathManipulation`. These new classes allow
consumers to create file-systems that provide a subset of the full file-system.
PR Close#40281
Now that `ReadonlyFileSystem` and `PathManipulation` interfaces are
available, this commit updates the localize package to use these more
focussed interfaces.
PR Close#40281
Now that `ReadonlyFileSystem` and `PathManipulation` interfaces are
available, this commit updates the compiler-cli to use these more
focussed interfaces.
PR Close#40281
This interface now extends `ReadonlyFileSystem` which in turn
extends `PathManipulation`. This means consumers of these
interfaces can be more specific about what is needed, and so
providers do not need to implement unnecessary methods.
PR Close#40281
The "monitoring" workflow has been failing since #40127 was merged,
due to a Saucelabs test failure in Internet Explorer 11. The issue is
with the test's expectation which does not account for Ivy instruction
invocations to use "anonymous" instead of the instruction's function
name. This commit changes the test expectation to also accept
"anonymous", which was already the case for similar expectations.
PR Close#40342
Now when the animation trigger output event is missing its phase value name, the `BoundEvent` will be ignored,
but it's useful for completion in language service.
PR Close#39925
This commit adds special handling to the completion builder by detecting
a two way binding context and ensuring that we filter out any `Input`s
that do not support two way binding.
PR Close#40185
Rather than expecting that a position in a template only targets a
single node, this commit simply adjusts the approach to account for two way
bindings. Specifically, we attempt to get references for each targeted
node and then return the combination of all results, or `undefined` if
none of the target nodes had references.
PR Close#40185
Rather than expecting that a position in a template only targets a
single node, this commit adjusts the approach to account for two way
bindings. In particular, we attempt to get definitions for each targeted
node and then return the combination of all results, or `undefined` if
none of the target nodes had definitions.
PR Close#40185
Adjust the visitor logic of the template target as well as the
consumption of the visitor result to account for two-way bindings.
This sets up downstream consumers for being able to handle the
possibility of a template position that targets both an input and an
output.
PR Close#40185
The current template target implementation only allows a way to
represent the template position as targeting a single node in the
template AST. However, there is at least one case (banana-in-a-box)
where a given template position refers to two template targets.
This commit expands the contexts that the `TemplateTarget` can return to
include support for the banana-in-a-box syntax, which has two logically
targetted AST nodes given a position within the `keySpan` of the
binding.
PR Close#40185
This commit fixes the Template Type Checker's `getSymbolOfNode` so that
it is able to retrieve a symbol for the `BoundEvent` of a two-way
binding. Previously, the implementation would locate the node in the TCB
for the input because it appeared first and shares the same `keySpan` as
the event binding. To fix this, the TCB node search now verifies that
the located node matches the expected name for the output subscription:
either `addEventListener` for a native listener or the class member of the Angular `@Output`
in the case of an Angular output, as would be the case for two-way
bindings.
PR Close#40185
Currently when analyzing the metadata of a directive, we bundle together the bindings from `host`
and the `HostBinding` and `HostListener` together. This can become a problem later on in the
compilation pipeline, because we try to evaluate the value of the binding, causing something like
`@HostBinding('class.foo') public true = 1;` to be treated the same as
`host: {'[class.foo]': 'true'}`.
While looking into the issue, I noticed another one that is closely related: we weren't treating
quoted property names correctly. E.g. `@HostBinding('class.foo') public "foo-bar" = 1;` was being
interpreted as `classProp('foo', ctx.foo - ctx.bar)` due to the same issue where property names
were being evaluated.
These changes resolve both of the issues by treating all `HostBinding` instance as if they're
reading the property from `this`. E.g. the `@HostBinding('class.foo') public true = 1;` from above
is now being treated as `host: {'[class.foo]': 'this.true'}` which further down the pipeline becomes
`classProp('foo', ctx.true)`. This doesn't have any payload size implications for existing code,
because we've always been prefixing implicit property reads with `ctx.`. If the property doesn't
have an identifier that can be read using dotted access, we convert it to a quoted one (e.g.
`classProp('foo', ctx['is-foo']))`.
Fixes#40220.
Fixes#40230.
Fixes#18698.
PR Close#40233
This commit changes the `PartialComponentLinker` to use the original source
of an external template when compiling, if available, to ensure that the
source-mapping of the final linked code is accurate.
If the linker is given a file-system and logger, then it will attempt
to compute the original source of external templates so that the final
linked code references the correct template source.
PR Close#40237
Now, if a source-mapping compliance test fails, the message displays both
the path to the generated file, and more helpfully the path to the expected
file.
PR Close#40237
Previously the names of the source and expectation files were often reused,
which caused potential confusion.
There is now a single source file for
each test-case, which is important when they are being compiled with different
compiler options, since the GOLDEN_PARTIAL file will only contain one copy
per file name.
The names of the expectation files have now been changed so that is clearer
which test-case they are related to.
PR Close#40237
The filename of the source-span is now added to the Babel location
when setting the source-map range in the `BabelAstHost`.
Note that the filename is only added if it is different to the main file
being processed. Otherwise Babel will generate two entries in its
generated source-map.
PR Close#40237
When a source-map/source-file tree has nodes that refer to the same file, the
flattened source-map rendering was those files multiple times, rather than
consolidating them into a single source-map source.
PR Close#40237
When partially compiling a component with an external template, we must
synthesize a new AST node for the string literal that holds the contents of
the external template, since we want to source-map this expression directly
back to the original external template file.
PR Close#40237
`Object.entries` is not supported in IE11 without a polyfill. The quickest,
most straightfoward fix for this is to simply use `Object.keys` instead.
We may want to consider including the polyfill in the CLI in the future
or just wait until IE11 support is dropped before using
`Object.entries`.
PR Close#40340
This commit ensures that the template type checker returns symbols for
all outputs if a template output listener binds to more than one.
PR Close#40144
During route activation, a componentless route will not have a context created
for it, but the logic continues to recurse so that children are still
activated. This can be seen here:
362f45c4bf/packages/router/src/operators/activate_routes.ts (L151-L158)
The current deactivation logic does not currently account for componentless routes.
This commit adjusts the deactivation logic so that if a context cannot
be retrieved for a given route (because it is componentless), we
continue to recurse and deactivate the children using the same
`parentContexts` in the same way that activation does.
Fixes#20694
PR Close#40196
We need a means to preserve typecheck files when a project is reloaded,
otherwise the Ivy compiler will throw an error when it's unable to find
them. This commit implements `getExternalFiles()` called by the langauge
server to achieve this goal.
For more info see https://github.com/angular/vscode-ng-language-service/issues/1030
PR Close#40162
`ts.server.ServerHost.resolvePath()` is different from Angular's
`FileSystem.resolve()` because the signature of the former is
```ts
resolvePath(path: string): string; // ts.server.ServerHost
```
whereas the signature of the latter is
```ts
resolve(...paths: string[]): AbsoluteFsPath; // FileSystem on compiler-cli
```
The current implementation calls `path.join()` to concatenate all the input
paths and pass the result to `ts.server.ServerHost.resolvePath()`, but doing
so results in filenames like
```
/foo/bar/baz/foo/bar/baz/tsconfig.json
```
if both input paths are absolute.
`ts.server.ServerHost` should not be used to implement the
`resolve()` method expected by Angular's `FileSystem`.
We should use Node's `path.resolve()` instead, which will correctly collapse
the absolute paths.
Fix https://github.com/angular/vscode-ng-language-service/issues/1035
PR Close#40242
When resolving references, the Ivy compiler has a few strategies it could use.
For relative path, one of strategies is [`RelativePathStrategy`](
https://github.com/angular/angular/blob/master/packages/compiler-cli/src/
ngtsc/imports/README.md#relativepathstrategy). This strategy
relies on `compilerOptions.rootDir` and `compilerOptions.rootDirs` to perform
the resolution, but language service only passes `rootDirs` to the compiler,
and not `rootDir`.
In reality, `rootDir` is very different from `rootDirs` even though they
sound the same.
According to the official [TS documentation][1],
> `rootDir` specifies the root directory of input files. Only use to control
> the output directory structure with --outDir.
> `rootDirs` is a list of root folders whose combined content represent the
> structure of the project at runtime. See [Module Resolution documentation](
> https://www.typescriptlang.org/docs/handbook/
> module-resolution.html#virtual-directories-with-rootdirs)
> for more details.
For now, we keep the behavior between compiler and language service consistent,
but we will revisit the notion of `rootDir` and how it is used later.
Fixangular/vscode-ng-language-service#1039
[1]: https://www.typescriptlang.org/docs/handbook/compiler-options.html
PR Close#40243
CSS supports escaping in selectors, e.g. writing `.foo:bar` will match an element with the
`foo` class and `bar` pseudo-class, but `.foo\:bar` will match the `foo:bar` class. Our
shimmed shadow DOM encapsulation always assumes that `:` means a pseudo selector
which breaks a selector like `.foo\:bar`.
These changes add some extra logic so that escaped characters in selectors are preserved.
Fixes#31844.
PR Close#40264
In some browsers, notably a mobile version of webkit on iPad, the
result of calling `DOMParser.parseFromString()` returns a document
whose `body` property is null until the next tick of the browser.
Since this is of no use to us for sanitization, we now fall back to the
"inert document" strategy for this case.
Fixes#39834
PR Close#40107
The `ɵɵngDeclareComponent` calls are designed to be translated to fully
AOT compiled code during a build transform, but in cases this is not
done it is still possible to compile the declaration object in the
browser using the JIT compiler. This commit adds a runtime
implementation of `ɵɵngDeclareComponent` which invokes the JIT compiler
using the declaration object, such that a compiled component definition
is made available to the Ivy runtime.
PR Close#40127
The `render3` test targets are currently also executed for ViewEngine
builds, even though the `render3` infrastructure only concerns Ivy
infrastructure. This commit tags the test targets as ivy-only to disable
those tests for View Engine.
PR Close#40127
The link to the "speeding-up-ngcc-compilation" URL does not exist,
it was removed shortly after it was added, but the link in the ngcc
error message was not updated.
Fixes#39837
PR Close#40285
Currently we check whether a property binding contains an interpolation using a regex so
that we can throw an error. The problem is that the regex doesn't account for quotes
which means that something like `[prop]="'{{ foo }}'"` will be considered an error, even
though it's not actually an interpolation.
These changes build on top of the logic from #39826 to account for interpolation
characters inside quotes.
Fixes#39601.
PR Close#40267
The trustConstantHtml and trustConstantResourceUrl functions are only
meant to be passed constant strings extracted from Angular application
templates, as passing other strings or variables could introduce XSS
vulnerabilities.
To better protect these APIs, turn them into template tags. This makes
it possible to assert that the associated template literals do not
contain any interpolation, and thus must be constant.
Also add tests for the change to prevent regression.
PR Close#40082
When talking about parameter inheritance, one might think that matrix
parameters can be inherited from the "parent" segment, or the segment
which appears immediately to the left. In reality, when we talk about
a "parent" in the `Router`, we mean the parent `Route` config. This
config may contain more than one segment and matrix parameters must
appear at the end or they do not "belong" to any config.
PR Close#40304
There are two parts to this commit:
1. Revert the changes from #38379. This change had an incomplete view of
how things worked and also diverged the implementations of
`applyRedirects` and `recognize` even more.
2. Apply the fixes from the `recognize` algorithm to ensure that named
outlets with empty path parents can be matched. This change also passes
all the tests that were added in #38379 with the added benefit of being
a more complete fix that stays in-line with the `recognize` algorithm.
This was made possible by using the same approach for `split` by
always creating segments for empty path matches (previously, this was
only done in `applyRedirects` if there was a `redirectTo` value). At the
end of the expansions, we need to squash all empty segments so that
serializing the final `UrlTree` returns the same result as before.
Fixes#39952Fixes#10726Closes#30410
PR Close#40029
The `applyRedirects` and `recognize` algorithms have the same overall goal:
match a `UrlTree` with the application's `Routes` config. There are a
few key functions in these algorithms which can be shared rather than
duplicated between the two. This also makes it easier to see how the two
are similar and where they diverge.
PR Close#40029
This commit updates the `recognize` algorithm to work with named outlets
which have empty path parents. For example, given the following config
```
const routes = [
{
path: '',
children: [
{path: 'a', outlet: 'aux', component: AuxComponent}
]}
];
```
The url `/(aux:a)` should match this config. In order to do so, we need
to allow the children of `UrlSegmentGroup`s to match a `Route` config
for a different outlet (in this example, the `primary`) when it's an
empty path. This should also *only* happen if we were unable to find a
match for the outlet in the level above. That is, the matching strategy
is to find the first `Route` in the list which _matches the given
outlet_. If we are unable to do that, then we allow empty paths from
other outlets to match and try to find some child there whose outlet
matches our segment.
PR Close#40029
To make the tests suite easier to follow, `Recognize#apply` can be made
into a synchronous function rather than one that return an `Observable`.
Also, as a chore, remove as many `any` types as possible.
PR Close#40029
This commit updates the `recognize` algorithm to return `null` when a
segment does not match a given config rather than throwing an error.
This makes the code much easier to follow because the "no match" result
has to be explicitly handled rather than catching the error in very
specific places.
PR Close#40029
When stepping through the `recognize` algorithm, it is much easier to
follow when using a simple `for...of` rather than the helper
`mapChildrenIntoArray` with the passed closure. The only special thing that
`mapChildrenIntoArray` does is ensure the primary route appears first.
This change will have no affect on the result because `processChildren` later calls
`sortActivatedRouteSnapshots`, which does the same thing.
PR Close#40029
Prior to this commit, removing `FormControlDirective` and `FormGroupName` directive instances didn't clear
the callbacks previously registered on FromControl/FormGroup class instances. As a result, these callbacks
were executed even after `FormControlDirective` and `FormGroupName` directive instances were destroyed. That was
also causing memory leaks since these callbacks also retained references to DOM elements.
This commit updates the cleanup logic to take care of properly detaching FormControl/FormGroup/FormArray instances
from the view by removing view-specific callback at destroy time.
Closes#20007, #37431, #39590.
PR Close#39235
DI providers can be defined via `useFactory` function, which may have arguments configured via `deps` array.
The `deps` array may contain DI flags represented by DI decorators (such as `@Self`, `@SkipSelf`, etc). Prior to this
commit, having the `@Host` decorator in `deps` array resulted in runtime error in Ivy. The problem was that the `@Host`
decorator was not taken into account while `useFactory` argument list was constructed, the `@Host` decorator was
treated as a token that should be looked up.
This commit updates the logic which prepares `useFactory` arguments to recognize the `@Host` decorator.
PR Close#40122
The CLI integration can provide code files in a non-deterministic
order, which led to the extracted translation files having
messages in a non-consistent order between extractions.
This commit fixes this by ensuring that serialized messages
are ordered by their location.
Fixes#39262
PR Close#40192
`Route` configs with `redirectTo` as well as `canActivate` are not valid
because the `canActivate` guards will never execute. Redirects are
applied before activation. There is no error currently for these
configs, but another commit will change this so that an error does
appear in dev mode. This migration fixes the configs by removing the
`canActivate` property.
PR Close#40067
Redirects in the router are processed before activations. This means that a canActivate will
never execute if a route has a redirect. Rather than silently ignoring
the invalid config, developers should be notified so they know why it
doesn't work.
Closes#18605
The feature request for a function/class redirect is covered in #13373.
PR Close#40067
Given the template
`<div (click)="doSomething($event)"></div>`
If you request references for the `$event`, the results include both `$event` and `(click)="doSomething($event)"`.
This happens because in the TCB, `$event` is passed to the `subscribe`/`addEventListener`
function as an argument. So when we ask typescript to give us the references, we
get the result from the usage in the subscribe body as well as the one passed in as an argument.
This commit adds an identifier to the `$event` parameter in the TCB so
that the result returned from `getReferencesAtPosition` can be
identified and filtered out.
fixes#40157
PR Close#40158
According to the [spec](https://html.spec.whatwg.org/#scroll-to-fragid),
we should attempt to set the browser focus after scrolling to a
fragment. Note that this change does not exactly follow the robust steps
outlined in the spec by finding a fallback target if the original is not
focusable. Instead, we simply attempt to focus the element by calling
`focus` on it, which will do nothing if the element is not focusable.
fixes#30067
PR Close#40241
The `ɵɵngDeclareDirective` calls are designed to be translated to fully
AOT compiled code during a build transform, but in cases this is not
done it is still possible to compile the declaration object in the
browser using the JIT compiler. This commit adds a runtime
implementation of `ɵɵngDeclareDirective` which invokes the JIT compiler
using the declaration object, such that a compiled directive definition
is made available to the Ivy runtime.
PR Close#40101
The linker is implemented using a Babel transform such that Babel needs
to parse and walk a source file to find the declarations that need to be
compiled. If it can be determined that a source file is known not to
contain any declarations the parsing and walking can be skipped as a
performance improvement. This commit adds an exposed function for tools
that integrate the linker to use to allow short-circuiting of the linker
transform.
PR Close#40137
Internally we store lifecycle hooks in the format `[index, hook, index, hook]` and when
iterating over them, we check one place ahead to figure out whether we've hit found
a hook or an index. The problem is that the loop is set up to iterate up to `hooks.length`
which means that we may go out of bounds on the last iteration, depending on where
we started. This appears to happen under a specific set of circumstances where a
directive calls `detectChanges` from an input setter while it has `ngOnChanges` and
`ngAfterViewInit` hooks.
These changes resolve the issue by only iterating up to `length - 1` which guarantees that
we can always look one place ahead.
This appears to have regressed some time in version 10.
Fixes#38611.
PR Close#40206
Previously `\r\n` was being treated as a single character in source-map
line start positions, which caused segment positions to become offset.
Now the `\r` is ignored when splitting, leaving it at the end of the
previous line, which solves the offsetting problem, and does not affect
source-mappings.
Fixes#40169Fixes#39654
PR Close#40187
This commit fixes an issue in the ivy native language service
that caused the logic that finds a target node given a template
position to throw away the results. This happened because the
source span of a variable node in the shorthand structural
directive syntax (i.e. `*ngIf=`) included the entire binding.
The result was that we would add the variable node to the path and then
later detect that the cursor was outside the key and value spans and
throw away the whole result. In general, we do this because we do not
want to show information when the cursor is between a key/value
(`inputA=¦"123"`). However, when using the shorthand syntax, we run into
the situation where we can match an `AttributeBinding` as well as the
vaariable in `*ngIf="som¦eValue as myLocalVar"`. This commit updates the
visitor to retain enough information in the visit path to throw away
invalid targets but keep valid ones if there were multiple results on a
`t.Element` or `t.Template`.
PR Close#40239
The linker entry-points were not previously exposed in the NPM Bazel
target so they were omitted from the bundle. This commit adds the
necessary entry-points to the compiler-cli's npm_package target.
PR Close#40180
The types of directives and pipes that are used in a component's
template may be emitted into the partial declaration wrapped inside a
closure, which is needed when the type is declared later in the module.
This poses a problem for JIT compilation of partial declarations, as
this closure is indistinguishable from a class reference itself. To mark
the forward reference function as such, this commit changes the partial
declaration codegen to emit a `forwardRef` invocation wrapped around
the closure, which ensures that the closure is properly tagged as a
forward reference. This allows the forward reference to be treated as
such during JIT compilation.
PR Close#40117
PR #39876 introduced an error where the `onDestroy` of `ComponentRef`
would only get called if `ngDevMode` was set to true. This was because
in dev mode we would freeze `TCleanup` to verify that no more
static cleanup would get added to `TCleanup` array. This ensured
that `TCleanup` was always present in dev mode. In production the
`TCleanup` would get created only when needed. The resulting cleanup
code was incorrectly indented and would only run if `TCleanup` was
present causing this issue.
Fix#40105
PR Close#40120
Escape the content of the strings so that it can be safely inserted into a comment node.
The issue is that HTML does not specify any way to escape comment end text inside the comment.
`<!-- The way you close a comment is with "-->". -->`. Above the `"-->"` is meant to be text
not an end to the comment. This can be created programmatically through DOM APIs.
```
div.innerHTML = div.innerHTML
```
One would expect that the above code would be safe to do, but it turns out that because comment
text is not escaped, the comment may contain text which will prematurely close the comment
opening up the application for XSS attack. (In SSR we programmatically create comment nodes which
may contain such text and expect them to be safe.)
This function escapes the comment text by looking for the closing char sequence `-->` and replace
it with `-_-_>` where the `_` is a zero width space `\u200B`. The result is that if a comment
contains `-->` text it will render normally but it will not cause the HTML parser to close the
comment.
PR Close#40136
Currently when `ɵɵtemplate` and `ɵɵelement` instructions are generated by compiler, all static attributes are
duplicated for both instructions. As a part of this duplication, i18n translation blocks for static i18n attributes
are generated twice as well, causing duplicate entries in extracted translation files (when Ivy extraction mechanisms
are used). This commit fixes this issue by introducing a cache for i18n translation blocks (for static attributes
only).
Also this commit further aligns `ɵɵtemplate` and `ɵɵelement` instruction attributes, which should help implement
more effective attributes deduplication logic.
Closes#39942.
PR Close#40077
Durring analysis we find template parse errors. This commit changes
where the type checking context stores the parse errors. Previously, we
stored them on the AnalysisOutput this commit changes the errors to be
stored on the TemplateData (which is a property on the shim). That way,
the template parse errors can be grouped by template.
Previously, if a template had a parse error, we poisoned the module and
would not procede to find typecheck errors. This change does not poison
modules whose template have typecheck errors, so that ngtsc can emit
typecheck errors for templates with parse errors.
Additionally, all template diagnostics are produced in the same place.
This allows requesting just the template template diagnostics or just
other types of errors.
PR Close#40026
Refactors the i18n error tests to be unit tests in ngtsc_spec.ts. There
is two reasons for doing this.
First is that the tests in compliace_old expected an expection to be be
thrown but did not fail the test if no exception was thrown. That means
that this test could miss catching a bug. It is also a big hacky to call
compile directly and expect an exception to be thrown for diagnostics.
Also, this can easily be unit tested and an end-to-end test is not
necessary since we are not making use of the goldfiles for these tests.
It is easier to maintain and less hacky to validate that we get helpful
error messages when nesting i18n sections by calling getDiagnostics
directly.
PR Close#40026
This commit temporarily excludes classes declared in .d.ts files from checks
regarding whether providers are actually injectable.
Such classes used to be ignored (on accident) because the
`TypeScriptReflectionHost.getConstructorParameters()` method did not return
constructor parameters from d.ts files, mostly as an oversight. This was
recently fixed, but caused more providers to be exposed to this check, which
created a breakage in g3.
This commit temporarily fixes the breakage by continuing to exclude such
providers from the check, until g3 can be patched.
PR Close#40118
This comit adds support for autocompletion of attributes that create
structural directives. Such completions differ from those of normal
attributes, as the structural directive syntax creates a synthetic
<ng-template> node which has different attributes from the main element.
PR Close#40032
This commit introduces an `isStructural` flag on directive metadata, which
is `true` if the directive injects `TemplateRef` (and thus is at least
theoretically usable as a structural directive). The flag is not used for
anything currently, but will be utilized by the Language Service to offer
better autocompletion results for structural directives.
PR Close#40032
This commit adds attribute completion to the Language Service. It completes
from 3 sources:
1. inputs/outputs of directives currently present on the element
2. inputs/outputs/attributes of directives in scope for the element, that
would become present if the input/output/attribute was added
3. DOM properties and attributes
We distinguish between completion of a property binding (`[foo|]`) and a
completion in an attribute context (`foo|`). For the latter, bindings to
the attribute are offered, as well as a property binding which adds the
square bracket notation.
To determine hypothetical matches (directives which would become present if
a binding is added), directives in scope are scanned and matched against a
hypothetical version of the element which has the attribute.
PR Close#40032
This commit adds two new APIs to the `TemplateTypeChecker`:
`getPotentialDomBindings` and `getDirectiveMetadata`. Together, these will
support the Language Service in performing autocompletion of directive
inputs/outputs.
PR Close#40032
The `annotations` package in the compiler previously contained a registry
which tracks NgModule scopes for template type-checking, including unifying
all type-checking metadata across class inheritance lines.
This commit generalizes this utility and prepares it for use in the
`TemplateTypeChecker` as well, to back APIs used by the language service.
PR Close#40032
This commit expands the autocompletion capabilities of the language service
to include element tag names. It presents both DOM elements from the Angular
DOM schema as well as any components (or directives with element selectors)
that are in scope within the template as options for completion.
PR Close#40032
This commit extends the template targeting system, which determines the node
being referenced given a template position, to return additional context if
needed about the particular aspect of the node to which the position refers.
For example, a position pointing to an element node may be pointing either
to its tag name or to somewhere in the node body. This is the difference
between `<div|>` and `<div foo | bar>`.
PR Close#40032
The vim editor produces temporarily files that can end in both .swo and
.swp. This commits add .swp to the .gitignore so we don't accidentaly
commit temporary files.
PR Close#40094
This commit replaces `bazel` with `yarn bazel` in the error message (that instructs to regenerate golden file)
thrown while executing compliance tests. We use `yarn bazel` in other places (so we use the local version of bazel,
not the global one).
PR Close#40078
Projects opened in the LS are often larger in scope than the compilation
units seen by the compiler when actually building. For example, in the LS
it's not uncommon for the project to include both application as well as
test files. This can create issues when the combination of files results
in errors that are not otherwise present - for example, if test files
have inline NgModules that re-declare components (a common Angular pattern).
Such code is valid when compiling the app only (test files are excluded, so
only one declaration is seen by the compiler) or when compiling tests only
(since tests run in JIT mode and are not seen by the AOT compiler), but when
both sets of files are mixed into a single compilation unit, the compiler
sees the double declaration as an error.
This commit attempts to mitigate the problem by forcing the compiler flag
`compileNonExportedClasses` to `false` in a LS context. When tests contain
duplicate declarations, often such declarations are inline in specs and not
exported from the top level, so this flag can be used to greatly improve the
IDE experience.
PR Close#40092
When `checkTypeOfPipes` is set to `false`, the configuration is meant to
ignore the signature of the pipe's `transform` method for diagnostics.
However, we still should produce some information about the pipe for the
`TemplateTypeChecker`. This change refactors the returned symbol for
pipes so that it also includes information about the pipe's class
instance as it appears in the TCB.
PR Close#39555
The TCB utility functions used to find nodes in the TCB are currently
configured to ignore results when an ignore marker is found. However,
these ignore markers are only meant to affect diagnostics requests. The
Language Service may have a need to find nodes with diagnostic ignore
markers. The most common example of this would be finding references for
generic directives. The reference appears to the generic directive's
class appears on the type ctor in the TCB, which is ignored for
diagnostic purposes.
These functions should only skip results when the request is in the
context of a larger request for _diagnostics_. In all other cases, we
should get matches, even if a diagnostic ignore marker is encountered.
PR Close#40071
The ignore marker is only used to ignore certain nodes in the TCB for
the purposes of diagnostics. The marker itself has been renamed as well
as the helper function to see if the marker is present. Both now
indicate that the marker is specifically for diagnostics.
PR Close#40071
With regard to the code in `router_module.ts`, the correct equivalent for `enabled`
should be` enabledBlocking` and not `enabledNonBlocking`.
PR Close#40061
This commit adds the ability to find references for a directive or component
from within a component template. That is, you can find component references
from the element tag `<my-c|omp></my-comp>` (where `|` is the cursor position)
as well as find references for directives that match a given attribute
`<div d|ir></div>`.
PR Close#40054
If we've already identified that we are within a `keySpan` of a node, we
exit the visitor logic early. It can be the case that we have two nodes
which technically match a given location when the end span of one node
touches the start of the keySpan for the candidate node. Because
our `isWithin` logic is inclusive on both ends, we can match both nodes.
This change exits the visitor logic once we've identified a node where
the position is within its `keySpan`.
PR Close#40047
The visitor has a check in it with the goal of preventing the structural directive
parent elements from matching when we have already found the candidate we want.
However, this code did not check to ensure that it was looking at the correct
type of node for this case and was evaluating this logic in places it shouldn't.
This special check can be more easily done by simply not traversing the
template children if we've already found a candidate on the template
node itself.
PR Close#40047
This commit moves the code for cleaning jqLite/jQuery data on an element
to a re-usable helper function. This way it is easier to keep the code
consistent across all places where we need to clean data (now and in the
future).
PR Close#40045
Prior to this change, the `setClassMetadata` call would be invoked
inside of an IIFE that was marked as pure. This allows the call to be
tree-shaken away in production builds, as the `setClassMetadata` call
is only present to make the original class metadata available to the
testing infrastructure. The pure marker is problematic, though, as the
`setClassMetadata` call does in fact have the side-effect of assigning
the metadata into class properties. This has worked under the assumption
that only build optimization tools perform tree-shaking, however modern
bundlers are also able to elide calls that have been marked pure so this
assumption does no longer hold. Instead, an `ngDevMode` guard is used
which still allows the call to be elided but only by tooling that is
configured to consider `ngDevMode` as constant `false` value.
PR Close#39987
Allow configuration of `relativeTo` in the `routerLink` directive. This
is related to the clearing of auxiliary routes, where you need to use
`relativeTo: route.parent` in order to clear it from the activated
auxiliary component itself. This is because `relativeTo: route` will
consume the segment that we're trying to clear, so there is really no
way to do this with routerLink at the moment.
Related issue: #13523
Related (internal link): https://yaqs.corp.google.com/eng/q/5999443644645376
PR Close#39720
Currently the compiler treats something like `{{ '{{a}}' }}` as a nested
binding and throws an error, because it doesn't account for quotes
when it looks for binding characters. These changes add a bit of
logic to skip over text inside quotes when parsing.
Fixes#39601.
PR Close#39826
This commit adds support to the Language Service for autocompletion within
expression contexts. Specifically, this is auto completion of property reads
and method calls, both in normal and safe-navigational forms.
PR Close#39727
When `checkTypeOfOutputEvents` is `false`, we still need to produce the access
to the `EventEmitter` so the Language Service can still get the
type information about the field. That is, in a template `<div
(output)="handle($event)"`, we still want to be able to grab information
when the cursor is inside the "output" parens. The flag is intended only
to affect whether the compiler produces diagnostics for the inferred
type of the `$event`.
PR Close#39515
PR #39665 added the `keySpan` to the output field access so we no longer
need to get there from the call expression and can instead just find the
node we want directly.
PR Close#39515
These tests started failing because they had type-check
errors in their templates, and a recent commit turned on
full template type-checking by default.\
This commit fixes those templates and updates the expected
files as necessary.
PR Close#40040
These tests do not pass the typecheck phase of the compiler and fail.
The option to disable typechecking was removed recently so these tests
need to be fixed to be valid applications.
PR Close#40033
A couple reasons to justify removing the flag:
* It adds code to the compiler that is only meant to support test cases
and not any production. We should avoid code in that's only
meant to support tests.
* The flag enables writing tests that do not mimic real-world behavior
because they allow invalid applications
PR Close#40013
Rather than returning `null`, we can provide some useful information to the Language Service
by returning a symbol for the `addEventListener` function call when the consumer
of a binding as an element.
PR Close#39312
Previously, due to the way the AngularJS and Angular clean-up processes
interfere with each other when removing an AngularJS element that
contains a downgraded Angular component, the data associated with the
host element of the downgraded component was not removed. This data was
kept in an internal AngularJS cache, which prevented the element and
component instance from being garbage-collected, leading to memory
leaks.
This commit fixes this by ensuring the element data is explicitly
removed when cleaning up a downgraded component.
NOTE:
This is essentially the equivalent of #26209 but for downgraded (instead
of upgraded) components.
Fixes#39911Closes#39921
PR Close#39965
The prior usage of a ternary expression caused the code to be formatted
in a weird way, so this commit replaces the ternary with an `if` statement.
PR Close#39961
Prior to this change the interpolation config value was cast to
`[string, string]` without checking whether there really were two
string values available. This commit extracts the logic of parsing the
interpolation config into a separate function and adds a check that
the array contains exactly two strings.
PR Close#39961
This change allows the `AstObject` and `AstValue` types to provide
their represented type as a generic type argument, which is helpful
for documentation and discoverability purposes.
PR Close#39961
This allows the code generation to correspond with a type, which is
helpful for documentation and discoverability purposes. This does not
offer any type-safety with respect to the actually generated code.
PR Close#39961
When the compiler option `checkTypeOfAttributes` is `false`, we should
still be able to produce type information from the
`TemplateTypeChecker`. The current behavior ignores all attributes that
map to directive inputs. This commit includes those attribute bindings
in the TCB but adds the "ignore for diagnostics" marker so they do not
produce errors. This way, consumers of the TTC (the Language Service)
can still get valid information about these attributes even when the
user has configured the compiler to not produce diagnostics/errors for them.
PR Close#39537
The golden files for the partial compliance tests need to be updated
with individual Bazel run invocations, which is not very ergonomic when
a large number of golden files need to updated. This commit adds a
script to query the Bazel targets that update the goldens and then runs
those targets sequentially.
PR Close#39989
This test migrates source-mapping tests to the new compliance test framework.
The original tests are found in the file at:
`packages/compiler-cli/test/ngtsc/template_mapping_spec.ts`.
These new tests also check the mappings resulting from partial compilation
followed by linking, after flattening the pair of source-maps that each
process generates.
Note that there are some differences between the mappings for full compile
and linked compile modes, due to how TypeScript and Babel use source-span
information on AST nodes. To accommodate this, there are two expectation
files for most of these source files.
PR Close#39939
This commit allows compliance test-cases to be written that specify
source-map mappings between the source and generated code.
To check a mapping, add a `// SOURCE:` comment to the end of a line:
```
<generated code> // SOURCE: "<source-url>" <source code>
```
The generated code will still be checked, stripped of the `// SOURCE` comment,
as normal by the `expectEmit()` helper.
In addition, the source-map segments are checked to ensure that there is a
mapping from `<generated code>` to `<source code>` found in the file at
`<source-url>`.
Note:
* The source-url should be absolute, with the directory containing the
TEST_CASES.json file assumed to be `/`.
* Whitespace is important and will be included when comparing the segments.
* There is a single space character between each part of the line.
* Newlines within a mapping must be escaped since the mapping and comment
must all appear on a single line of this file.
PR Close#39939
Previously one could set a flag in a `TEST_CASES.json` file to exclude
the test-cases from being run if the input files were being compiled
partially and then linked.
There are also scenarios where one might want to exclude test-cases
from "full compile" mode test runs.
This commit changes the compliance test tooling to support a new
property `compilationModeFilter`, which is an array containing one or
more of `"full compile"` and `"linked compile"`. Only the tests
whose `compilationModeFilter` array contains the current compilation
mode will be run.
PR Close#39939
Previously files were serialized with an extra newline seperator that
was not removed when parsing. This caused the parsed file to start with
an extra newline that invalidated its source-map.
Also, the splitting was producing an empty entry at the start of the extracted
golden files which is now ignored.
PR Close#39939
The schema accidentally included the `expectedErrors` and `extraCheck`
properties below the `files` property instead of below the `expectations`
property.
PR Close#39939
Add a TaggedTemplateExpr to represent tagged template literals in
Angular's syntax tree (more specifically Expression in output_ast.ts).
Also update classes that implement ExpressionVisitor to add support for
tagged template literals in different contexts, such as JIT compilation
and conversion to JS.
Partial support for tagged template literals had already been
implemented to support the $localize tag used by Angular's i18n
framework. Where applicable, this code was refactored to support
arbitrary tags, although completely replacing the i18n-specific support
for the $localize tag with the new generic support for tagged template
literals may not be completely trivial, and is left as future work.
PR Close#39122
Add test for when `checkTypeOfDomReferences = false` to ensure that we
do not regress in behavior at any point. The desired behavior for this
case is that the `TemplateTypeChecker` will honor the user's
configuration and not produce symbols for the dom reference.
PR Close#39539
Differs tries to inject parent differ in order to support extending.
This does not work in the 'root' injector as the provider overrides the
default injector. The fix is to just assume standard set of providers
and extend those instead.
PR close#25015
Issue close#11309 `Can't extend IterableDiffers`
Issue close#18554 `IterableDiffers.extend is not AOT compatible`
(This is fixed because we no longer have an arrow function in the
factory but a proper function which can be imported.)
PR Close#39981
We intend to run the `@angular/upgrade` tests against all supported
versions of AngularJS (v1.5+). Previously, we only ran them against
v1.5, v1.6 and v1.7.
Since AngularJS v1.8 was released recently, this commit adds it to the
list of AngularJS versions we test against.
PR Close#39972
This commit adds `ngDevMode` guard to call `_ngModelWarning` only
in dev mode (similar to how things work in other parts of Ivy runtime code).
The `ngDevMode` flag helps to tree-shake this function from production builds
(since it will act as no-op, in dev mode everything will work as it works right now)
to decrease production bundle size.
PR Close#39964
This commit adds ngDevMode guard to show warning only
in dev mode (similar to how things work in other parts of Ivy runtime code).
The ngDevMode flag helps to tree-shake this warning from production builds
(in dev mode everything will work as it works right now) to decrease production bundle size.
PR Close#39964
This commit adds ngDevMode guard to show warnings only
in dev mode (similar to how things work in other parts of Ivy runtime code).
The ngDevMode flag helps to tree-shake these warnings from production builds
(in dev mode everything will work as it works right now) to decrease production bundle size.
PR Close#39964
This commit adds `ngDevMode` guard to run `checkNoChanges` only
in dev mode (similar to how things work in other parts of Ivy runtime code).
The `ngDevMode` flag helps to tree-shake this code from production builds
(in dev mode everything will work as it works right now) to decrease production bundle size.
PR Close#39964
The partial compiler will add a version number to the objects that are
generated so that the linker can select the appropriate partial linker
class to process the metadata.
Previously this version matching was a simple number check. Now
the partial compilation writes the current Angular compiler version
into the generated metadata, and semantic version ranges are used
to select the appropriate partial linker.
PR Close#39847
This commit adds `ngDevMode` guard to show sanitization warnings only
in dev mode (similar to how things work in other parts of Ivy runtime code).
The `ngDevMode` flag helps to tree-shake these warnings from production builds
(in dev mode everything will work as it works right now) to decrease production bundle size.
PR Close#39959
This commit adds support in the Ivy Language Service for autocompletion in a
global context - e.g. a {{foo|}} completion.
Support is added both for the primary function `getCompletionsAtPosition` as
well as the detail functions `getCompletionEntryDetails` and
`getCompletionEntrySymbol`. These latter operations are not used yet as an
upstream change to the extension is required to advertise and support this
capability.
PR Close#39250
This expands the deprecation message that started to pop up in v11.0.3
after the landing of commit e148382bd0,
that deprecated the `{[key: string]: any}` type for the options property of the `FormBuilder.group` method.
It turns out that having a custom validator declared as
`{ validators: (group: FormGroup) => ValidationErrors|null }` works in practice,
but is now inferred by TS as the deprecated version of `group`
(because `FormGroup` is a subclass of `AbstractControl` that `ValidatorFn` expects).
We considered the possibility of tweaking the forms API to accept such validators,
but it turns out to generate too many changes in the framework or possible breaking changes for Angular users.
We settled for a more explicit deprecation message, elaborated with the help of @petebacondarwin.
This will hopefully help developers to understand why the deprecation warning is showing up
when they think they are already using the non-deprecated overload.
PR Close#39946
The newly built compliance test runner was not using the shared source
file cache that was added in b627f7f02e,
which offers a significant performance boost to the compliance test
targets.
PR Close#39956
In the past, the legacy (VE-based) language service would use a
`UrlResolver` instance to resolve file paths, primarily for compiler
resources like external templates. The problem with this is that the
UrlResolver is designed to resolve URLs in general, and so for a path
like `/a/b/#c`, `#c` is treated as hash/fragment rather than as part
of the path, which can lead to unexpected path resolution (f.x.,
`resolve('a/b/#c/d.ts', './d.html')` would produce `'a/b/d.html'` rather
than the expected `'a/b/#c/d.html'`).
This commit resolves the issue by using Node's `path` module to resolve
file paths directly, which aligns more with how resources are resolved
in the Ivy compiler.
The testing story here is not great, and the API for validating a file
path could be a little bit prettier/robust. However, since the VE-based
language service is going into more of a "maintenance mode" now that
there is a clear path for the Ivy-based LS moving forward, I think it is
okay not to spend too much time here.
Closes https://github.com/angular/vscode-ng-language-service/issues/892
Closes https://github.com/angular/vscode-ng-language-service/issues/1001
PR Close#39917
Create stubs for `findRenameLocations` for both VE and Ivy Language Service
implementations. This will prevent failed requests when it is implemented on the vscode plugin side.
PR Close#39919
At the moment, when creating a root module, a subscription to the
`onError` subject is also created. It captures the scope where `NgModuleRef`
is created and prevents it from being garbage collected. Also note that this
`NgModuleRef` has a reference to the root module instance (e.g. `AppModule`),
which also prevents it from being GC'd.
PR Close#39940
When the compiler is invoked via ngc or the Angular CLI, its APIs are used
under the assumption that Angular analysis/diagnostics are only requested if
the program has no TypeScript-level errors. A result of this assumption is
that the incremental engine has not needed to resolve changes via its
dependency graph when the program contained broken imports, since broken
imports are a TypeScript error.
The Angular Language Service for Ivy is using the compiler as a backend, and
exercising its incremental compilation APIs without enforcing this
assumption. As a result, the Language Service has run into issues where
broken imports cause incremental compilation to fail and produce incorrect
results.
This commit introduces a mechanism within the compiler to keep track of
files for which dependency analysis has failed, and to always treat such
files as potentially affected by future incremental steps. This is tested
via the Language Service infrastructure to ensure that the compiler is doing
the right thing in the case of invalid imports.
PR Close#39923
Previously, if a component had an external template with a hard error, the
compiler would "forget" the link between that component and its NgModule.
Additionally, the NgModule would be marked as being in error, because the
template issue would prevent the compiler from registering the component
class as a component, so from the NgModule it would look like a declaration
of a non-directive/pipe class. As a combined result, the next incremental
step could fix the template error, but would not refresh diagnostics for the
NgModule, leading to an incrementality issue.
The various facets of this problem were fixed in prior commits. This commit
adds a test verifying the above case works now as expected.
PR Close#39923
To avoid overwhelming a user with secondary diagnostics that derive from a
"root cause" error, the compiler has the notion of a "poisoned" NgModule.
An NgModule becomes poisoned when its declaration contains semantic errors:
declarations which are not components or pipes, imports which are not other
NgModules, etc. An NgModule also becomes poisoned if it imports or exports
another poisoned NgModule.
Previously, the compiler tracked this poisoned status as an alternate state
for each scope. Either a correct scope could be produced, or the entire
scope would be set to a sentinel error value. This meant that the compiler
would not track any information about a scope that was determined to be in
error.
This method presents several issues:
1. The compiler is unable to support the language service and return results
when a component or its module scope is poisoned.
This is fine for compilation, since diagnostics will be produced showing the
error(s), but the language service needs to still work for incorrect code.
2. `getComponentScopes()` does not return components with a poisoned scope,
which interferes with resource tracking of incremental builds.
If the component isn't included in that list, then the NgModule for it will
not have its dependencies properly tracked, and this can cause future
incremental build steps to produce incorrect results.
This commit changes the tracking of poisoned module scopes to use a flag on
the scope itself, rather than a sentinel value that replaces the scope. This
means that the scope itself will still be tracked, even if it contains
semantic errors. A test is added to the language service which verifies that
poisoned scopes can still be used in template type-checking.
PR Close#39923
Previously, if a trait's analysis step resulted in diagnostics, the trait
would be considered "errored" and no further operations, including register,
would be performed. Effectively, this meant that the compiler would pretend
the class in question was actually undecorated.
However, this behavior is problematic for several reasons:
1. It leads to inaccurate diagnostics being reported downstream.
For example, if a component is put into the error state, for example due to
a template error, the NgModule which declares the component would produce a
diagnostic claiming that the declaration is neither a directive nor a pipe.
This happened because the compiler wouldn't register() the component trait,
so the component would not be recorded as actually being a directive.
2. It can cause incorrect behavior on incremental builds.
This bug is more complex, but the general issue is that if the compiler
fails to associate a component and its module, then incremental builds will
not correctly re-analyze the module when the component's template changes.
Failing to register the component as such is one link in the larger chain of
issues that result in these kinds of issues.
3. It lumps together diagnostics produced during analysis and resolve steps.
This is not causing issues currently as the dependency graph ensures the
right classes are re-analyzed when needed, instead of showing stale
diagnostics. However, the dependency graph was not intended to serve this
role, and could potentially be optimized in ways that would break this
functionality.
This commit removes the concept of an "errored" trait entirely from the
trait system. Instead, analyzed and resolved traits have corresponding (and
separate) diagnostics, in addition to potentially `null` analysis results.
Analysis (but not resolution) diagnostics are carried forward during
incremental build operations. Compilation (emit) is only performed when
a trait reaches the resolved state with no diagnostics.
This change is functionally different than before as the `register` step is
now performed even in the presence of analysis errors, as long as analysis
results are also produced. This fixes problem 1 above, and is part of the
larger solution to problem 2.
PR Close#39923
If the testcase has not specified that errors were expected, then any
errors that have occurred should be reported. These errors may have
prevented an output file from being generated, which resulted in hard
to debug test failures due to missing files.
PR Close#39862
This commit adds a few tests to verify that the `onDestroy` callbacks are invoked when `ComponentRef` instance
is destroyed and the logic is consistent between ViewEngine and Ivy.
PR Close#39876
In the new behavior Angular removes applications from the testability registry when the
root view gets destroyed. This eliminates a memory leak, because before that the
TestabilityRegistry holds references to HTML elements, thus they cannot be GCed.
PR Close#22106
PR Close#39876
The Language Service "find references" currently uses the
`ngtypecheck.ts` suffix to determine if a file is a shim file. Instead,
a better API would be to expose a method in the template type checker
that does this verification so that the LS does not have to "know" about
the typecheck suffix. This also fixes an issue (albeit unlikely) whereby a file
in the user's program that _actually_ is named with the `ngtypecheck.ts`
suffix would have been interpreted as a shim file.
PR Close#39768
This commit adds "find references" functionality to the Ivy integrated
language service. The basic approach is as follows:
1. Generate shims for all files to ensure we find references in shims
throughout the entire program
2. Determine if the position for the reference request is within a
template.
* Yes, it is in a template: Find which node in the template AST the
position refers to. Then find the position in the shim file for that
template node. Pass the shim file and position in the shim file along
to step 3.
* No, the request for references was made outside a template: Forward
the file and position to step 3.
3. (`getReferencesAtTypescriptPosition`): Call the native TypeScript LS
`getReferencesAtPosition`. For each reference that is in a shim file, map those
back to a template location, otherwise return it as-is.
PR Close#39768
There were two issues with the current TCB:
1. The logic for only wrapping the right hand side of the property write
if it was not already a parenthesized expression was incorrect. A
parenthesized expression could still have a trailing comment, and if
that were the case, that span comment would still be ambiguous, as explained
by the comment in the code before `wrapForTypeChecker`.
2. The right hand side of keyed writes was not wrapped in parens at all
PR Close#39768
In order to map the a safe property read's method access in the type check block
directly back to the property in the template source, we need to
include the `SafePropertyRead`'s `nameSpan` with the `ts.propertyAccess` for
the pipe's transform method.
Note that this is specifically relevant to the Language Service's "find
references" feature. As an example, with something like `{{a?.value}}`,
when calling "find references" on the 'value' we want the text
span of the reference to just be `value` rather than the entire source
`a?.value`.
PR Close#39768
In order to map the pipe's `transform` method in the type check block
directly back to the pipe name in the template source, we need to
include the `BindingPipe`'s `nameSpan` with the `ts.methodAccess` for
the pipe's transform method.
Note that this is specifically relevant to the Language Service's "find
references" feature. As an example, with something like `-2.5 | number:'1.0-0'`,,
when calling "find references" on the 'number' pipe we want the text
span of the reference to just be `number` rather than the entire binding
pipe's source `-2.5 | number:'1.0-0'`.
PR Close#39768
Fix a case where, if the parent class had already been patched, it would
not patch the child class. In addition to checking if the method is
defined in the prototype, and not inherited, it also does the same for
the unpatched method.
PR Close#39850
Currently we convert objects to strings using `'' + value` which is quickest,
but it stringifies the value using its `valueOf`, rather than `toString`. These
changes switch to using `String(value)` which has identical performance
and calls the `toString` method as expected. Note that another option
was calling `toString` directly, but benchmarking showed it to be slower.
I've included the benchmark I used to verify the performance so we have it
for future reference and we can reuse it when making changes to `renderStringify`
in the future.
Also for reference, here are the results of the benchmark:
```
Benchmark: renderStringify
concat: 2.006 ns(0%)
concat with toString: 2.201 ns(-10%)
toString: 237.494 ns(-11741%)
toString with toString: 121.072 ns(-5937%)
constructor: 2.201 ns(-10%)
constructor with toString: 2.201 ns(-10%)
toString mono: 14.536 ns(-625%)
toString with toString mono: 9.757 ns(-386%)
```
Fixes#38839.
PR Close#39843
Previously this would have just printed that `false` was not equal to
`true`, which, although true, is not very helpful. This commit adds
details about which special check failed together with the generated
code, for easier debugging.
PR Close#39863
We currently only wrap the event listener in the function which ensures
ancestors are marked for check when the listener is placed on an element
that has a native method for listening to an event. We actually need to do
this wrapping in all cases so that events that are attached to non-rendered
template items (`ng-template` and `ng-container`) also mark ancestors for check
when they receive the event.
fixes#39832
PR Close#39833
DEPRECATION:
Mark the {[key: string]: any} type for the options property of the FormBuilder.group method as deprecated.
Using AbstractControlOptions gives the same functionality and is type-safe.
PR Close#39769
`@angular/core` support zone.js `^0.10.2 and ^0.11.3`, so this PR updates the
peerDependencies to `^0.10.2 || ^ 0.11.3`, so the app will not show warning about
peer denepdency not consistent when using zone.js 0.10.x version.
PR Close#39809
This commit provides the machinery for the new file-based compliance test
approach for i18n tests, and migrates the i18n tests to this new format.
PR Close#39661
For element queries that return sufficiently large NodeList
objects, using spread syntax to populate the results array
causes a RangeError due to the call stack limit being reached.
This commit updates the code to use regular "for" loop instead.
Fixes#38551.
PR Close#39646
The ARB format is a JSON file containing an object where the keys are the
message ids and the values are the translations.
It is extensible because it can also contain metadata about each message.
For example:
```
{
"@@locale": "...",
"message-id": "Translated message string",
"@message-id": {
"type": "text",
"description": "Some description text",
"x-locations": [{ "start": {"line": 23, "column": 145}, "file": "some/file.ts" }]
},
}
```
For more information, see:
https://github.com/google/app-resource-bundle/wiki/ApplicationResourceBundleSpecification
PR Close#36795
Create stubs for getTypeDefinitionAtPosition for both VE and Ivy Language Service
implementations. This will prevent failed requests when it is implemented on the vscode plugin side.
PR Close#39829
This commit implements partial compilation of components, together with
linking the partial declaration into its full AOT output.
This commit does not yet enable accurate source maps into external
templates. This requires additional work to account for escape sequences
which is non-trivial. Inline templates that were represented using a
string or template literal are transplated into the partial declaration
output, so their source maps should be accurate. Note, however, that
the accuracy of source maps is not currently verified in tests; this is
also left as future work.
The golden files of partial compilation output have been updated to
reflect the generated code for components. Please note that the current
output should not yet be considered stable.
PR Close#39707
In production mode this flag defaults to `true`, but the compliance
tests override this to `false` unless it is provided. As such, the
linker should also adhere to this default as otherwise the compilation
output would not align with the output of the full tests.
There are still tests that exercise the value of this flag, together
with it being `undefined` to verify the behavior of the actual default
value.
PR Close#39707
The linker does not currently support outputting ES5 syntax, so any
compliance tests that request ES5 output cannot be run in partial
compilation mode. This commit marks these tests as pending.
PR Close#39707
This commit adds the `i18nUseExternalIds` option to the linker options,
as the compliance tests exercise compilation results with and without
this flag enabled. We therefore need to configure the linker to take
this option into account, as otherwise the compliance test output would
not be identical.
Additionally, this commit switches away from spread syntax to set
the default options. This introduced a problem when the user-provided
options object did specify the keys, but with an undefined value. This
would have prevented the default options from being applied.
PR Close#39707
This commit is a precursor to supporting the partial compilation of
components, which leverages some of the compilation infrastructure that
is in place for directives.
PR Close#39707
The metadata specification of queries allows for the boolean properties
`first`, `descendants` and `static` to be missing, but the linker did
not account for their omission.
This fix is tested in subsequent commits that implement compilation of
components, at which point this will be covered by the compliance tests.
PR Close#39707
The compilation result of components may have inserted template
functions into the constant pool, which would be inserted into the Babel
AST upon program exit. Babel will then proceed with visiting this newly
inserted subtree, but we have already cleaned up the linker instance
when exiting the program. Any call expressions within the template
functions would then fail to be processed, as a file linker would no
longer be available.
Since the inserted AST subtree is known not to contain yet more partial
declarations, it is safe to skip visiting call expressions when no
file linker is available.
PR Close#39707
The type checker had to do extensive work in resolving the
`NodePath.get` method call for the `NodePath` that had an intersection
type of `ts.VariableDeclarator&{init:t.Expression}`. The `NodePath.get`
method is typed using a conditional type which became expensive to
compute with this intersection type. As a workaround, the original
`init` property is explicitly omitted which avoids the performance
cliff. This brings down the compile time by 15s.
PR Close#39707
The JSON schema reference was off-by-one, preventing IDEs from finding
the file and offering suggestions and documentation. Additionally the
name of the golden file was slightly off.
PR Close#39707
If a template declares a reference to a missing target then referring to
that reference from elsewhere in the template would crash the template
type checker, due to a regression introduced in #38618. This commit
fixes the crash by ensuring that the invalid reference will resolve to
a variable of type any.
Fixes#39744
PR Close#39805
Archives most of the content in the template expression operators doc.
The pipes precedence section that was originally in
template expression operators moves into the pipes doc
with some editing and an addition of a ternary example.
PR Close#39170
Since 5be4edfa17, a failing cache-busted
network request (such as requests for fetching uncached assets) will
cause the ServiceWorker to incorrectly enter a degraded
`EXISTING_CLIENTS_ONLY` mode. A failing network request could be caused
by many reasons, including the client or server being offline, and does
not necessarily signify a broken ServiceWorker state.
This commit fixes the logic in `cacheBustedFetchFromNetwork()` to
correctly handle errors in network requests.
For more details on the problem and the implemented fix see #39775.
Fixes#39775
PR Close#39786
Script tags, inline event handlers and other script contexts are
forbidden or stripped from Angular templates by the compiler. In the
context of Trusted Types, this leaves no sinks that require use of a
TrustedScript. This means that trustConstantScript is never used, and
can be removed.
PR Close#39554
Previously all constant values of security-sensitive attributes and
properties were promoted to Trusted Types. While this is not inherently
bad, it is also not optimal.
Use the newly added Trusted Types schema to restrict promotion to
constants that are in a Trusted Types-relevant context.
PR Close#39554
To minimize security risk (XSS in particular) in the i18n pipeline,
disallow i18n translation of attributes that are Trusted Types sinks.
Add integration tests to ensure that such sinks cannot be translated.
PR Close#39554
As only methods from the Subscribable interface are currently used in the
implementation of the async pipe, it makes sense to make it explicit so
that it works successfully with any other implementation instead of
only Observable.
PR Close#39627
The value of a `FormControl` is treated in a special way (called boxed values) when it's an object with exactly
2 fields: `value` and `disabled`. This commit adds a test which verifies that an object is not treated as a boxed
value when `disabled` field is present, but `value` is missing.
PR Close#39801
Currently all of our migrations are set up to find the tsconfig paths within a project,
create a `Program` out of each and migrate the files inside of the `Program`. The
problem is that the `Program` can include files outside of the project and the CLI
APIs that we use to interact with the file system assume that all files are within
the project.
These changes consolidate the logic, that determines whether a file can be migrated,
in a single place and add an extra check to exclude files outside of the root.
Fixes#39778.
PR Close#39790
We need to expose the declaration files for Ivy sources so that they can
be consumed by the Angular language server (`@angular/language-server`).
PR Close#39748
There are many places where examples use just a string for the command
in outlets. When using nested outlets, we do not correctly handle
this case, as the types and algorithm always expect an array.
This PR updates the `createUrlTree` algorithm to account for the
possibility of a string literal as the command for an outlet.
Fixes#18928
PR Close#39728
When parsing for i18n messages, interpolated strings are
split into `Text` and `Placeholder` pieces. The method that
does this `_visitTextWithInterpolation()` was becoming too
complex. This commit refactors that method along with some
associated functions that it uses.
PR Close#39717
When the `preserveWhitespaces` is not true, the template parser will
process the parsed AST nodes to remove excess whitespace. Since the
generated `goog.getMsg()` statements rely upon the AST nodes after
this whitespace is removed, the i18n extraction must make a second pass.
Previously this resulted in innacurrate source-spans for the i18n text and
placeholder nodes that were extracted in the second pass.
This commit fixes this by reusing the source-spans from the first pass
when extracting the nodes in the second pass.
Fixes#39671
PR Close#39717