Currently we always generate the `read` parameter for the view and content query instructions, however since most of the time the `read` parameter won't be set, we'll end up generating `null` which adds 5 bytes for each query when minified. These changes make it so that the `read` parameter only gets generated if it has a value.
PR Close#31667
This commit fixes a bug where we assumed all dynamically created
components would have tag-name selectors, so we passed through the
"tag name" to the renderer as the first index of the selector. For
components with attribute selectors, the tag name would be "", so
the renderer would try to create an element with tag name "" and
throw. Now we default to a "div" element the same way that View
Engine did.
Closes#31785
PR Close#31812
The way the `ComponentFactory.create` is set up at the moment is that if a `rootSelectorOrNode` is passed in, the root context will be injected instead of creating dedicated one for the component. As far as I can tell, there doesn't seem to be a reason to do this and nothing seems to break because of it. These changes switch to always create the root context.
PR Close#31661
In ViewEngine nodes that were inserted through `Renderer2` would also be picked up by `DebugNode.query` and `DebugNode.queryAll`. This worked because everything in ViewEngine went through `Renderer2` and `DebugRenderer2` in dev mode which was able to keep track of the child nodes as they're being inserted. This no longer works in Ivy, because we don't use `DebugRenderer2` and debug nodes work a little differently. These changes work around the issue by walking the DOM as the logical tree is being walked and looking for matches. Note that this is __not__ optimal, because we're walking similar trees multiple times. ViewEngine could do it more efficiently, because all the insertions go through Renderer2, however that's not the case in Ivy. This approach is being used because:
1. Matching the ViewEngine behavior would mean potentially introducing a depedency from `Renderer2` to Ivy which could bring Ivy code into ViewEngine.
2. We would have to make `Renderer3` "know" about debug nodes.
3. It allows us to capture nodes that were inserted directly via the DOM.
PR Close#31716
When injecting a `ChangeDetectorRef` into a pipe, the expected result is that the ref will be tied to the component in which the pipe is being used. This works for most cases, however when a pipe is used inside a property binding of a component (see test case as an example), the current `TNode` is pointing to component's host so we end up injecting the inner component's view. These changes fix the issue by only looking up the component view of the `TNode` if the `TNode` is a parent.
This PR resolves FW-1419.
PR Close#31438
In the previous patch () all the existing styling code was turned
off in favor of using the new refactored ivy styling code. This
patch is a follow up patch to that and removes all old, unused
styling code from the render3 directory.
PR Close#31193
This commit fixes a bug where ICU expressions inserted into ngTemplateOutlets
that are inside ngFor blocks would throw an error. We were assuming in view
insertion code that text nodes would always exist by the time a view\`s
creation block had executed. This is not true for text nodes created dynamically
by ICUs because this happens in the update block (in `i18nApply`).
This change ensures such dynamically created nodes are skipped when encountered
too early (as they will be attached later by i18n code anyway).
PR Close#31789
We currently have a handwritten version of the Ivy directive def for NgClass so
we can switch between Ivy and View Engine behavior. This generated code needs to
be kept up-to-date with what the Ivy compiler generates.
PR 30742 recently changed `classMap` such that it now requires allocation of
host binding slots. This means that the `allocHostVars()` function must be
called in the NgClass directive def to match compiler output, but the
handwritten directive def was not updated. This caused a bug where NgClass
was inappropriately overwriting data for other directives because space was
not allocated for its values.
PR Close#31788
This commit is the final patch of the ivy styling algorithm refactor.
This patch swaps functionality from the old styling mechanism to the
new refactored code by changing the instruction code the compiler
generates and by pointing the runtime instruction code to the new
styling algorithm.
PR Close#30742
Fixes all TypeScript failures caused by enabling the `--strict`
flag for test source files. We also want to enable the strict
options for tests as the strictness enforcement improves the
overall codehealth, unveiled common issues and additionally it
allows us to enable `strict` in the `tsconfig.json` that is picked
up by IDE's.
PR Close#30993
In View Engine, developers can pass bootstrap and entry components
as nested arrays. e.g.
```ts
export const MyOtherEntryComponents = [A, B, C]
@NgModule({
entryComponents: [MyComp, MyOtherEntryComponents]
})
```
Currently using nested arrays for these properties causes
unexpected errors to be reported in Ivy since the semantic
NgModule checks aren't properly recursing into the nested
entry/bootstrap components. This issue has been unveiled by
enabling the strict function parameter checks.
PR Close#30993
As part of FW-1265, the `@angular/core` package is made compatible
with the TypeScript `--strict` flag. This already unveiled a few bugs,
so the strictness flag seems to help with increasing the overall code health.
Read more about the strict flag [here](https://www.typescriptlang.org/docs/handbook/compiler-options.html)
PR Close#30993
Fixes Ivy matching directives against attribute bindings (e.g. `[attr.some-directive]="foo"`). Works by excluding attribute bindings from the attributes array during compilation. This has the added benefit of generating less code.
**Note:** My initial approach to implementing this was to have a different marker for attribute bindings so that they can be ignored when matching directives, however as I was implementing it I realized that the attributes in that array were only used for directive matching (as far as I could tell). I decided to drop the attribute bindings completely, because it results in less generated code.
PR Close#31541
Currently we reuse the same instruction both for regular property bindings and property bindings on the `host`. The only difference between the two is that when it's on the host we shouldn't support inputs. We have an optional parameter called `nativeOnly` which is used to differentiate the two, however since `nativeOnly` is preceeded by another optional parameter (`sanitizer`), we have to generate two extra parameters for each host property bindings every time (e.g. `property('someProp', 'someValue', null, true)`).
These changes add a new instruction called `hostProperty` which avoids the need for the two parameters by removing `nativeOnly` which is always set and it allows us to omit `sanitizer` when it isn't being used.
These changes also remove the `nativeOnly` parameter from the `updateSyntheticHostBinding` instruction, because it's only generated for host elements which means that we can assume that its value will always be `true`.
PR Close#31550
Prior to this fix, the logic to set the right placeholder format for ICUs was a bit incorrect: if there was a nested ICU in one of the root ICU cases, that led to a problem where placeholders in subsequent branches used the wrong ({$placeholder}) format instead of {PLACEHOLDER} one. This commit updates the logic to make sure we properly transform all placeholders even if nested ICUs are present.
PR Close#31516
Prior to this commit, it was impossible to override providers defined via ModuleWithProviders using TestBed.overrideProvider API. The problem was caused by the fact that we were not taking into account these providers while calculating accumulated set of provider overrides. This commit updates the logic to extract providers from ModuleWithProviders and calculate the final set of overrides taking them into account.
PR Close#31415
Removes direct calls from one instruction into another, moves the shared logic into a separate function and removes the state getters from the shared function.
This PR resolves FW-1340.
PR Close#31456
Since `goog.getMsg` does not process ICUs (post-processing is required via goog.i18n.MessageFormat, https://google.github.io/closure-library/api/goog.i18n.MessageFormat.html) and placeholder format used for ICUs and regular messages inside `goog.getMsg` are different, the current implementation (that assumed the same placeholder format) needs to be updated. This commit updates placeholder format used inside ICUs from `{$placeholder}` to `{PLACEHOLDER}` to better align with Closure. ICU placeholders (that were left as is prior to this commit) are now replaced with actual values in post-processing step (inside `i18nPostprocess`).
PR Close#31459
Prior to this commit, the `` unicode symbol that represents `&ngsp` in translations was not handled correctly, i.e. was not replaced with a whitespace, thus appearing on a screen. This commit adds post-processing and replaces the mentioned symbol with a whitespace.
PR Close#31479
`i18nStart` was calling `allocExpando` even if there was 0 new variable created.
This created a new expando instruction with the value 0 which was later interpreted as the start of a new expando block instead of just skipping 0 instructions.
FW-1417 #resolve
PR Close#31451
Fixes Ivy's return value for `DebugNode.context` not being consistent for the case where there is both a structural directive and a component on a node. In `ViewEngine` the instance of the component would be returned, whereas in Ivy the context of the directive is returned.
Also adds a couple of extra test cases for how `DebugNode.context` deals with directives.
This PR resolves FW-1343.
PR Close#31442
Adds a new `elementContainer` instruction that can be used to avoid two instruction (`elementContainerStart` and `elementContainerEnd`) for `ng-container` that has text-only content. This is particularly useful when we have `ng-container` inside i18n sections.
This PR resolves FW-1105.
PR Close#31444
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
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
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
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
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
Kara Erickson
crisbeto
Paul Gschwendtner
Matias Niemelä
Pawel Kozlowski
Miško Hevery
Joey Perrott
Andrew Kushnir
Olivier Combe
Greg Magolan
Alan
Alex Rickabaugh
Alex Eagle