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