`ɵɵgetInheritedFactory()` is called from generated code for a component which extends another class. This function is detected by Closure to have a side effect and is not able to tree shake the component as a result. Marking it with `noSideEffects()` tells Closure it can remove this function under the relevant tree shaking conditions.
PR Close#35769
`ɵɵNgOnChangesFeature()` would set `ngInherit`, which is a side effect and also not necessary. This was pulled out to module scope so the function itself can be pure. Since it only curries another function, the call is entirely unnecessary. Updated the compiler to only generate a reference to this function, rather than a call to it, and removed the extra curry indirection.
PR Close#35769
This marks the function are "pure" and eligible to be tree shaken by Closure. Without this, initializing `ngDevMode` is considered a side effect which prevents this function from being tree shaken and also any component which calls it.
PR Close#35769
This is useful for propagating return values without them being converted to a string. It still provides the same guarantees to Closure, which will assume that the function invoked is pure and can be tree-shaken accordingly.
PR Close#35769
This commit performs a modularization of the Language Service's existing
diagnostic messages. Such a modularization has two primary advantages:
- Centralization and decoupling of error messages from the code that
generates them makes it easy to add/delete/edit diagnostic messages,
and allows for independent iteration of diagnostic messages and
diagnostic generation.
- Prepares for additional features like annotating the locations where a
diagnostic is generated and enabling the configuration of which
diagnostics should be reported by the language service.
Although it would be preferable to place the diagnostics registry in an
independent JSON file, for ease of typing diagnostic types as an enum
variant of 'ts.DiagnosticCategory', the registry is stored as an object.
Part of #32663.
PR Close#35678
Before this change ngIvy implementation of queries would throw upon
encountering null / undefined query result collected from an embedded
view. It turns out that we might have a provider that explicitly provides
a null / undefined value in a place of a token queried for.
This commit removes a check from the ngIvy query implementation that was
asserting on a query result to be defined.
Fixes#35673
PR Close#35796
Before this change `[class]` and `[className]` were both converted into `ɵɵclassMap`. The implication of this is that at runtime we could not differentiate between the two and as a result we treated `@Input('class')` and `@Input('className)` as equivalent.
This change makes `[class]` and `[className]` distinct. The implication of this is that `[class]` becomes `ɵɵclassMap` instruction but `[className]` becomes `ɵɵproperty' instruction. This means that `[className]` will no longer participate in styling and will overwrite the DOM `class` value.
Fix#35577
PR Close#35668
Prior to this commit, i18n attributes defined on `<ng-template>` tags were not processed by the compiler. This commit adds the necessary logic to handle i18n attributes in the same way how these attrs are processed for regular elements.
PR Close#35681
Prior to this patch, the `margin` and `padding` properties were not
detected properly by Firefox due to them being shorthand properties.
This patch ensures that both `margin` and `padding` are converted
read as `top right bottom left` in the event that the shorthand
property detection fails for auto-styling in Angular animations.
Fix#35463 (FW-1886)
PR Close#35701
With this change we spawn workers lazily based on the amount of work that needs to be done.
Before this change we spawned the maximum of workers possible. However, in some cases there are less tasks than the max number of workers which resulted in created unnecessary workers
Reference: #35717
PR Close#35719
The `ng_package` rule currently creates incorrect UMD module exports
if an entry-point has a module name with numbers included.
For example, consider an entry-point called `@angular/cdk/a11y`. The UMD
module name should be `ng.cdk.a11y`. Instead, `ng_package` currently generates
an UMD module export called `ng.cdk.a11Y`.
This is because the logic for converting dash-case to camel case is
invalid as it uses Starlark's `title()` method. The title method
converts text to title case while we actually just want to capitalize
the first letter of a dash-case segment.
Fixes angular/components#18652.
PR Close#35792
Adds a new entry-point to the `@angular/bazel` `ng_package` test that
contains numbers in the name. e.g. `example/a11y`. This test is added
to replicate a bug where the UMD module export for such entry-points
is incorrectly generated. i.e. `example.a11Y` is generated instead of
`example.a11y`.
PR Close#35792
This commit updates the host bindings micro benchmark to run tests with mutliple directives (where each directive contains host bindings). The number of directives is configurable as a constant in the micro benchmark file. This change is needed to have an ability to measure/compare perf in different scenarios.
PR Close#35736
This commit extends the range of tNode types that may have local refs to include `TNodeType.Container` to account for `<ng-template>`s. Original changes in https://github.com/angular/angular/pull/33415 didn't include that type and as a result, an error is thrown at runtime in case an i18n block contains an `<ng-template>` with local refs.
PR Close#35758
When the `NgIf` directive is used in a template, its context variables
can be used to capture the bound value. This is typically used together
with a pipe or function call, where the resulting value is captured in a
context variable. There's two syntax forms available:
1. Binding to `NgIfContext.ngIf` using the `as` syntax:
```html
<span *ngIf="(user$ | async) as user">{{user.name}}</span>
```
2. Binding to `NgIfContext.$implicit` using the `let` syntax:
```html
<span *ngIf="user$ | async; let user">{{user.name}}</span>
```
Because of the semantics of `ngIf`, it is known that the captured
context variable is non-nullable, however the template type checker
would not consider them as such and still report errors when
`strictNullTypes` is enabled.
This commit updates `NgIf`'s context guard to make the types of the
context variables non-nullable, avoiding the issue.
Fixes#34572
PR Close#35125
This commit removes the `NullAstVisitor` and `visitAstChildren` exported
from `packages/compiler/src/expression_parser/ast.ts` because they
contain duplicate and buggy implementation, and their use cases could be
sufficiently covered by `RecursiveAstVisitor` if the latter implements the
`visit` method. This use case is only needed in the language service.
With this change, any visitor that extends `RecursiveAstVisitor` could
just define their own `visit` function and the parent class will behave
correctly.
A bit of historical context:
In language service, we need a way to tranverse the expression AST in a
selective manner based on where the user's cursor is. This means we need a
"filtering" function to decide which node to visit and which node to not
visit. Instead of refactoring `RecursiveAstVisitor` to support this,
`visitAstChildren` was created. `visitAstChildren` duplicates the
implementation of `RecursiveAstVisitor`, but introduced some bugs along
the way. For example, in `visitKeyedWrite`, it visits
```
obj -> key -> obj
```
instead of
```
obj -> key -> value
```
Moreover, because of the following line
```
visitor.visit && visitor.visit(ast, context) || ast.visit(visitor, context);
```
`visitAstChildren` visits every node *twice*.
PR Close#35619
This commit differentiates language service feature and language service
infrastructure tests. This is primarily to make testing of different
components at the development level easier. This commit continues a
small effort to expand our test coverage and normalize testing
structure.
Also adds test coverage to language service tests. We have quite a bit
to go on that front 🙂.
PR Close#35688
The `packages/localize/src/tools` folder was excluded
from the top level `tsconfig.json` which meant that in IDEs
these source files were not being given the correct configuration.
It was originally excluded because it required the native `node` typings
but this is no longer a requirement.
Removing this folder from the exclusion list exposed a new issue
where there was a typings mismatch between `@babel/...` sources
and the associated `@types/babel__...` typings packages.
A clean up of the package.json and yarn.lock appears to fix this.
PR Close#35711
For view and content queries, the Ivy compiler attempts to statically
evaluate the predicate token so that string predicates containing
comma-separated reference names can be split into an array of strings
during compilation. When the predicate is a dynamic value that cannot be
statically interpreted at compile time, the compiler would previously
produce an error. This behavior breaks a use-case where an `InjectionToken`
is being used as query predicate, as the usage of the `new` keyword
prevents such predicates from being statically evaluated.
This commit changes the behavior to no longer produce an error for
dynamic values. Instead, the expression is emitted as is into the
generated code, postponing the evaluation to happen at runtime.
Fixes#34267
Resolves FW-1828
PR Close#35307
Prior to this change, the logic that compiles Injectable in JIT mode used incorrect configuration that triggers a problem when `ChangeDetectorRef` is used as a dependency. This commit updates the logic to generate correct inject instruction to add the `ChangeDetectorRef` dependency in case it's requested in @Injectable class.
PR Close#35706
This commit adds micro benchmark for host bindings, so that we can assess the impact of changes related to host bindings (for example PR #35568).
PR Close#35705
Source-maps in the wild could be badly formatted,
causing the source-map flattening processing to fail
unexpectedly. Rather than causing the whole of ngcc
to crash, we gracefully fallback to just returning the
generated source-map instead.
PR Close#35718
Previously when rendering flattened source-maps, it was assumed that no
mapping would come from a line that is outside the lines of the actual
source content. It turns out this is not a valid assumption.
Now the code that renders flattened source-maps will handle such
mappings, with the additional benefit that the rendered source-map
will only contain mapping lines up to the last mapping, rather than a
mapping line for every content line.
Fixes#35709
PR Close#35718
If a package has a source-map but it does not provide
the actual content of the sources, then the source-map
flattening was crashing.
Now we ignore such mappings that have no source
since we are not able to compute the merged
mapping if there is no source file.
Fixes#35709
PR Close#35718
This commit adds a new ngcc configuration, `ignorableDeepImportMatchers`
for packages. This is a list of regular expressions matching deep imports
that can be safely ignored from that package. Deep imports that are not
ignored cause a warning to be logged.
// FW-1892
Fixes#35615
PR Close#35683
If an injectable has a `useClass`, Ivy injects the token in `useClass`, rather than the original injectable, if the injectable is re-provided under a different token. The correct behavior is that it should inject the re-provided token, no matter whether it has `useClass`.
Fixes#34110.
PR Close#34574
ɵAnimationDriver can be safely removed from private exports as AnimationDriver
is already a public export. Since its already available, we can safely remove
its declaration and migrate its only usage in our repo to rely on the public
AnimationDriver symbol.
PR Close#35690
Template type-checking within the Ivy compiler has been disabled internally
in g3 until compatibility with the whole codebase could be verified. As that
verification is now complete and template type-checking is known to be
compatible with g3, this commit enables it.
PR Close#35672
This commit normalizes hover and util tests in the language service.
This is part of a small effort to simplify and normalize the language
service testing structure, which currently contains specs that are
largely created and left without relation to other tests.
PR Close#35656
It's possible to pass a directive as an input to itself. Consider:
```html
<some-cmp #ref [value]="ref">
```
Since the template type-checker attempts to infer a type for `<some-cmp>`
using the values of its inputs, this creates a circular reference where the
type of the `value` input is used in its own inference:
```typescript
var _t0 = SomeCmp.ngTypeCtor({value: _t0});
```
Obviously, this doesn't work. To resolve this, the template type-checker
used to generate a `null!` expression when a reference would otherwise be
circular:
```typescript
var _t0 = SomeCmp.ngTypeCtor({value: null!});
```
This effectively asks TypeScript to infer a value for this context, and
works well to resolve this simple cycle. However, if the template
instead tries to use the circular value in a larger expression:
```html
<some-cmp #ref [value]="ref.prop">
```
The checker would generate:
```typescript
var _t0 = SomeCmp.ngTypeCtor({value: (null!).prop});
```
In this case, TypeScript can't figure out any way `null!` could have a
`prop` key, and so it infers `never` as the type. `(never).prop` is thus a
type error.
This commit implements a better fallback pattern for circular references to
directive types like this. Instead of generating a `null!` in place for the
reference, a type is inferred by calling the type constructor again with
`null!` as its input. This infers the widest possible type for the directive
which is then used to break the cycle:
```typescript
var _t0 = SomeCmp.ngTypeCtor(null!);
var _t1 = SomeCmp.ngTypeCtor({value: _t0.prop});
```
This has the desired effect of validating that `.prop` is legal for the
directive type (the type of `#ref`) while also avoiding a cycle.
Fixes#35372Fixes#35603Fixes#35522
PR Close#35622
NG6002/NG6003 are errors produced when an NgModule being compiled has an
imported or exported type which does not have the proper metadata (that is,
it doesn't appear to be an @NgModule, or @Directive, etc. depending on
context).
Previously this error message was a bit sparse. However, Github issues show
that this is the most common error users receive when for whatever reason
ngcc wasn't able to handle one of their libraries, or they just didn't run
it. So this commit changes the error message to offer a bit more useful
context, instructing users differently depending on whether the class in
question is from their own project, from NPM, or from a monorepo-style local
dependency.
PR Close#35620
When binding to `[style]` we correctly sanitized/unwrapped properties but we did not do it for the object itself.
```
@HostBinding("style")
style: SafeStyle = this.sanitizer.bypassSecurityTrustStyle(
"background: red; color: white; display: block;"
);
```
Above code would fail since the `[style]` would not unwrap the `SafeValue` and would treat it as object resulting in incorrect behavior.
Fix#35476 (FW-1875)
PR Close#35564
The library used by ngcc to update the source files (MagicString) is able
to generate a source-map but it is not able to account for any previous
source-map that the input text is already associated with.
There have been various attempts to fix this but none have been very
successful, since it is not a trivial problem to solve.
This commit contains a novel approach that is able to load up a tree of
source-files connected by source-maps and flatten them down into a single
source-map that maps directly from the final generated file to the original
sources referenced by the intermediate source-maps.
PR Close#35132
Currently we resolve the `NgModuleRef.componentFactoryResolver` by going through the injector, but the problem is that `ComponentFactoryResolver` has a dependency on `NgModuleRef`, which means that if the module that's attached to the ref tries to inject `ComponentFactoryResolver` in its constructor, we'll create a circular dependency which throws at runtime.
These changes resolve the issue by creating the `ComponentFactoryResolver` manually ahead of time without going through the injector. We can do this safely, because the only dependency for the resolver is the current module ref which is providing it.
Aside from fixing the issue, another advantage to this approach is that it should reduce the amount of generated JS, because it removes a getter and a provider definitio.
Fixes#35580.
PR Close#35637
Technically, function definitions can live anywhere because they are
hoisted. However, in this case Closure optimizations break when exported
function definitions are referred in another static object that is
exported.
The bad pattern is:
```
exports const obj = {f};
export function f() {...}
```
which turns to the following in Closure's module system:
```
goog.module('m');
exports.obj = {f};
function f() {...}
exports.f = f;
```
which badly optimizes to (note module objects are collapsed)
```
var b = a; var a = function() {...}; // now b is undefined.
```
This is an optimizer bug and should be fixed in Closure, but in the
meantime this change is a noop and will unblock other changes we want to
make.
PR Close#32230
For example, '<div><p string-model~{cursor}></p></div>', when provide the hover info for 'string-model', the 'path.head' is root tag 'div'. Use the parent of 'path.tail' instead.
PR Close#35317
Currently if TestBed detects that TestBed.overrideModule was used for module X, transitive scopes are recalculated recursively for all modules that X imports and previously calculated data (stored in cache) is ignored. This behavior was introduced in https://github.com/angular/angular/pull/33787 to fix stale transitive scopes issue (cache was not updated if module overrides are present).
The perf issue comes from a "diamond" problem, where module X is overridden which imports modules A and B, which both import module C. Under previous logic, module C gets its transitive deps recomputed multiple times, during the recompute for both A and B. For deep graphs and big common/shared modules this can be super costly.
This commit updates the logic to recalculate ransitive scopes for the overridden module, while keeping previously calculated scopes of other modules untouched.
PR Close#35454
Prior to this commit the service worker only treated 504 errors as "effectively offline".
This commit changes the behaviour to treat both 503 (Service Unavailable) and 504 as "offline".
Fixes#35571
PR Close#35595
The only test case for `ngFor` exercises an incorrect usage which causes
two bound attributes to be generated . This commit adds a canonical and
correct usage to show the difference between the two.
PR Close#35671
Now Angular doesn't support add event listeners as passive very easily.
User needs to use `elem.addEventListener('scroll', listener, {passive: true});`
or implements their own EventManagerPlugin to do that.
Angular may finally support new template syntax to support passive event, for now,
this commit introduces a temp solution to allow user to define the passive event names
in zone.js configurations.
User can define a global varibale like this.
```
(window as any)['__zone_symbol__PASSIVE_EVENTS'] = ['scroll'];
```
to let all `scroll` event listeners passive.
PR Close#34503
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
we should be documenting when an API is eligible for removal and not when it will be removed.
The actual removal depends on many factors, e.g. if we were able to automate the refactoring to
the recommended API in time or not.
PR Close#35263
Prior to this change, element namespace was not set for host elements of dynamically created components that resulted in incorrect rendering in a browser. This commit adds the logic to pick and set correct namespace for host element when component is created dynamically.
PR Close#35136
In Ivy's template type checker, event bindings are checked in a closure
to allow for accurate type inference of the `$event` parameter. Because
of the closure, any narrowing effects of template guards will no longer
be in effect when checking the event binding, as TypeScript assumes that
the guard outside of the closure may no longer be true once the closure
is invoked. For more information on TypeScript's Control Flow Analysis,
please refer to https://github.com/microsoft/TypeScript/issues/9998.
In Angular templates, it is known that an event binding can only be
executed when the view it occurs in is currently rendered, hence the
corresponding template guard is known to hold during the invocation of
an event handler closure. As such, it is desirable that any narrowing
effects from template guards are still in effect within the event
handler closure.
This commit tweaks the generated Type-Check Block (TCB) to repeat all
template guards within an event handler closure. This achieves the
narrowing effect of the guards even within the closure.
Fixes#35073
PR Close#35193
The `TargetedEntryPointFinder` must work out what the
containing package is for each entry-point that it finds.
The logic for doing this was flawed in the case that the
package was in a path-mapped directory and not in a
node_modules folder. This meant that secondary entry-points
were incorrectly setting their own path as the package
path, rather than the primary entry-point path.
Fixes#35188
PR Close#35227
This commit implements an experimental integration with tsc_wrapped, where
it can load the Angular compiler as a plugin and perform Angular
transpilation at a user's request.
This is an alternative to the current ngc_wrapped mechanism, which is a fork
of tsc_wrapped from several years ago. tsc_wrapped has improved
significantly since then, and this feature will allow Angular to benefit
from those improvements.
Currently the plugin API between tsc_wrapped and the Angular compiler is a
work in progress, so NgTscPlugin does not yet implement any interfaces from
@bazel/typescript (the home of tsc_wrapped). Instead, an interface is
defined locally to guide this standardization.
PR Close#34792
This commit moves the calculation of `ignoreFiles` - the set of files to be
ignored by a consumer of the `NgCompiler` API - from its `prepareEmit`
operation to its initialization. It's now available as a field on
`NgCompiler`.
This will allow a consumer to skip gathering diagnostics for `ignoreFiles`
as well as skip emit.
PR Close#34792
A bug previously caused the template type-checking diagnostics produced by
TypeScript for template expressions to use -99-prefixed error codes. These
codes are converted to "NG" errors instead of "TS" errors during diagnostic
printing. This commit fixes the issue.
PR Close#35146
- Adds `TView` into `LFrame`, read the `TView` from `LView` on `enterView`.
- Before this change the `TView` was ofter looked up from `LView` as `lView[TVIEW]`. This is suboptimal since reading from an Array, requires that the read checks array size before the read. This means that such a read has a much higher cost than reading from the property directly. By passing in the `TView` explicitly it makes the code more explicit and faster.
- Some rearrangements of arguments so that `TView` would come before `LView` for consistency.
PR Close#35069
In #34021 the ngtsc compiler gained the ability to emit type parameter
constraints, which would generate imports for any type reference that
is used within the constraint. However, the `AbsoluteModuleStrategy`
reference emitter strategy did not consider interface declarations as a
valid declaration it can generate an import for, throwing an error
instead.
This commit fixes the issue by including interface declarations in the
logic that determines whether something is a declaration.
Fixes#34837
PR Close#34849
In #33551, a bug in `ngc --watch` mode was fixed so that a component is
recompiled when its template file is changed. Due to insufficient
normalization of files paths, this fix did not have the desired effect
on Windows.
Fixes#32869
PR Close#34015
Inside `*ngFor` the second run of the styling instructions can get into situation where it tries to read a value from a binding which has not yet executed. As a result the read is `NO_CHANGE` value and subsequent property read cause an exception as it is of wrong type.
Fix#35118
PR Close#35133
This commit cleans up `expression_type.ts` by
1. Removing the unnecessary `TypeDiagnostic` class. It's replaced by
`ng.Diagnostic`.
2. Consolidating `reportError()` and `reportWarning()` to
`reportDiagnostic()`.
This is prep work so that we could make some of the type diagnostics a
suggestion in later PRs.
PR Close#35085
`TNode.directives` was introduced in https://github.com/angular/angular/pull/34938. Turns out that it is unnecessary because the information is already present it `TData` when combining with `TNode.directiveStart` and `TNode.directiveEnd`
Mainly this is true (conceptually):
```
expect(tNode.directives).toEqual(
tData.slice(
tNode.directivesStart,
tNode.directivesEnd - tNode.DirectivesStart -1
)
);
```
The refactoring removes `TNode.directives` and adds `TNode.directiveStyling` as we still need to keep location in the directive in `TNode`
PR Close#35050
These tests are used for perf testing and don't run as part of CI, as a result they bit-rotted. This fixes that. Long term these tests should be run as part of CI.
PR Close#35071
To support parallel CLI builds we instruct developers to pre-process
their node_modules via ngcc at the command line.
Despite doing this ngcc was still trying to set a lock when it was being
triggered by the CLI for packages that are not going to be processed,
since they are not compiled by Angular for instance.
This commit checks whether a target package needs to be compiled
at all before attempting to set the lock.
Fixes#35000
PR Close#35057
If ngcc gets updated to a new version then the artifacts
left in packages that were processed by the previous
version are possibly invalid.
Previously we just errored if we found packages that
had already been processed by an outdated version.
Now we automatically clean the packages that have
outdated artifacts so that they can be reprocessed
correctly with the current ngcc version.
Fixes#35082
PR Close#35079
Now `hasBeenProcessed()` will no longer throw if there
is an entry-point that has been built with an outdated
version of ngcc.
Instead it just returns `false`, which will include it in this
processing run.
This is a precursor to adding functionality that will
automatically revert outdate build artifacts.
PR Close#35079
Fixes issue with yarn_install not following yarn-path in .yarnrc when bazel run from yarn with `yarn bazel ...` (rules_nodejs: fix: unset YARN_IGNORE_PATH in yarn_install before calling yarn #1588)
PR Close#34961
Fixes#18013
Previously it was hard to debug an `expectOne` if the request had no match, as the error message was:
Expected one matching request for criteria "Match URL: /some-url?query=hello", found none.
This commit adds a bit more info to the error, by listing the actual requests received:
Expected one matching request for criteria "Match URL: /some-url?query=hello", found none. Requests received are: POST /some-url?query=world.
PR Close#27005
Previous to this commit, HTTP params like `{ a: '1', b: [], c: '3' }` resulted in a request like `a=1&&c=3` (note the double &&).
The ideal fix would probably be to stringify these params to `a=1&b=&c=3` like we do for empty string values. But that might be breaking as some APIs may rely on the absence of the parameter.
This fixes the issue in a compatible way by just removing the extra and unnecessary `&`, resulting in `a=1&c=3`.
PR Close#34896
This reverts commit cb142b6df9.
The intention of this commit was for a consumer of the `compile` function to
pass the `bazelHost` it returns into future invocations, reusing the
`FileCache` between them. However, first-party ngc_wrapped does not do this,
which caused a performance regression as the `FileCache` was no longer
shared between compilations.
PR Close#35063
Update from chokidar 2.x to 3.x in ngc/ngtsc, to eliminate any possibility
of a security issue with a downstream dependency of the package.
FW-1809 #resolve
PR Close#35047
This change changes the priority order of static styling.
Current priority:
```
(least priority)
- Static
- Component
- Directives
- Template
- Dynamic Binding
- Component
- Map/Interpolation
- Property
- Directives
- Map/Interpolation
- Property
- Template
- Map/Interpolation
- Property
(highest priority)
```
The issue with the above priority is this use case:
```
<div style="color: red;" directive-which-sets-color-blue>
```
In the above case the directive will win and the resulting color will be `blue`. However a small change of adding interpolation to the example like so. (Style interpolation is coming in https://github.com/angular/angular/pull/34202)
```
<div style="color: red; width: {{exp}}px" directive-which-sets-color-blue>
```
Changes the priority from static binding to interpolated binding which means now the resulting color is `red`. It is very surprising that adding an unrelated interpolation and style can change the `color` which was not changed. To fix that we need to make sure that the static values are associated with priority of the source (directive or template) where they were declared. The new resulting priority is:
```
(least priority)
- Component
- Static
- Map/Interpolation
- Property
- Directives
- Static
- Map/Interpolation
- Property
- Template
- Static
- Map/Interpolation
- Property
(highest priority)
```
PR Close#34938
The current logic pulls multiproviders up to the parent module's
provider list. The result is that the multi provider being defined both in
the imported ModuleWithProviders and the parent and getting an extra
item in the multi provided array of values. This PR fixes that problem
by not pulling providers in ModuleWithProviders up to the parent module.
PR Close#34914
The language service reports an error when a directive's template
context is missing a member that is being used in a template (e.g. if
`$implicit` is being used with a template context typed as `any`).
While this diagnostic message is valuable, typing template contexts
loosely as `any` or `object` is very widespread in community packages,
and often still compiles correctly, so reporting the diagnostic as an
error may be misleading to users.
This commit changes the diagnostic to be a warning, and adds additional
information about how the user can eliminate the warning entirely -- by
refining the template context type.
Closes https://github.com/angular/vscode-ng-language-service/issues/572
PR Close#35036
Sometimes, a request for definitions will return multiple of the same
definition. This can happen in at least the cases of
- two-way bindings (one of the same definition for the property and
event binding)
- multiple template binding expressions in the same attribute
- something like "*ngFor="let i of items; trackBy: test" has two
template bindings, resulting in two template binding ASTs at the
same location (the attribute span). The language service then parses
both of these bindings individually, resulting in two independent
but identical definitions. For more context, see https://github.com/angular/angular/pull/34847#discussion_r371006680.
This commit prunes duplicate definitions by signing definitions with
their location, and checking if that location signature has been seen in
a previous definition returned to the client.
PR Close#34995
In #34974 the top level dependency on `@babel/core` was bumped to
7.8.3. This commit ensures that the package.json that gets included
in the `@angular/localize` distributable is at the same version.
PR Close#35008
There are different `DebugNode`/`DebugElement` implementations (and
associated helper functions) for ViewEngine and Ivy. Additionally, these
classes/functions, which are defined inside the `core` package, are
imported by the `platform-browser` package.
Previously, this code was not tree-shaken as expected in Ivy. #30130
partially addressed the issue, but only for the case where `core` and
`platform-browser` end up in the same closure after webpack's scope
hoisting. In cases where this is not the case, our webpack/terser based
tooling is not capable of tree-shaking it.
This commit fixes the problem, by ensuring that the code retained in Ivy
mode (due to the cross-package import) does not unnecessarily reference
`DebugNode`/`DebugElement`, allowing the code to be tree-shaken away.
This results in a 7.6KB reduction in the size of the main angular.io
bundle.
Jira issue: [FW-1802](https://angular-team.atlassian.net/browse/FW-1802)
PR Close#35003
The message now gives concrete advice to developers who
experience the error due to running multiple simultaneous builds
via webpack.
Fixes#35000
PR Close#35001
This commit adds support for completions of properties on `$event`
variables in bound outputs.
This is the second major PR to support completions for `$event`
variables (https://github.com/angular/vscode-ng-language-service/issues/531).
The final completion support that must be provided is for `$event`
variables in bindings targeting DOM events, like `(click)`.
PR Close#34570
This commit makes `findOutputBinding` a utility function for the
language service, which will be used by the `expression_diagnostics`
module in #34570. Keeping the function in `locate_symbol` results in a
circular dependency between `expression_diagnostics` and
`locate_symbol`.
PR Close#34997
We had some logic for generating and passing in the `elIndex` parameter into the `hostBindings` function, but it wasn't actually being used for anything. The only place left that had a reference to it was the `StylingBuilder` and it only stored it without referencing it again.
PR Close#34969
Component's decorator handler exposes `preanalyze` method to preload async resources (templates, stylesheets). The logic in preanalysis phase may throw `FatalDiagnosticError` errors that contain useful information regarding the origin of the problem. However these errors from preanalysis phase were not intercepted in TraitCompiler, resulting in just error message text be displayed. This commit updates the logic to handle FatalDiagnosticError and transform it before throwing, so that the result diagnostic errors contain the necessary info.
PR Close#34801
For the structural directive, the 'path' will contain multiple `BoundDirectivePropertyAst` which depends on the number of directive property in the attribute value(e.g. '*ngFor="let item of []; trackBy: test;"', it has 2 `BoundDirectivePropertyAst`, 'ngForOf' and 'ngForTrackBy').
PR Close#34847
the test failures most likely result from the babel updates in the previous commit.
it does look like we lost the file path from the error message, which is something that we
should follow up no in a separate change.
PR Close#34974
Previously we relied on this package being hoisted and available, which is error prone and it would
be just a matter of time before the build would break due rehoisting of deps upon future npm package.json
changes.
PR Close#34974
It's not clear how this ever worked (npm hoisting is a suspect though), but this dep is required because
one of the tests imports @babel/generator in the ts sources.
The lack of this dep is breaking builds on the master branch.
More discussion about this issue on Slack: https://angular-team.slack.com/archives/C07DT5M6V/p1579934766007500
PR Close#34974
As part of the effort to tighten the API surface of
`TypeScriptServiceHost` in preparation for the migration to Ivy, I realized
some recently added APIs are not strictly needed.
They can be safely removed without sacrificing functionality.
This allows us to clean up the code, especially in the implementation of
QuickInfo, where the `TypeScriptServiceHost` is leaked outside of the
`LanguageService` class.
This refactoring also cleans up some duplicate code where the QuickInfo
object is generated. The logic is now consolidated into a simple
`createQuickInfo` method shared across two different implementations.
PR Close#34941
Previously we would write to class/style as strings `element.className` and `element.style.cssText`. Turns out that approach is good for initial render but not good for updates. Updates using this approach are problematic because we have to check to see if there was an out of bound write to style and than perform reconciliation. This also requires the browser to bring up CSS parser which is expensive.
Another problem with old approach is that we had to queue the DOM writes and flush them twice. Once on element advance instruction and once in `hostBindings`. The double flushing is expensive but it also means that a directive can observe that styles are not yet written (they are written after directive executes.)
The new approach uses `element.classList.add/remove` and `element.style.setProperty/removeProperty` API for updates only (it continues to use `element.className` and `element.style.cssText` for initial render as it is cheaper.) The other change is that the styling changes are applied immediately (no queueing). This means that it is the instruction which computes priority. In some circumstances it may result in intermediate writes which are than overwritten with new value. (This should be rare)
Overall this change deletes most of the previous code and replaces it with new simplified implement. The simplification results in code savings.
PR Close#34804
This change introduces several functions for manipulating items in an array in an efficient (binary search) way.
- `arraySplice` a faster version of `Array.splice()`.
- `arrayInsert` a faster version of `Array.splice(index, 0, value)`.
- `arrayInsert2` a faster version of `Array.splice(index, 0, value1, value2)`.
- `arrayInsertSorted` a way to insert a value into sorted list.
- `arrayRemoveSorted` a way to remove a value from a sorted list.
- `arrayIndexOfSorted` a way to find a value in a sorted list.
- `ArrayMap` Efficient implementation of `Map` as an `Array`.
- `arrayMapSet`, `arrayMapGet`, `arrayMapIndexOf`, and `arrayMapDelete` for manipulating `ArrayMap`s.
PR Close#34804
NOTE: This change must be reverted with previous deletes so that it code remains in build-able state.
This change deletes old styling code and replaces it with a simplified styling algorithm.
The mental model for the new algorithm is:
- Create a linked list of styling bindings in the order of priority. All styling bindings ere executed in compiled order and than a linked list of bindings is created in priority order.
- Flush the style bindings at the end of `advance()` instruction. This implies that there are two flush events. One at the end of template `advance` instruction in the template. Second one at the end of `hostBindings` `advance` instruction when processing host bindings (if any).
- Each binding instructions effectively updates the string to represent the string at that location. Because most of the bindings are additive, this is a cheap strategy in most cases. In rare cases the strategy requires removing tokens from the styling up to this point. (We expect that to be rare case)S Because, the bindings are presorted in the order of priority, it is safe to resume the processing of the concatenated string from the last change binding.
PR Close#34616
NOTE: This change deletes code and creates a BROKEN SHA. If reverting this SHA needs to be reverted with the next SHA to get back into a valid state.
PR Close#34616
This change reverts https://github.com/angular/angular/pull/28711
NOTE: This change deletes code and creates a BROKEN SHA. If reverting this SHA needs to be reverted with the next SHA to get back into a valid state.
The change removes the fact that `NgStyle`/`NgClass` is special and colaborates with the `[style]`/`[class]` to merge its styles. By reverting to old behavior we have better backwards compatiblity since it is no longer treated special and simply overwrites the styles (same as VE)
PR Close#34616
Compiler keeps track of number of slots (`vars`) which are needed for binding instructions. Normally each binding instructions allocates a single slot in the `LView` but styling instructions need to allocate two slots.
PR Close#34616
The `computeStaticStyling` will be used for computing static styling value during `firstCreatePass`.
The function takes into account static styling from the template as well as from the host bindings. The host bindings need to be merged in front of the template so that they have the correct priority.
PR Closes#34418
Parsing styling is now simplified to be used like so:
```
for (let i = parseStyle(text); i <= 0; i = parseStyleNext(text, i)) {
const key = getLastParsedKey();
const value = getLastParsedValue();
...
}
```
This change makes it easier to invoke the parser from other locations in the system without paying the cost of creating and iterating over `Map` of styles.
PR Closes#34418
This change moves information from instructions to declarative position:
- `ɵɵallocHostVars(vars)` => `DirectiveDef.hostVars`
- `ɵɵelementHostAttrs(attrs)` => `DirectiveDef.hostAttrs`
When merging directives it is necessary to know about `hostVars` and `hostAttrs`. Before this change the information was stored in the `hostBindings` function. This was problematic, because in order to get to the information the `hostBindings` would have to be executed. In order for `hostBindings` to be executed the directives would have to be instantiated. This means that the directive instantiation would happen before we had knowledge about the `hostAttrs` and as a result the directive could observe in the constructor that not all of the `hostAttrs` have been applied. This further complicates the runtime as we have to apply `hostAttrs` in parts over many invocations.
`ɵɵallocHostVars` was unnecessarily complicated because it would have to update the `LView` (and Blueprint) while existing directives are already executing. By moving it out of `hostBindings` function we can access it statically and we can create correct `LView` (and Blueprint) in a single pass.
This change only changes how the instructions are generated, but does not change the runtime much. (We cheat by emulating the old behavior by calling `ɵɵallocHostVars` and `ɵɵelementHostAttrs`) Subsequent change will refactor the runtime to take advantage of the static information.
PR Close#34683
This adds `insertTStyleValue` but does not hook it up to anything yet.
The purpose of this function is to create a linked-list of styling
related bindings. The bindings can be traversed during flush.
The linked list also keeps track of duplicates. This is important
for binding to know if it needs to check other styles for reconciliation.
PR Close#34004
This change introduces class/style reconciliation algorithm for DOM elements.
NOTE: The code is not yet hooked up, it will be used by future style algorithm.
Background:
Styling algorithm currently has [two paths](https://hackmd.io/@5zDGNGArSxiHhgvxRGrg-g/rycZk3N5S)
when computing how the style should be rendered.
1. A direct path which concatenates styling and uses `elemnent.className`/`element.style.cssText` and
2. A merge path which uses internal data structures and uses `element.classList.add/remove`/`element.style[property]`.
The situation is confusing and hard to follow/maintain. So a future PR will remove the merge-path and do everything with
direct-path. This however breaks when some other code adds class or style to the element without Angular's knowledge.
If this happens instead of switching from direct-path to merge-path algorithm, this change provides a different mental model
whereby we always do `direct-path` but the code which writes to the DOM detects the situation and reconciles the out of bound write.
The reconciliation process is as follows:
1. Detect that no one has modified `className`/`cssText` and if so just write directly (fast path).
2. If out of bounds write did occur, switch from writing using `className`/`cssText` to `element.classList.add/remove`/`element.style[property]`.
This does require that the write function computes the difference between the previous Angular expected state and current Angular state.
(This requires a parser. The advantage of having a parser is that we can support `style="width: {{exp}}px" kind of bindings.`)
Compute the diff and apply it in non destructive way using `element.classList.add/remove`/`element.style[property]`
Properties of approach:
- If no out of bounds style modification:
- Very fast code path: Just concatenate string in right order and write them to DOM.
- Class list order is preserved
- If out of bounds style modification detected:
- Penalty for parsing
- Switch to non destructive modification: `element.classList.add/remove`/`element.style[property]`
- Switch to alphabetical way of setting classes.
PR Close#34004
Fixes Ivy detecting changes inside child embedded views, even though they're detached.
Note that there's on subtlety here: I made the changes inside `refreshDynamicEmbeddedViews` rather than `refreshView`, because we support detecting changes on a detached view (evidenced by a couple of unit tests), but only if it's triggered directly from the view's `ChangeDetectorRef`, however we shouldn't be detecting changes in the detached child view when something happens in the parent.
Fixes#34816.
PR Close#34846
Previously, NgtscProgram lived in the main @angular/compiler-cli package
alongside the legacy View Engine compiler. As a result, the main package
depended on all of the ngtsc internal packages, and a significant portion of
ngtsc logic lived in NgtscProgram.
This commit refactors NgtscProgram and moves the main logic of compilation
into a new 'core' package. The new package defines a new API which enables
implementers of TypeScript compilers (compilers built using the TS API) to
support Angular transpilation as well. It involves a new NgCompiler type
which takes a ts.Program and performs Angular analysis and transformations,
as well as an NgCompilerHost which wraps an input ts.CompilerHost and adds
any extra Angular files.
Together, these two classes are used to implement a new NgtscProgram which
adapts the legacy api.Program interface used by the View Engine compiler
onto operations on the new types. The new NgtscProgram implementation is
significantly smaller and easier to reason about.
The new NgCompilerHost replaces the previous GeneratedShimsHostWrapper which
lived in the 'shims' package.
A new 'resource' package is added to support the HostResourceLoader which
previously lived in the outer compiler package.
As a result of the refactoring, the dependencies of the outer
@angular/compiler-cli package on ngtsc internal packages are significantly
trimmed.
This refactoring was driven by the desire to build a plugin interface to the
compiler so that tsc_wrapped (another consumer of the TS compiler APIs) can
perform Angular transpilation on user request.
PR Close#34887
Right now, if an Angular diagnostic is generated for a TypeScript node,
the span points to the decorator Identifier, i.e. the Identifier node
like `@NgModule`, `@Component`, etc.
This is weird. It should point to the class name instead.
Note, we do not have a more fine-grained breakdown of the span when
diagnostics are emitted, this work remains to be done.
PR Close#34932
In #34288, ngtsc was refactored to separate the result of the analysis
and resolve phase for more granular incremental rebuilds. In this model,
any errors in one phase transition the trait into an error state, which
prevents it from being ran through subsequent phases. The ngcc compiler
on the other hand did not adopt this strict error model, which would
cause incomplete metadata—due to errors in earlier phases—to be offered
for compilation that could result in a hard crash.
This commit updates ngcc to take advantage of ngtsc's `TraitCompiler`,
that internally manages all Ivy classes that are part of the
compilation. This effectively replaces ngcc's own `AnalyzedFile` and
`AnalyzedClass` types, together with all of the logic to drive the
`DecoratorHandler`s. All of this is now handled in the `TraitCompiler`,
benefiting from its explicit state transitions of `Trait`s so that the
ngcc crash is a thing of the past.
Fixes#34500
Resolves FW-1788
PR Close#34889
This syntax is invalid in these source files and does result in
compilation errors as the constructor parameters could not be resolved.
This hasn't been an issue until now as those errors were ignored in the
tests, but future work to introduce the Trait system of ngtsc into
ngcc will cause these errors to prevent compilation, resulting in broken
tests.
PR Close#34889
Previously, while trying to build an `NgccReflectionHost`'s
`privateDtsDeclarationMap`, `computePrivateDtsDeclarationMap()` would
try to collect exported declarations from all source files of the
program (i.e. without checking whether they were within the target
package, as happens for declarations in `.d.ts` files).
Most of the time, that would not be a problem, because external packages
would be represented as `.d.ts` files in the program. But when an
external package had no typings, the JS files would be used instead. As
a result, the `ReflectionHost` would try to (unnecessarilly) parse the
file in order to extract exported declarations, which in turn would be
harmless in most cases.
There are certain cases, though, where the `ReflectionHost` would throw
an error, because it cannot parse the external package's JS file. This
could happen, for example, in `UmdReflectionHost`, which expects the
file to contain exactly one statement. See #34544 for more details on a
real-world failure.
This commit fixes the issue by ensuring that
`computePrivateDtsDeclarationMap()` will only collect exported
declarations from files within the target package.
Jira issue: [FW-1794](https://angular-team.atlassian.net/browse/FW-1794)
Fixes#34544
PR Close#34811
The value changes emitted additionally when enable disable were called
documented the above behaviour in AbstractControl class documentaion
Fixes#34407
PR Close#34497
This commit fixes a bug in the incremental rebuild engine of ngtsc, where if
a component was removed from its NgModule, it would not be properly
re-emitted.
The bug stemmed from the fact that whether to emit a file was a decision
based purely on the updated dependency graph, which captures the dependency
structure of the rebuild program. This graph has no edge from the component
to its former module (as it was removed, of course), so the compiler
erroneously decides not to emit the component.
The bug here is that the compiler does know, from the previous dependency
graph, that the component file has logically changed, since its previous
dependency (the module file) has changed. This information was not carried
forward into the set of files which need to be emitted, because it was
assumed that the updated dependency graph was a more accurate source of that
information.
With this commit, the set of files which need emit is pre-populated with the
set of logically changed files, to cover edge cases like this.
Fixes#34813
PR Close#34912
It was previously defined in core without being exposed publicly, whereas `getLocaleCurrencyName` and `getLocaleCurrencySymbol` were defined in common, and publicly exposed.
This commit now privately exposes `ɵgetLocaleCurrencyCode` from core, and reexports it publicly from common.
PR Close#34810
This commit elaborates diagnostics produced for invalid template
contexts by including the name of the embedded template type using the
template context, and in the common case that the implicity property is
being referenced (e.g. in a `for .. of ..` expression), suggesting to
refine the type of the context. This suggestion is provided because
users will sometimes use a base class as the type of the context in the
embedded view, and a more specific context later on (e.g. in an
`ngOnChanges` method).
Closes https://github.com/angular/vscode-ng-language-service/issues/251
PR Close#34751
Previously, the template type-checker would always construct a generic
template context type with correct bounds, even when strictTemplates was
disabled. This meant that type-checking of expressions involving that type
was stricter than View Engine.
This commit introduces a 'strictContextGenerics' flag which behaves
similarly to other 'strictTemplates' flags, and switches the inference of
generic type parameters on the component context based on the value of this
flag.
PR Close#34649
FileToModuleHost aliasing supports compilation within environments that have
two properties:
1. A `FileToModuleHost` exists which defines canonical module names for any
given TS file.
2. Dependency restrictions exist which prevent the import of arbitrary files
even if such files are within the .d.ts transitive closure of a
compilation ("strictdeps").
In such an environment, generated imports can only go through import paths
which are already present in the user program. The aliasing system supports
the generation and consumption of such imports at runtime.
`FileToModuleHost` aliasing does not emit re-exports in .d.ts files. This
means that it's safe to rely on alias re-exports in generated .js code (they
are guaranteed to exist at runtime) but not in template type-checking code
(since TS will not be able to follow such imports). Therefore, non-aliased
imports should be used in template type-checking code.
This commit adds a `NoAliasing` flag to `ImportFlags` and sets it when
generating imports in template type-checking code. The testing environment
is also patched to support resolution of FileToModuleHost canonical paths
within the template type-checking program, enabling testing of this change.
PR Close#34649
Previously, `ReferenceEmitter.emit()` took an `ImportMode` enum value, where
one value of the enum allowed forcing new imports to be generated when
emitting a reference to some value or type.
This commit refactors `ImportMode` to be an `ImportFlags` value instead.
Using a bit field of flags will allow future customization of reference
emitting.
PR Close#34649
Previously, when generating template type-checking code, casts to 'any' were
produced as `expr as any`, regardless of the expression. However, for
certain expression types, this led to precedence issues with the cast. For
example, `a !== b` is a `ts.BinaryExpression`, and wrapping it directly in
the cast yields `a !== b as any`, which is semantically equivalent to
`a !== (b as any)`. This is obviously not what is intended.
Instead, this commit adds a list of expression types for which a "bare"
wrapping is permitted. For other expressions, parentheses are added to
ensure correct precedence: `(a !== b) as any`
PR Close#34649
Currently, the template type-checker gives an error if there are multiple
bindings to the same input. This commit aligns the behavior of the template
type-checker with the View Engine runtime: only the first binding to a field
has any effect. The rest are ignored.
PR Close#34649
It's possible to declare multiple inputs for a directive/component which all
map to the same property name. This is usually done in error, as only one of
any bindings to the property will "win".
In the template type-checker, an error was previously being raised as a
result of this ambiguity. Specifically, a type constructor was produced
which required a binding for each field, but only one of the fields had
a value via the binding. TypeScript would (rightfully) error on missing
values for the remaining fields. This ultimately was happening when the
code which generated the default values for "unset" inputs belonging to
directives or pipes used the final mapping from properties to fields as
a source for field names.
Instead, this commit uses the original list of fields to generate unset
input values, which correctly provides values for fields which shared a
property name but didn't receive the final binding.
PR Close#34649
Consider a library that uses a shared constant for host bindings. e.g.
```ts
export const BASE_BINDINGS= {
'[class.mat-themed]': '_isThemed',
}
----
@Directive({
host: {...BASE_BINDINGS, '(click)': '...'}
})
export class Dir1 {}
@Directive({
host: {...BASE_BINDINGS, '(click)': '...'}
})
export class Dir2 {}
```
Previously when these components were shipped as part of the
library to NPM, consumers were able to consume `Dir1` and `Dir2`.
No errors showed up.
Now with Ivy, when ngcc tries to process the library, an error
will be thrown. The error is stating that the host bindings should
be an object (which they obviously are). This happens because
TypeScript transforms the object spread to individual
`Object.assign` calls (for compatibility).
The partial evaluator used by the `@Directive` annotation handler
is unable to process this expression because there is no
integrated support for `Object.assign`. In View Engine, this was
not a problem because the `metadata.json` files from the library
were used to compute the host bindings.
Fixes#34659
PR Close#34661
Ngcc adds properties to the `package.json` files of the entry-points it
processes to mark them as processed for a format and point to the
created Ivy entry-points (in case of `--create-ivy-entry-points`). When
running ngcc in parallel mode (which is the default for the standalone
ngcc command), multiple formats can be processed simultaneously for the
same entry-point and the order of completion is not deterministic.
Previously, ngcc would append new properties at the end of the target
object in `package.json` as soon as the format processing was completed.
As a result, the order of properties in the resulting `package.json`
(when processing multiple formats for an entry-point in parallel) was
not deterministic. For tools that use file hashes for caching purposes
(such as Bazel), this lead to a high probability of cache misses.
This commit fixes the problem by ensuring that the position of
properties added to `package.json` files is deterministic and
independent of the order in which each format is processed.
Jira issue: [FW-1801](https://angular-team.atlassian.net/browse/FW-1801)
Fixes#34635
PR Close#34870
This commit adds a regression test to check that the language service
recognizes inputs and outputs declared in a directive decorator.
See #34874.
PR Close#34875
In Angular, symbol can have multiple definitions (e.g. a two-way
binding). This commit adds support for for multiple definitions for a
queried location in a template.
PR Close#34782
by DebugElement.triggerEventHandler. ZoneJS tracks the eventListeners on
a node but we need to be able to differentiate between those added by
Angular and those that were added outside the Angular context. This fix
aligns with the behavior that was present in View Engine (not calling
those listeners). If we decide later that we want to call those
listeners, we still need a way to differentiate between those that
we have wrapped in dom_renderer and those that were not (because they
were added outside the Angular context).
PR Close#34514
The Angular CLI will continue to call ngcc on all possible packages, even if they
have already been processed by ngcc in a postinstall script.
In a parallel build environment, this was causing ngcc to complain that it was
being run in more than one process at the same time.
This commit moves the check for whether the targeted package has been
processed outside the locked code section, since there is no issue with
multiple ngcc processes from doing this check.
PR Close#34722
Previously, it was possible for multiple instance of ngcc to be running
at the same time, but this is not supported and can cause confusing and
flakey errors at build time.
Now, only one instance of ngcc can run at a time. If a second instance
tries to execute it fails with an appropriate error message.
See https://github.com/angular/angular/issues/32431#issuecomment-571825781
PR Close#34722
This commit adds an `exclusive` parameter to the
`FileSystem.writeFile()` method. When this parameter is
true, the method will fail with an `EEXIST` error if the
file already exists on disk.
PR Close#34722
Fixes Ivy throwing an error when trying to access the `DebugNode.classes` of an SVG element. The problem is that the `className` of an SVG element is an `SVGAnimatedString`, rather than a plain string.
Fixes#34868.
PR Close#34872
This commit makes the Angular Language Service interface a strict subset
of TypeScript's Language Service by renaming all methods to be
consistent with TypeScript's.
The custom Angular `LanguageService` interface was needed before the
inception of TypeScript tsserver plugin, but is now obsolete since
Angular LS is a proper tsserver plugin.
This allows us to easily adapt to upstream TS changes in the future, and
also allows us to reuse all data types defined in TypeScript.
PR Close#34888
This patch removes the need for the styleSanitizer() instruction in
favor of passing the sanitizer into directly into the styleProp
instruction.
This patch also increases the binding index size for all style/class bindings in preparation for #34418
PR Close#34480
Pipes in host binding expressions are not supported in View Engine and Ivy, but in some more complex cases (like `(value | pipe) === true`) compiler was not reporting errors. This commit extends Ivy logic to detect pipes in host binding expressions and throw in cases bindings are present. View Engine behavior remains the same.
PR Close#34655
In #28162 we introduced an extra `removeNode` call for host elements which can cause the parent element to be removed before all child animations have finished. The issue is only in Ivy, because that the only place where we pass in the `isHostElement` flag. These changes fix the issue by not re-triggering the removal logic if the element has in-progress animations.
Fixes#33597.
PR Close#34702
Before ivy it was possible to configure a mutable service value
in an application initializer (by providing an `APP_INITIALIZER`)
that could be read in the provider of `LOCALE_ID`. This is a common
scenario if you wanted to load the locale id asynchronously from
an HTTP request for instance.
When using the ivy, the runtime needs to be told what the current
locale is, which is done by calling the `setLocaleId()` function with
the value injected by the `LOCALE_ID` token. Previously this was
being done before the application initializers were run, which meant
that the `LOCALE_ID` provider was being executed before the
app initializers had a chance to get a new value for it.
Now this initalization of the locale for the ivy runtime is done after the
application initializers have been run.
Closes#34701
PR Close#34830
Since I was learning the codebase and had a hard time understanding what was going on I've done a
bunch of changes in one commit that under normal circumstances should have been split into several
commits. Because this code is likely going to be overwritten with Misko's changes I'm not going to
spend the time with trying to split this up.
Overall I've done the following:
- I processed review feedback from #34307
- I did a bunch of renaming to make the code easier to understand
- I refactored some internal functions that were either inefficient or hard to read
- I also updated lots of type signatures to correct them and to remove many casts in the code
PR Close#34307
Prior to this change, in Ivy mode ngStyle/ngClass would accidentally emit value changes for static
(string-based) values even if the value itself had not changed. This patch ensures that
the style/class diffing code is more strict and when it signals ngClass/ngStyle that there has been
a value change.
Fixes#34336, #34444
PR Close#34307
remove unnecessary underscore suffix and the corresponding TODO comments,
because the rollup bug was fixed: github.com/rollup/rollup/issues/2047
PR Close#34757
Typescript 3.7 now emits d.ts files for getters differently than prior versions,
and there seems to be a bug in how it strips private types without replacing them
with explicit 'any' type. This then leads to compilation failures in projects compiled
against our packages that don't have skipLibCheck turned on but do have strict or
noImplicitAny check on.
I'm working around this by marking the affected getters as @internal and
adding a test to prevent future regressions.
I believe this is a TypeScript bug, and I filed a bug report:
https://github.com/microsoft/TypeScript/issues/36216
PR Close#34798
This release resolves the bootstrap require patching issue with jasmine_node_test. Require patches are now included before any bootstrap scripts.
PR Close#34736
This is recommended in the Bazel docs as $(location) is ambiguous and can mean either $(execpath) or $(rootpath) depending on the context.
PR Close#34736
This brings in a few minor fixes including a better way to patch require for bootstrap scripts
Also remove install_source_map_support attribute from nodejs_binary targets This attribute will be removed from nodejs_binary in the future
PR Close#34736
For the purposes of the integration test the zone.js script & bundle script tags can just go into the source index.html itself. The purpose of the integration test is is to test @angular/bazel & ng_module & ng_package so there is no need to exercise html_insert_assets in integration/bazel.
PR Close#34736
The major one that affects the angular repo is the removal of the bootstrap attribute in nodejs_binary, nodejs_test and jasmine_node_test in favor of using templated_args --node_options=--require=/path/to/script. The side-effect of this is that the bootstrap script does not get the require.resolve patches with explicitly loading the targets _loader.js file.
PR Close#34736
crisbeto
Doug Parker
ayazhafiz
Pawel Kozlowski
Miško Hevery
Andrew Kushnir
Matias Niemelä
Alan Agius
Paul Gschwendtner
JoostK
Keen Yee Liau
Pete Bacon Darwin
JiaLiPassion
Joey Perrott
Alex Rickabaugh
Ayaz Hafiz
KostyaTretyak
Lars Gyrup Brink Nielsen
Zac
Felix Becker
Rado Kirov
ivanwonder
Alex Wiese
Alex Eagle
Greg Magolan
Pusztai Tibor
George Kalpakas
zhuyujie
Kara Erickson
Francesco Leardini
Igor Minar
Stephen Fluin
Andrew Scott
Sonu Kapoor
cexbrayat
Adam
ajitsinghkaler
Martin Probst
Mike Casebolt