As a part of the process of setting up Router providers, we use `ApplicationRef` as a dependency while providing `Router` token. The thing is that `ApplicationRef` is actually unused (all referenced were removed in 5a849829c4 (diff-c0baae5e1df628e1a217e8dc38557fcb)), but it's still listed as dependency. This is causing problems in case `Router` is used as a dependency for factory functions provided as `APP_INITIALIZERS` multi-token (causing cyclic dependency). This commit removes unused `ApplicationRef` dependency in `Router`, so it can be used without causing cyclic dependency issue.
PR Close#35642
Previously if there were two path-mapped libraries that are in
different directories but the path of one started with same string
as the path of the other, we would incorrectly return the shorter
path - e.g. `dist/my-lib` and `dist/my-lib-second`. This was because
the list of `basePaths` was searched in ascending alphabetic order and
we were using `startsWith()` to match the path.
Now the `basePaths` are searched in reverse alphabetic order so the
longer path will be matched correctly.
// FW-1873
Fixes#35536
PR Close#35592
* it's tricky to get out of the runfiles tree with `bazel test` as `BUILD_WORKSPACE_DIRECTORY` is not set but I employed a trick to read the `DO_NOT_BUILD_HERE` file that is one level up from `execroot` and that contains the workspace directory. This is experimental and if `bazel test //:test.debug` fails than `bazel run` is still guaranteed to work as `BUILD_WORKSPACE_DIRECTORY` will be set in that context
* test //integration:bazel_test and //integration:bazel-schematics_test exclusively
* run "exclusive" and "manual" bazel-in-bazel integration tests in their own CI job as they take 8m+ to execute
```
//integration:bazel-schematics_test PASSED in 317.2s
//integration:bazel_test PASSED in 167.8s
```
* Skip all integration tests that are now handled by angular_integration_test except the tests that are tracked for payload size; these are:
- cli-hello-world*
- hello_world__closure
* add & pin @babel deps as newer versions of babel break //packages/localize/src/tools/test:test
@babel/core dep had to be pinned to 7.6.4 or else //packages/localize/src/tools/test:test failed. Also //packages/localize uses @babel/generator, @babel/template, @babel/traverse & @babel/types so these deps were added to package.json as they were not being hoisted anymore from @babel/core transitive.
NB: integration/hello_world__systemjs_umd test must run with systemjs 0.20.0
NB: systemjs must be at 0.18.10 for legacy saucelabs job to pass
NB: With Bazel 2.0, the glob for the files to test `"integration/bazel/**"` is empty if integation/bazel is in .bazelignore. This glob worked under these conditions with 1.1.0. I did not bother testing with 1.2.x as not having integration/bazel in .bazelignore is correct.
PR Close#33927
When a pipe inherits its constructor, and as a result its factory, from an injectable in AOT mode, it can end up throwing an error, because the inject implementation hasn't been set yet. These changes ensure that the implementation is set before the pipe's factory is invoked.
Note that this isn't a problem in JIT mode, because the factory inheritance works slightly differently, hence why this test isn't going through `TestBed`.
Fixes#35277.
PR Close#35468
Under View Engine's default (non-fullTemplateTypeCheck) checking, object and
array literals which appear in templates are treated as having type `any`.
This allows a number of patterns which would not otherwise compile, such as
indexing an object literal by a string:
```html
{{ {'a': 1, 'b': 2}[value] }}
```
(where `value` is `string`)
Ivy, meanwhile, has always inferred strong types for object literals, even
in its compatibility mode. This commit fixes the bug, and adds the
`strictLiteralTypes` flag to specifically control this inference. When the
flag is `false` (in compatibility mode), object and array literals receive
the `any` type.
PR Close#35462
In its default compatibility mode, the Ivy template type-checker attempts to
emulate the View Engine default mode as accurately as is possible. This
commit addresses a gap in this compatibility that stems from a View Engine
type-checking bug.
Consider two template expressions:
```html
{{ obj?.field }}
{{ fn()?.field }}
```
and suppose that the type of `obj` and `fn()` are the same - both return
either `null` or an object with a `field` property.
Under View Engine, these type-check differently. The `obj` case will catch
if the object type (when not null) does not have a `field` property, while
the `fn()` case will not. This is due to how View Engine represents safe
navigations:
```typescript
// for the 'obj' case
(obj == null ? null as any : obj.field)
// for the 'fn()' case
let tmp: any;
((tmp = fn()) == null ? null as any : tmp.field)
```
Because View Engine uses the same code generation backend as it does to
produce the runtime code for this expression, it uses a ternary for safe
navigation, with a temporary variable to avoid invoking 'fn()' twice. The
type of this temporary variable is 'any', however, which causes the
`tmp.field` check to be meaningless.
Previously, the Ivy template type-checker in compatibility mode assumed that
`fn()?.field` would always check for the presence of 'field' on the non-null
result of `fn()`. This commit emulates the View Engine bug in Ivy's
compatibility mode, so an 'any' type will be inferred under the same
conditions.
As part of this fix, a new format for safe navigation operations in template
type-checking code is introduced. This is based on the realization that
ternary based narrowing is unnecessary.
For the `fn()` case in strict mode, Ivy now generates:
```typescript
(null as any ? fn()!.field : undefined)
```
This effectively uses the ternary operator as a type "or" operation. The
resulting type will be a union of the type of `fn()!.field` with
`undefined`.
For the `fn()` case in compatibility mode, Ivy now emulates the bug with:
```typescript
(fn() as any).field
```
The cast expression includes the call to `fn()` and allows it to be checked
while still returning a type of `any` from the expression.
For the `obj` case in compatibility mode, Ivy now generates:
```typescript
(obj!.field as any)
```
This cast expression still returns `any` for its type, but will check for
the existence of `field` on the type of `obj!`.
PR Close#35462
I think the bug is introduced in my PR#34847. For example, 'model="{{title}}"', the attribute value cannot be parsed by 'parseTemplateBindings'.
PR Close#35494
In ES5 code, TypeScript requires certain helpers (such as
`__spreadArrays()`) to be able to support ES2015+ features. These
helpers can be either imported from `tslib` (by setting the
`importHelpers` TS compiler option to `true`) or emitted inline (by
setting the `importHelpers` and `noEmitHelpers` TS compiler options to
`false`, which is the default value for both).
Ngtsc's `StaticInterpreter` (which is also used during ngcc processing)
is able to statically evaluate some of these helpers (currently
`__assign()`, `__spread()` and `__spreadArrays()`), as long as
`ReflectionHost#getDefinitionOfFunction()` correctly detects the
declaration of the helper. For this to happen, the left-hand side of the
corresponding call expression (i.e. `__spread(...)` or
`tslib.__spread(...)`) must be evaluated as a function declaration for
`getDefinitionOfFunction()` to be called with.
In the case of imported helpers, the `tslib.__someHelper` expression was
resolved to a function declaration of the form
`export declare function __someHelper(...args: any[][]): any[];`, which
allows `getDefinitionOfFunction()` to correctly map it to a TS helper.
In contrast, in the case of emitted helpers (and regardless of the
module format: `CommonJS`, `ESNext`, `UMD`, etc.)), the `__someHelper`
identifier was resolved to a variable declaration of the form
`var __someHelper = (this && this.__someHelper) || function () { ... }`,
which upon further evaluation was categorized as a `DynamicValue`
(prohibiting further evaluation by the `getDefinitionOfFunction()`).
As a result of the above, emitted TypeScript helpers were not evaluated
in ES5 code.
---
This commit changes the detection of TS helpers to leverage the existing
`KnownFn` feature (previously only used for built-in functions).
`Esm5ReflectionHost` is changed to always return `KnownDeclaration`s for
TS helpers, both imported (`getExportsOfModule()`) as well as emitted
(`getDeclarationOfIdentifier()`).
Similar changes are made to `CommonJsReflectionHost` and
`UmdReflectionHost`.
The `KnownDeclaration`s are then mapped to `KnownFn`s in
`StaticInterpreter`, allowing it to statically evaluate call expressions
involving any kind of TS helpers.
Jira issue: https://angular-team.atlassian.net/browse/FW-1689
PR Close#35191
This is in preparation of using the `KnownFn` type for known TypeScript
helpers (in addition to built-in functions/methods). This will in turn
allow simplifying the detection of both imported and emitted TypeScript
helpers.
PR Close#35191
When statically evaluating CommonJS code it is possible to find that we
are looking for the declaration of an identifier that actually came from
a typings file (rather than a CommonJS file).
Previously, the CommonJS reflection host would always try to use a
CommonJS specific algorithm for finding identifier declarations, but
when the id is actually in a typings file this resulted in the returned
declaration being the containing file of the declaration rather than the
declaration itself.
Now the CommonJS reflection host will check to see if the file
containing the identifier is a typings file and use the appropriate
stategy.
(Note: This is the equivalent of #34356 but for CommonJS.)
PR Close#35191
In #33705 we made it so that we generate pure functions for object/array literals in order to avoid having them be shared across elements/views. The problem this introduced is that further down the line the `ContantPool` uses the generated literal in order to figure out whether to share an existing factory or to create a new one. `ConstantPool` determines whether to share a factory by creating a key from the AST node and using it to look it up in the factory cache, however the key generation function didn't handle function invocations and replaced them with `null`. This means that the key for `{foo: pureFunction0(...)}` and `{foo: null}` are the same.
These changes rework the logic so that instead of generating a `null` key
for function invocations, we generate a variable called `<unknown>` which
shouldn't be able to collide with anything.
Fixes#35298.
PR Close#35481
Currently Ivy always generates the `$event` function argument, even if it isn't being used by the listener expressions. This can lead to unnecessary bytes being generated, because optimizers won't remove unused arguments by default. These changes add some logic to avoid adding the argument when it isn't required.
PR Close#35097
The completions list don't include the 'ngTemplateOutlet'. The directive is a structural directive when it has injected the 'TemplateRef' or 'ViewContainerRef'.
PR Close#35466
In ES5 and ES2015, class identifiers may have aliases. Previously, the
`NgccReflectionHost`s recognized the following formats:
- ES5:
```js
var MyClass = (function () {
function InnerClass() {}
InnerClass_1 = InnerClass;
...
}());
```
- ES2015:
```js
let MyClass = MyClass_1 = class MyClass { ... };
```
In addition to the above, this commit adds support for recognizing an
alias outside the IIFE in ES5 classes (which was previously not
supported):
```js
var MyClass = MyClass_1 = (function () { ... }());
```
Jira issue: [FW-1869](https://angular-team.atlassian.net/browse/FW-1869)
Partially addresses #35399.
PR Close#35527
`Esm5ReflectionHost#getInnerFunctionDeclarationFromClassDeclaration()`
was already called with `ts.Declaration`, not `ts.Node`, so we can
tighten its parameter type and get rid of a redundant check.
`getIifeBody()` (called inside
`getInnerFunctionDeclarationFromClassDeclaration()`) will check whether
the given `ts.Declaration` is a `ts.VariableDeclaration`.
PR Close#35527
Prior to this commit, decorator handling logic in Ngtsc used `Error` to throw errors. This commit replaces most of these instances with `FatalDiagnosticError` class, which provider a better diagnostics error (including location of the problematic code).
PR Close#35244
Some minification tooling modifies `$localize` calls
to contain a comma sequence of items, where the
cooked and raw values are assigned to variables.
This change improves our ability to recognize these
structures.
Fixes#35376
PR Close#35562
We have to do some extra work in the animations module when we identify a Node environment which we determine based on the `process` global. The problem is that by default Webpack will polyfill the `process`, causing us to incorrectly identify it. These changes make it so that the check isn't thrown off by Webpack.
Fixes#35117.
PR Close#35134
We recently updated chokidar to `3.0.0`. The latest version of
chokidar provides TypeScript types on its own and makes the extra
dependency on the `@types` unnecessary.
This seems to have caused the `build-packages-dist` script to fail with
an error like:
```
[strictDeps] transitive dependency on external/npm/node_modules/chokidar/types/index.d.ts
not allowed. Please add the BUILD target to your rule's deps.
```
It's unclear why that happens, but a reasonable theory would be that
the TS compilation accidentally picked up the types from `chokidar`
instead of `@types/chokidar`, and the strict deps `@bazel/typescript`
check reported this as issue because it's not an explicit target dependency.
PR Close#35371
ngcc uses a lockfile to prevent two ngcc instances from executing at the
same time. Previously, if a lockfile was found the current process would
error and exit.
Now, when in async mode, the current process is able to wait for the previous
process to release the lockfile before continuing itself.
PR Close#35131
In View Engine, host element of dynamically created component received attributes and classes extracted from component's selector. For example, if component selector is `[attr] .class`, the `attr` attribute and `.class` class will be add to host element. This commit adds similar logic to Ivy, to make sure this behavior is aligned with View Engine.
PR Close#34481
Before this change content queries with the `descendants: false` option, as implemented in ivy,
would not descendinto `<ng-container>` elements. This behaviour was different from the way the
View Engine worked. This change alligns ngIvy and VE behaviours when it comes to queries and the
`<ng-container>` elements and fixes a common bugs where a query target was placed inside the
`<ng-container>` element with a * directive on it.
Before:
```html
<needs-target>
<ng-container *ngIf="condition">
<div #target>...</div> <!-- this node would NOT match -->
</ng-container>
</needs-target>
```
After:
```html
<needs-target>
<ng-container *ngIf="condition">
<div #target>...</div> <!-- this node WILL match -->
</ng-container>
</needs-target>
```
Fixes#34768
PR Close#35384
When the same provider is resolved multiple times on the same node, the first invocation had the correct context, but all subsequent ones were incorrect because we were registering the hook multiple times under different indexes in `destroyHooks`.
Fixes#35167.
PR Close#35249
Currently the logic that handles ICUs located outside of i18n blocks may throw exceptions at runtime. The problem is caused by the fact that we store incorrect TNode index for previous TNode (index that includes HEADER_OFFSET) and do not store a flag whether this TNode is a parent or a sibling node. As a result, the logic that assembles the final output uses incorrect TNodes and in some cases throws exceptions (when incompatible structure is extracted from tView.data due to the incorrect index). This commit adjusts the index and captures whether TNode is a parent to make sure underlying logic manipulates correct TNode.
PR Close#35347
In https://github.com/angular/angular/pull/33058, we removed support
for the `ngForm` selector in the NgForm directive. We deleted most
of the deprecation notices in that PR, but we missed a paragraph
of documentation in the API docs for NgForm.
This commit removes the outdated paragraph that makes it seem like
the selector is still around and deprecated (as opposed to removed),
as it might confuse users.
PR Close#35435
Given:
```
<div class="s1" [class]="null" [ngClass]="exp">
```
Notice that `[class]` binding is not a `string`. As a result the existing logic would not concatenate `[class]` with `class="s1"`. The resulting falsy value would than be sent to `ngClass` which would promptly clear all styles on the `<div>`
The new logic correctly handles falsy values for `[class]` bindings.
Fix#35335
PR Close#35350
Root cause is that for perf reasons we cache `LFrame` so that we don't have to allocate it all the time. To be extra fast we clear the `LFrame` on `enterView()` rather that on `leaveView()`. The implication of this strategy is that the deepest `LFrame` will retain objects until the `LFrame` allocation depth matches the deepest object.
The fix is to simply clear the `LFrame` on `leaveView()` rather then on `enterView()`
Fix#35148
PR Close#35156
Includes new feature to honor .bazelignore in external repositories. rules_nodejs 1.3.0 now generates a .bazelignore for the @npm repository so that Bazel ignores the @npm//:node_modules folder.
PR Close#35430
Brings in feat: builtin: expose @npm//foo__all_files filegroup that includes all files in the npm package (https://github.com/bazelbuild/rules_nodejs/commit/8d77827) that is needed for npm_integration_test @npm//puppeteer pkg_tar on OSX (as the OSX Chrrome libs are extracted to paths that contain spaces)
PR Close#35430
In the `loadRenderer` we make an assumption that the value will always be an `LView`, but if there's a directive on the same node which injects `ViewContainerRef` the `LView` will be wrapped in an `LContainer`. These changes add a call to unwrap the value before we try to read the value off of it.
Fixes#35342.
PR Close#35343
There is currently a bug in Chrome 80 that makes Array.reduce
not work according to spec. The functionality in forms that
retrieves controls from FormGroups and FormArrays (`form.get`)
relied on Array.reduce, so the Chrome bug broke forms for
many users.
This commit refactors our forms code to rely on Array.forEach
instead of Array.reduce to fix forms while we are waiting
for the Chrome fix to go live.
See https://bugs.chromium.org/p/chromium/issues/detail?id=1049982.
PR Close#35349
Under strict mode, the language service fails to typecheck nullable
symbols that have already been verified to be non-null.
This generates incorrect (false positive) and confusing diagnostics
for users.
To work around this issue in the short term, this commit changes the
diagnostic message from an error to a suggestion, and prompts users to
use the safe navigation operator (?) or non-null assertion operator (!).
For example, instead of
```typescript
{{ optional && optional.toString() }}
```
the following is cleaner:
```typescript
{{ optional?.toString() }}
{{ optional!.toString() }}
```
Note that with this change, users who legitimately make a typo in their
code will no longer see an error. I think this is acceptable, since
false positive is worse than false negative. However, if users follow
the suggestion, add ? or ! to their code, then the error will be surfaced.
This seems a reasonable trade-off.
References:
1. Safe navigation operator (?)
https://angular.io/guide/template-syntax#the-safe-navigation-operator----and-null-property-paths
2. Non-null assertion operator (!)
https://angular.io/guide/template-syntax#the-non-null-assertion-operator---
PR closes https://github.com/angular/angular/pull/35070
PR closes https://github.com/angular/vscode-ng-language-service/issues/589
PR Close#35200
Currently, would-be binding attributes that are missing binding names
are not parsed as bindings, and fall through as regular attributes. In
some cases, this can lead to a runtime error; trying to assign `#` as a
DOM attribute in an element like in `<div #></div>` fails because `#` is
not a valid attribute name.
Attributes composed of binding prefixes but not defining a binding
should be considered invalid, as this almost certainly indicates an
unintentional elision of a binding by the developer. This commit
introduces error reporting for attributes with a binding name prefix but
no actual binding name.
Closes https://github.com/angular/vscode-ng-language-service/issues/293.
PR Close#34595
Support for re-exports in UMD were added in e9fb5fdb8. This commit adds
some tests for re-exports (similar to the ones used for
`CommonJsReflectionHost`).
PR Close#35312
Andrew Kushnir
Pete Bacon Darwin
Greg Magolan
Miško Hevery
crisbeto
Alex Rickabaugh
ivanwonder
Pusztai Tibor
George Kalpakas
JiaLiPassion
zhuyujie
Paul Gschwendtner
Pawel Kozlowski
Kara Erickson
Alan Agius
Keen Yee Liau
ayazhafiz
Francesco Leardini