Commit Graph

1190 Commits

Author SHA1 Message Date
crisbeto e30f494a39 refactor(ivy): remove interpolation instructions (#31395)
Makes the `interpolateX` instructions internal-only and removes their use of global state.

This PR resolves FW-1387.

PR Close #31395
2019-07-08 09:34:13 -07:00
crisbeto 02491a6ce8 refactor(ivy): move classMap interpolation logic internally (#31211)
Adds the new `classMapInterpolate1` through `classMapInterpolate8` instructions which handle interpolations inside the `class` attribute and moves the interpolation logic internally. This allows us to remove the `interpolationX` instructions in a follow-up PR.

These changes also add an error if an interpolation is encountered inside a `style` tag (e.g. `style="width: {{value}}"`). Up until now this would actually generate valid instructions, because `styleMap` goes through the same code path as `classMap` which does support interpolation. At runtime, however, `styleMap` would set invalid styles that look like `<div style="0:w;1:i;2:d;3:t;4:h;5::;7:1;">`. In `ViewEngine` interpolations inside `style` weren't supported either, however there we'd output invalid styles like `<div style="unsafe">`, even if the content was trusted.

PR Close #31211
2019-07-02 11:07:14 -07:00
Greg Magolan 361109d80f build: update to rules_nodejs 0.32.2 (#31325)
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 #31325
2019-07-01 14:16:42 -07:00
Alan f83dfd6f5a fix(core): handle `undefined` meta in `injectArgs` (#31333)
In the recent versions of the CLI we introduced a ctor downleveler tranformer for VE JIT builds based on the one found in tsickle, to fix the TDZ issue of `forwardRef`.

However this caused a regression as the injector is not handling that a position `paramType` can be undefined. Which is bubbled down to c6b29f4c6d/packages/core/src/di/injector_compatibility.ts (L162) and will crash c6b29f4c6d/packages/core/src/di/injector_compatibility.ts (L174-L186)

Fixes https://github.com/angular/angular-cli/issues/14888

PR Close #31333
2019-07-01 10:11:08 -07:00
Pawel Kozlowski 7ca611cd12 fix(ivy): properly handle re-projection with an empty set of nodes to re-project (#31306)
PR Close #31306
2019-06-28 12:21:37 -07:00
Andrew Kushnir c12b6fa028 fix(ivy): attach host element for views created via TestBed.createComponent (#31318)
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
2019-06-28 12:20:53 -07:00
Olivier Combe 4f38419e33 fix(ivy): handle ICU expressions in `executeActionOnNode` (#31313)
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
2019-06-27 15:53:10 -07:00
Kristiyan Kostadinov f2360aab9d fix(ivy): incorrect namespace for root node created through ViewContainerRef (#31232)
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
2019-06-27 09:50:52 -07:00
Alex Rickabaugh 32c760f5e7 fix(ivy): don't mask errors by calling lifecycle hooks after a crash (#31244)
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
2019-06-26 08:01:14 -07:00
Pawel Kozlowski 4fe0e75365 refactor(ivy): rename applyXYZ to executeActionOnXYZ for clarity (#31182)
PR Close #31182
2019-06-25 17:03:52 -07:00
Misko Hevery 29a9909232 refactor(ivy): simplify `walkTNodeTree` method for readability (#31182)
PR Close #31182
2019-06-25 17:03:51 -07:00
Alex Rickabaugh 660800ca4e Revert "build: update to rules_nodejs 0.32.2 (#31019)" (#31267)
This reverts commit a38433f36b.

Reason: this causes failures in g3 with i18n extraction. See #31267.

PR Close #31267
2019-06-25 14:36:00 -07:00
Greg Magolan a38433f36b build: update to rules_nodejs 0.32.2 (#31019)
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
2019-06-25 10:21:07 -07:00
crisbeto 23c017121e perf(ivy): chain multiple attribute instructions (#31152)
Similarly to what we did in #31078, these changes implement chaining for the `attribute` instruction.

This PR resolves FW-1390.

PR Close #31152
2019-06-24 12:33:49 -07:00
crisbeto fcb03abc72 fix(ivy): injecting incorrect Renderer2 into child components (#31063)
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
2019-06-24 11:33:31 -07:00
Alex Rickabaugh bd3b0564e6 fix(ivy): sync ViewRefs between multiple ViewContainerRefs (#30985)
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
2019-06-21 10:18:06 -07:00
Olivier Combe 7ff628f8d5 refactor(ivy): make `bind` an internal-only function (#31131)
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
2019-06-20 17:20:56 -07:00
Alex Eagle db08fd2607 build: enable shard_count for some jasmine tests that have many specs (#31009)
This partitions the spects across multiple processes so they run in parallel.

PR Close #31009
2019-06-18 12:11:30 -07:00
Olivier Combe 65544ac742 fix(ivy): reprojected ICU expression nodes when creating embedded views (#30979)
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
2019-06-18 09:48:46 -07:00
Ben Lesh 16aa6ceff8 refactor(ivy): update ɵɵtextBinding to not take index (#30792)
- 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
2019-06-14 12:22:11 -07:00
crisbeto 8f5c396a7c fix(ivy): don't throw when attempting to destroy a destroyed ComponentRef (#31022)
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
2019-06-14 10:44:30 -07:00
Paul Gschwendtner 58be2ff884 fix(ivy): unable to bind to implicit receiver in embedded views (#30897)
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
2019-06-11 14:29:42 -07:00
Alex Rickabaugh 7912db3829 fix(ivy): call factory functions with correct type for derived classes (#30855)
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
2019-06-11 14:27:17 -07:00
Alex Rickabaugh 0ee09cdd7e refactor(ivy): fix type of factory functions to allow explicit types (#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
2019-06-11 14:27:17 -07:00
Alex Rickabaugh a4b4f35533 feat(ivy): require 'token' for ngInjectableDef (#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
2019-06-11 14:27:16 -07:00
Paul Gschwendtner 8c4bd61b2f test(core): add missing static flag to view_insertion test view queries (#30967)
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
2019-06-11 09:37:19 -07:00
Olivier Combe 4155ed439a fix(ivy): correctly set `TView.firstChild` with runtime i18n (#30920)
`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
2019-06-11 00:09:32 +00:00
Alex Eagle ef0b2cc74d build: convert entry_point to label (#30627)
PR Close #30627
2019-06-11 00:03:11 +00:00
Ben Lesh 17d87d4e10 test(ivy): Add acceptance tests for view insertion (#30625)
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
2019-06-10 23:53:34 +00:00
Ben Lesh c0386757b1 refactor(ivy): inherently call ɵɵselect(0) (#30830)
- 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
2019-06-07 08:48:31 -07:00
Pawel Kozlowski 11a4454ab3 perf(ivy): remove check for function type in renderStringify (#30838)
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
2019-06-06 13:47:41 -07:00
crisbeto 04587a33c5 fix(ivy): DebugNode.attributes not preserving attribute name casing (#30864)
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
2019-06-06 13:47:16 -07:00
Olivier Combe b4b7af86c2 fix(ivy): trigger directive inputs for each template creation (#30870)
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
2019-06-06 13:46:40 -07:00
Ben Lesh 3859bcc70c refactor(ivy): remove ɵɵelementAttribute instruction (#30640)
PR Close #30640
2019-06-05 21:29:38 -07:00
Olivier Combe 30efb6b8ea fix(ivy): don't project removed placeholders with runtime i18n (#30783)
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
2019-06-05 21:28:39 -07:00
Ben Lesh d1df0a94d4 refactor(ivy): remove ɵɵelementProperty instruction (#30645)
- 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
2019-06-05 09:04:43 -07:00
Olivier Combe 00cc905b98 feat(ivy): support `ng-content` in runtime i18n translations (#30782)
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
2019-06-05 09:04:13 -07:00
Olivier Combe 0d4f8c7dd9 fix(ivy): allow empty cases for ICU expressions (#30846)
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
2019-06-04 13:36:28 -07:00
Miško Hevery 55a14e4866 feat(ivy): in `ngDevMode` use named object literals and arrays for easier debugging/profiling (#30542)
PR Close #30542
2019-06-04 12:01:46 -07:00
Miško Hevery c7850fff3b refactor(ivy): rename debug.ts to lview_debug.ts (#30542)
PR Close #30542
2019-06-04 12:01:46 -07:00
Misko Hevery fcdd784667 refactor(core): cleanup code with side-effects which was preventing tree-shaking (#30580)
PR Close #30580
2019-06-03 09:01:51 -07:00
Olivier Combe 680d38513b fix(ivy): correctly project bare ICU expressions (#30696)
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
2019-06-03 08:59:14 -07:00
crisbeto f936590573 fix(ivy): DebugNode.queryAll matching sibling nodes (#30788)
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
2019-05-31 15:19:18 -07:00
Paul Gschwendtner aca339e864 fix(ivy): unable to project into multiple slots with default selector (#30561)
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
2019-05-31 09:52:32 -07:00
Paul Gschwendtner 6d861f240b fix(ivy): module with providers are processed too early (#30688)
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
2019-05-31 09:48:39 -07:00
Alex Rickabaugh d82adbe8c4 fix(ivy): align DebugNode/DebugElement behavior with View Engine (#30756)
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
2019-05-30 20:45:31 -07:00
Matias Niemelä 19bbf4bc8e test(ivy): fix sanitization state leaking issue
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.
2019-05-30 13:53:06 -07:00
Olivier Combe 91699259b2 fix(ivy): trigger directive inputs with i18n translations (#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
2019-05-30 16:39:24 -04:00
Alex Rickabaugh b4644d7bb0 fix(ivy): correct timing of NgModuleFactory registration (#30706)
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
2019-05-30 15:13:20 -04:00
Olivier Combe 5e0f982961 feat(ivy): use i18n locale data to determine the plural form of ICU expressions (#29249)
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
2019-05-30 15:09:02 -04:00