Prior to this commit, host element of a view created via TestBed.createComponent was not attached to the component's host, making it problematic to use TestBed.createComponent API in component factories, which might be used for testing purposes only. This behavior is observed in google3 app tests and was supported by VE, so this commit aligns Ivy and VE.
PR Close#31318
Adds chaining to the `property`, `attribute` and `updateSyntheticHostBinding` instructions when they're used in a host binding.
This PR resolves FW-1404.
PR Close#31296
When `walkTNodeTree` was refactored, the case of ICU expressions was forgotten (because it was handled in the `else` previously).
This PR fixes that to handle it like `ElementContainer`.
FW-1411 #resolve
PR Close#31313
Currently in Ivy whenever we encounter a new namespace, we set it in the global state so that all subsequent nodes are created under the same namespace. Next time a template is run the namespace will be reset back to HTML.
This breaks down if the last node that was rendered was under the SVG or MathML namespace and we create a component through `ViewContainerRef.create`, because the next template function hasn't run yet and it hasn't had the chance to update the namespace. The result is that the root node of the new component will retain the wrong namespace and may not end up rendering at all (e.g. if we're trying to show a `div` inside the SVG namespace). This issue has the potential to affect a lot of apps, because all components inserted through the router also go through `ViewContainerRef.create`.
PR Close#31232
These files have not been formatted properly, due to issues in the
`gulp format*` tasks. See previous commits (or #31295) for more details.
PR Close#31295
The Angular runtime frequently calls into user code (for example, when
writing to a property binding). Since user code can throw errors, calls to
it are frequently wrapped in a try-finally block. In Ivy, the following
pattern is common:
```typescript
enterView();
try {
callUserCode();
} finally {
leaveView();
}
```
This has a significant problem, however: `leaveView` has a side effect: it
calls any pending lifecycle hooks that might've been scheduled during the
current round of change detection. Generally it's a bad idea to run
lifecycle hooks after the application has crashed. The application is in an
inconsistent state - directives may not be instantiated fully, queries may
not be resolved, bindings may not have been applied, etc. Invariants that
the app code relies upon may not hold. Further crashes or broken behavior
are likely.
Frequently, lifecycle hooks are used to make assertions about these
invariants. When these assertions fail, they will throw and "swallow" the
original error, making debugging of the problem much more difficult.
This commit modifies `leaveView` to understand whether the application is
currently crashing, via a parameter `safeToRunHooks`. This parameter is set
by modifying the above pattern:
```typescript
enterView();
let safeToRunHooks = false;
try {
callUserCode();
safeToRunHooks = true;
} finally {
leaveView(..., safeToRunHooks);
}
```
If `callUserCode` crashes, then `safeToRunHooks` will never be set to `true`
and `leaveView` won't call any further user code. The original error will
then propagate back up the stack and be reported correctly. A test is added
to verify this behavior.
PR Close#31244
Brings in ts_library fixes required to get angular/angular building after 0.32.0:
typescript: exclude typescript lib declarations in node_module_library transitive_declarations
typescript: remove override of @bazel/tsetse (+1 squashed commit)
@npm//node_modules/foobar:foobar.js labels changed to @npm//:node_modules/foobar/foobar.js with fix for bazelbuild/rules_nodejs#802
also updates to rules_rass commit compatible with rules_nodejs 0.32.0
PR Close#31019
In ViewEngine injecting a Renderer2 returns a renderer that is specific to the particular component, however in Ivy we inject the renderer for the parent view instead. This causes it to set incorrect `ngcontent` attributes when creating elements through the renderer.
The issue comes from the fact that the `Renderer2` is created according to the current `LView`, but because DI happens before we've entered the `LView` of the component that's injecting the renderer, we end up with one that's one level up. We work around the issue by finding the `LView` that corresponds to the `previousOrParentTNode` inside of the parent view and associating the `Renderer2` with it.
This PR resolves FW-1382.
PR Close#31063
Previously, multiple ViewContainerRef instances (obtained by injecting
ViewContainerRef multiple times) each had private state that could be out of
sync with actual LContainer, if views were inserted/removed/queried across
the different instances. In particular each instance had its own array which
tracked ViewRefs inserted via that instance.
This commit moves the ViewRefs array onto the LContainer itself, so that it
can be shared across multiple ViewContainerRef instances. A test is added
that verifies ViewContainerRefs now provide a consistent view of the
container.
FW-1377 #resolve
PR Close#30985
The function `bind` has been internalized wherever it was needed, this PR makes sure that it is no longer publicly exported.
FW-1385 #resolve
PR Close#31131
Currently the `RecursiveAstVisitor` that is part of the template expression
parser does not _always_ properly pass through the context that can be
specified when visting a given expression. Only a handful of AST types
pass through the context while others are accidentally left out. This causes
unexpected and inconsistent behavior and basically makes the `context`
parameter not usable if the type of template expression is not known.
e.g. the template variable assignment migration currently depends on
the `RecursiveAstVisitor` but sometimes breaks if developers use
things like conditionals in their template variable assignments.
Fixes#31043
PR Close#31085
Currently the `placeholders` parameter inside `i18nLocalize` is defaulted to `{}`, which means that we'll always hit the `Object.keys` call below. Since it's very likely that we won't have placeholders in the majority of strings, these changes add an extra guard so that we don't hit it on every invocation.
PR Close#31106
i18nExp now uses `bind` internally rather than having the compiler generate it in order to bring it in line with other functions like `textBinding` & `property`.
FW-1384 #resolve
PR Close#31089
When using `createEmbeddedView` after the creation of an ICU expression, the nodes for the current selected case were not reprojected (only the anchor comment node was moved to the new location).
Now we reproject correctly all the child nodes of an ICU expression when an anchor comment node is projected.
FW-1372 #resolve
PR Close#30979
- Splits core functionality off into a shared internal function
- ɵɵtextBinding will no longer require an index
- Alters the compiler to stop generating an index argument for the instruction
- Updates tests
- Updates some usage of ɵɵtextBinding in i18n to use the helper function instead
PR Close#30792
The DI's `locateDirectiveOrProvider` function operates on `TView` / `TNode`
data structures only so doesn't need to access `LView`. This refactoring
changes the argument list so the mentioned function takes less info to do
its work.
This refactoring is also mandatory for the upcoming query matching move
to TView.
PR Close#31006
Currently in Ivy we throw when attempting to destroy a `ComponentRef` that has been destroyed, however in ViewEngine we didn't which can cause some tests to break. These changes remove the error to match ViewEngine.
These changes resolve FW-1379.
PR Close#31022
To provide some context: The implicit receiver is part of the
parsed Angular template AST. Any property reads in bindings,
interpolations etc. read from a given object (usually the component
instance). In that case there is an _implicit_ receiver which can also
be specified explicitly by just using `this`.
e.g.
```html
<ng-template>{{this.myProperty}}</ng-template>
```
This works as expected in Ivy and View Engine, but breaks in case the
implicit receiver is not used for property reads. For example:
```html
<my-dir [myFn]="greetFn.bind(this)"></my-dir>
```
In that case the `this` will not be properly translated into the generated
template function code because the Ivy compiler currently always treats
the `ctx` variable as the implicit receiver. This is **not correct** and breaks
compatibility with View Engine. Rather we need to ensure that we retrieve
the root context for the standalone implicit receiver similar to how it works
for property reads (as seen in the example above with `this.myProperty`)
Note that this requires some small changes to the `expression_converter`
because we only want to generate the `eenextContent()` instruction if the
implicit receiver is _actually_ used/needed. View Engine determines if that is the case by recursively walking through the converted output AST and
checking for usages of the `o.variable('_co')` variable ([see here][ve_check]). This would work too for Ivy, but involves most likely more code duplication
since templates are isolated in different functions and it another pass
through the output AST for every template expression.
[ve_check]: 0d6c9d36a1/packages/compiler/src/view_compiler/view_compiler.ts (L206-L208)
Resolves FW-1366.
PR Close#30897
In a derived service class with no decorator (and therefore no factory) of
its own, the factory function of the base class will be used instead.
Previously this logic had a bug where the factory function would be called
with no arguments, which would incorrectly create an instance of the base
class.
This commit adds logic to call the base class' factory and pass the type of
the derived class, which will correctly construct an instance of the
derived class using the base class' factory. A test is also added to verify
correctness of this behavior.
PR Close#30855
Factory functions written by the compiler optionally allow an explicit type
to be passed. If called with this type, an instance of the given type will
be created instead of the type for which the factory was generated. This is
used to power inheritance of Angular types, as if the constructor of a class
is inherited from its superclass, then the factory function of the
superclass must be used (it has all the DI info) to construct an instance of
the derived class.
This commit adjusts typings in a few places to allow factory functions to be
called with this extra type parameter.
PR Close#30855
In older browsers such as IE10, inheritance of static properties on class
types is implemented by copying the properties. This makes hasOwnProperty
unreliable for checking if a static property is defined directly on a class
or is inherited. This causes problems when trying to read the
ngInjectableDef properties of inherited services, as in IE10 even inherited
definitions will appear to be local.
Instead, the 'token' property on ngInjectableDef can be leveraged to detect
when a definition is read which doesn't belong to the type itself.
PR Close#30855
The compiler generates a 'token' field when it emits an ngInjectableDef,
but this field was not required by defineInjectable or the InjectableDef
interface, nor was it added by InjectionToken.
This commit makes 'token' required and adds it where missing.
PR Close#30855
17d87d4e10 has been created before the changes
to the `@ViewChild` and `@ContentChild` decorators. Meaning that it still uses the queries
without the `static` flag. This results in failures in `master` because #30625 has been merged.
PR Close#30967
`TView.firstChild` was defined as the first node with index 0, but when we use runtime i18n then the instruction `i18nStart` can be the one with index 0 and the first node will be index 1 or more.
With this fix we set `TView.firstChild` when we see the first node with index 0 or later if `TView.firstChild` is still null.
FW-1367 #resolve
PR Close#30920
This work is being done ahead of changes to how view insertion is done in Ivy in accordance with [this design document](https://hackmd.io/Ae3W_2pOQlKouu9YNy1t6A?view).
The idea is to make sure we have acceptance tests ahead of that change.
PR Close#30625
- Refactors compiler to stop generating `ɵɵselect(0)` instructions
- Alters template execution to always call the equivalent of `ɵɵselect(0)` before running a template in update mode
- Updates tests to not check for or call `ɵɵselect(0)`.
The goal here is to reduce the size of generated templates
PR Close#30830
The `renderStringify` function shows up pretty high in the CPU profiling.
Turns out that this function contained unnecessary `typeof` check for
function types - the check only makes sense / is used in error messages.
The PR also alligns how ivy and view engine stringify functions used in
interpolations.
PR Close#30838
In Ivy the `DebugNode.attributes` is populated directly from the DOM, but the problem is that the browser will lowercase all attribute names. These changes preserve the case by first going through the `TNode.attrs`, populating the map with the case-sensitive names and saving a reference to the lower case name. Afterwards when we're going through the attributes from the DOM, we can check whether we've mapped the attribute by its case-sensitive name already.
This PR resolves FW-1358.
PR Close#30864
A directive input that doesn't use a binding is triggered during the creation phase. But this was only executed at the first template pass, and not on subsequent ones.
Now only the creation of the update instruction is executed on the first template pass, anything else is executed every time a template is created.
FW-1361 #resolve
PR Close#30870
When translated content was projected, all of the content was reappended, even the placeholders that had been removed in the translation.
To avoid that we added a new flag on `TNode` that specifies that a node is detached, in which case it should be ignored by the projection.
FW-1319 #resolve
PR Close#30783
- Removes ɵɵelementProperty instruction
- Updates tests that were using it
- NOTE: There is one test under `render3/integration_spec.ts` that is commented out, and needs to be reviewed. Basically, I could not find a good why to test what it was doing, because it was doing things that I am not sure we could generate in an acceptance test.
PR Close#30645
Added a new syntax for projections (`¤` will represent `ng-content` nodes) so that we can treat them specifically.
When we enter an i18n block with the instruction `i18nStart`, a new `delayProjection` variable is set to true to prevent the instruction `projection` from projecting the nodes. Once we reach the `i18nEnd` instruction and encounter a projection in the translation we will project its nodes.
If a projection was removed from a translation, then its nodes won't be projected at all.
The variable `delayProjection` is restored to `false` at the end of `i18nEnd` so that it doesn't stop projections outside of i18n blocks.
FW-1261 #resolve
PR Close#30782
We used to ignore empty strings for optimization purposes, but it turns out that empty strings are also valid values for ICU cases and we shouldn't ignore those.
FW-1290 #resolve
PR Close#30846
Some HTML attributes don't correspond to their DOM property name, in which
case the runtime will apply the appropriate transformation when assigning
a property using its attribute name. One example of this is the `for`
attribute, for which the DOM property is named `htmlFor`.
The type-checking machinery in ngtsc must also take this mapping into
account, as it generates type-check code in which unclaimed property bindings
are assigned to properties of (subtypes of) `HTMLElement`.
Fixes#30607
Fixes FW-1327
PR Close#30675
Before this change we would systematically call LQueries.clone() when executting
elementStart / elementContainerStart instructions. This was often unnecessary as
LQueries can be mutated under 2 conditions only:
- we are crossing an element that has directives with content queries
(new queries must be added);
- we are descending into element hierarchy (creating a child element of an existing element)
and the current LQueries object is tracking shallow queries (shallow queries are removed).
With this PR LQueires.clone() is only done when needed and this gratelly reduces number
of LQueries object created: in the "expanding rows" benchmark number of allocated
(and often GCed just after!) LQueries is reduced from ~100k -> ~35k. This represents
over 1MB of memory that is not allocated.
PR Close#30664
Projecting bare ICU expressions failed because we would assume that component's content nodes would be projected later and doing so at that point would be wasteful. But ICU nodes are handled independently and should be inserted immediately because they will be ignored by projections.
FW-1348 #resolve
PR Close#30696
Inside of `DebugNode.queryAll` we walk through all of the descendants of the node that we're querying against, however the logic that walks sideways through a node siblings also applies to the root node. This means that eventually we'll match against its nodes as well which will give us invalid results. These changes add an extra check to ensure that we aren't matching against the siblings of the root node.
This PR resolves FW-1353.
PR Close#30788
With View engine it was possible to declare multiple projection
definitions and to programmatically project nodes into the slots.
e.g.
```html
<ng-content></ng-content>
<ng-content></ng-content>
```
Using `ViewContainerRef#createComponent` allowed projecting
nodes into one of the projection defs (through index)
This no longer works with Ivy as the `projectionDef` instruction only
retrieves a list of selectors instead of also retrieving entries for
reserved projection slots which appear when using the default
selector multiple times (as seen above).
In order to fix this issue, the Ivy compiler now passes all
projection slots to the `projectionDef` instruction. Meaning that
there can be multiple projection slots with the same wildcard
selector. This allows multi-slot projection as seen in the
example above, and it also allows us to match the multi-slot node
projection order from View Engine (to avoid breaking changes).
It basically ensures that Ivy fully matches the View Engine behavior
except of a very small edge case that has already been discussed
in FW-886 (with the conclusion of working as intended).
Read more here: https://hackmd.io/s/Sy2kQlgTE
PR Close#30561
Currently with Ivy, `ModuleWithProvider` providers are processed in order
of declaration in the `NgModule` imports. This technically makes makes
sense but is a potential breaking change as `ModuleWithProvider` providers
are processed after all imported modules in View Engine.
In order to keep the behavior of View Engine, the `r3_injector` is updated
to no longer process `ModuleWithProvider` providers egarly.
Resolves FW-1349
PR Close#30688
The "async" around a couple tests was removed because of the merge conflict in one of the previous commits (80394ce08b). This change restores missing "async"s.
PR Close#30762
fix(@schematics/angular): TypeScript related migrations should cater for BOM
In the CLI `UpdateRecorder` methods such as `insertLeft`, `remove` etc.. accepts positions which are not offset by a BOM. This is because when a file has a BOM a different recorder will be used https://github.com/angular/angular-cli/blob/master/packages/angular_devkit/schematics/src/tree/recorder.ts#L72 which caters for an addition offset/delta.
The main reason for this is that when a developer is writing a schematic they shouldn't need to compute the offset based if a file has a BOM or not and is handled out of the box.
Example
```ts
recorder.insertLeft(5, 'true');
```
However this is unfortunate in the case if a ts SourceFile is used and one uses `getWidth` and `getStart` method they will already be offset by 1, which at the end it results in a double offset and hence the problem.
Fixes#30713
PR Close#30719
Two issues with DebugNode/DebugElement in Ivy were causing problems in user
tests.
1. The DebugNodes returned by Ivy were not actually instances of DebugNode.
This was due to an issue with the Ivy switch logic in debug_node.ts.
The declaration of the exported DebugNode reference was set to
`DebugNode__PRE_R3__ as any`. The cast prevented the Ivy switch transform
from detecting this as a switchable declaration. The transform cannot handle
arbitrary syntax, and exports *must* be of the form "const x = y__PRE_R3__;"
or they will not work. The cast to any in this case was not needed, so this
commit removes it.
2. DebugNodes returned by Ivy multiple times for the same element were not
reference-identical. This was previously considered a minor breaking change
in Ivy, but testing has shown that users depend on referential equality of
DebugNodes. This commit caches a DebugNode on a DOM node when first creating
it, to allow returning the same instance in subsequent operations.
PR Close#30756
Due to sanitization being stored as a temp/global value between
instructions, unit tests randomly failed because one test failed
to clean up its temporary value. This patch fixes this issue.
`i18nAttributes` instructions always occur after the element instruction. This means that we need to treat `i18n-` attributes differently.
By defining a specific `AttributeMarker` we can ensure that we won't trigger directive inputs with untranslated attribute values.
FW-1332 #resolve
PR Close#30402
Changed runtime i18n to define attributes with bindings, or matching directive inputs/outputs as element properties as we are supposed to do in Angular.
This PR fixes the issue where directive inputs wouldn't be trigged.
FW-1315 #resolve
PR Close#30402
Commit 0df719a46 introduced registration of NgModules with ids when compiled
with AOT, and f74373f2d corrected the timing to avoid issues with tree
shaking. Neither of these approaches were correct.
This commit fixes the timing to match View Engine and avoid tree shaking
issues, as well as fixes a bug with the registration of imported module ids.
A new Ivy-only test is added which verifies that modules get registered
correctly under real-world conditions.
PR Close#30706
Plural ICU expressions depend on the locale (different languages have different plural forms). Until now the locale was hard coded as `en-US`.
For compatibility reasons, if you use ivy with AOT and bootstrap your app with `bootstrapModule` then the `LOCALE_ID` token will be set automatically for ivy, which is then used to get the correct plural form.
If you use JIT, you need to define the `LOCALE_ID` provider on the module that you bootstrap.
For `TestBed` you can use either `configureTestingModule` or `overrideProvider` to define that provider.
If you don't use the compat mode and start your app with `renderComponent` you need to call `ɵsetLocaleId` manually to define the `LOCALE_ID` before bootstrap. We expect this to change once we start adding the new i18n APIs, so don't rely on this function (there's a reason why it's a private export).
PR Close#29249
This patch is one of the final patches to refactor the styling algorithm
to be more efficient, performant and less complex.
This patch enables sanitization support for map-based and prop-based
style bindings.
PR Close#30667
Instead of linking to a markdown file explaining what the migration warnings
are about, we should link to the deprecation guide which now also contains
an entry for that schematic. This makes the deprecation explanations
consistent and more centralized.
PR Close#30702
This fixes a collision between #30639 and #30543 where the latter added
usages of @ViewChild without the static flag present, but the former
made the flag required.
PR Close#30666
This commit makes the static flag on @ViewChild and @ContentChild required.
BREAKING CHANGE:
In Angular version 8, it's required that all @ViewChild and @ContentChild
queries have a 'static' flag specifying whether the query is 'static' or
'dynamic'. The compiler previously sorted queries automatically, but in
8.0 developers are required to explicitly specify which behavior is wanted.
This is a temporary requirement as part of a migration; see
https://angular.io/guide/static-query-migration for more details.
@ViewChildren and @ContentChildren queries are always dynamic, and so are
unaffected.
PR Close#30639
This patch in the second runtime change which refactors how styling
bindings work in Angular. This patch refactors how map-based
`[style]` and `[class]` bindings work using a new algorithm which
is faster and less complex than the former one.
This patch is a follow-up to an earlier refactor which enabled
support for prop-based `[style.name]` and `[class.name]`
bindings (see f03475cac8).
PR Close#30543
This is a new feature of the Ivy TestBed.
A common user pattern is to test one component with another. This is
commonly done by creating a `TestFixture` component which exercises the
main component under test.
This pattern is more difficult if the component under test is declared in an
NgModule but not exported. In this case, overriding the module is necessary.
In g3 (and View Engine), it's possible to use an NgSummary to override the
recompilation of a component, and depend on its AOT compiled factory. The
way this is implemented, however, specifying a summary for a module
effectively overrides much of the TestBed's other behavior. For example, the
following is legal:
```typescript
TestBed.configureTestingModule({
declarations: [FooCmp, TestFixture],
imports: [FooModule],
aotSummaries: [FooModuleNgSummary],
});
```
Here, `FooCmp` is declared in both the testing module and in the imported
`FooModule`. However, because the summary is provided, `FooCmp` is not
compiled within the context of the testing module, but _is_ made available
for `TestFixture` to use, even if it wasn't originally exported from
`FooModule`.
This pattern breaks in Ivy - because summaries are a no-op, this amounts
to a true double declaration of `FooCmp` which raises an error.
Fixing this in user code is possible, but is difficult to do in an
automated or backwards compatible way. An alternative solution is
implemented in this PR.
This PR attempts to capture the user intent of the following previously
unsupported configuration:
```typescript
TestBed.configureTestingModule({
declarations: [FooCmp, TestFixture],
imports: [FooModule],
});
```
Note that this is the same as the configuration above in Ivy, as the
`aotSummaries` value provided has no effect.
The user intent here is interpreted as follows:
1) `FooCmp` is a pre-existing component that's being used in the test
(via import of `FooModule`). It may or may not be exported by this
module.
2) `FooCmp` should be part of the testing module's scope. That is, it
should be visible to `TestFixture`. This is because it's listed in
`declarations`.
This feature effectively makes the `TestBed` testing module special. It's
able to declare components without compiling them, if they're already
compiled (or configured to be compiled) in the imports. And crucially, the
behavior matches the first example with the summary, making Ivy backwards
compatible with View Engine for tests that use summaries.
PR Close#30578
Depending on which placeholders the translation uses, there are some legitimate cases where we might not use all placeholder replacements in `i18nPostprocess`. For example if some of the placeholders of the original messages have been removed in the translation.
FW-1312 #resolve
PR Close#30632
8479cb4233 updated the static-query migration
to refer to the new guide on AIO. Unfortunately these URLs are currently not
valid as the guide is only available on `next.angular.io` right now. In order to
make the link work permanently (e.g. if we eventually remove the guide in future
major versions), we use the permalink from the `v8` subdomain.
PR Close#30649
DEPRECATION:
Angular previously has supported an integration with the Web Tracing
Framework (WTF) for performance testing of Angular applications. This
integration has not been maintained and likely does not work for the
majority of Angular applications today. As a result, we are deprecating
the integration in Angular version 8.
This deprecation covers the following public APIs:
* `WtfScopeFn`
* `wtfCreateScope`
* `wtfStartTimeRange`
* `wtfEndTimeRange`
* `wtfLeave`
FW-1338 #resolve
PR Close#30642
Currently if a project has source-files with syntax failures and the migration
has been started on a broken project, we silently migrate *as much as possible*,
but never notify the developer that the project was in a broken state and that
he can re-run the migration after the failures are fixed.
Additionally the template strategy does not need to exit gracefully if it detects
Angular semantic diagnostics on generated files (template type checking). These
diagnostics are not relevant for the query timing analysis.
PR Close#30628
We are removing the prompt for the `static-query` migration and make the
template strategy the migration strategy for the migration. The usage
strategy is good for best-practices, but for now we want to ensure that
the migration is a seamless as possible and that is only achievable my
re-using the same logic that View Engine uses for determining the
timing of a query.
PR Close#30628
Alan
Pawel Kozlowski
Andrew Kushnir
crisbeto
Olivier Combe
George Kalpakas
Alex Rickabaugh
Misko Hevery
Umair Sarfraz
Santosh Yadav
Greg Magolan
Judy Bogart
Paul Gschwendtner
Alex Eagle
Ben Lesh
cexbrayat
JoostK
Umberto Tarantino
Vikram Subramanian
Tim Deschryver
Matias Niemelä
Kara Erickson