Commit Graph

2326 Commits

Author SHA1 Message Date
Misko Hevery 2ca7984d55 fix(core): remove support for `Map`/`Set` in `[class]`/`[style]` bindings (#35392)
Close FW-1863

PR Close #35392
2020-02-14 11:14:43 -08:00
Misko Hevery b9b512f729 fix(ivy): `LFrame` needs to release memory on `leaveView()` (#35156)
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
2020-02-14 11:13:36 -08:00
crisbeto d6bc63fa38 fix(ivy): error if directive with synthetic property binding is on same node as directive that injects ViewContainerRef (#35343)
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
2020-02-12 17:14:25 -08:00
Andrew Kushnir ae0253f34a fix(ivy): set namespace for host elements of dynamically created components (#35136)
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
2020-02-07 17:22:53 -08:00
Stephen Fluin b5fa4b58ca docs: improve description of providedIn any (#35192)
PR Close #35192
2020-02-07 09:59:09 -08:00
Andrew Scott 0cbdd54fd0 fix(ivy): ensure module imports are instantiated before the module being declared (#35172)
PR Close #35172
2020-02-07 09:58:49 -08:00
Miško Hevery d528dedd50 refactor(ivy): Explicitly pass in `TView` (#35069)
- 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
2020-02-04 12:56:47 -08:00
Miško Hevery 770980de69 test(ivy): fix broken tests (#35069)
PR Close #35069
2020-02-04 12:56:47 -08:00
Sonu Kapoor 7fb66eca25 docs: changes AoT to AOT for consistency (#35112)
PR Close #35112
2020-02-04 10:43:33 -08:00
Miško Hevery ab931cf872 fix(ivy): host-styling throws assert exception inside *ngFor (#35133)
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
2020-02-03 14:03:40 -08:00
Misko Hevery 01308e4c7c refactor(ivy): Remove `TNode.directives` in favor of `TData` (#35050)
`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
2020-02-03 08:56:51 -08:00
Misko Hevery 65dbd50594 test(ivy): correct var count in perf benchmarks. (#35071)
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
2020-02-03 08:48:41 -08:00
Kristiyan Kostadinov cd9ae66b35 refactor(ivy): remove markDirty from global utilities (#34453)
Removes `markDirty` from the global debugging utilities API since we've already exposed `detectChanges`.

PR Close #34453
2020-01-30 11:30:32 -08:00
crisbeto 471375adbe docs(ivy): document global debugging utilities and clean up API (#34453)
Cleans up the public API of the global debugging utilities, documents them and exposes them in the API docs.

PR Close #34453
2020-01-30 11:30:32 -08:00
Miško Hevery c1cf46c5c3 refactor(ivy): clean of #34804 from previous merge (#35022)
This is a follow up for https://github.com/angular/angular/pull/34804

PR Close #35022
2020-01-29 16:35:08 -08:00
Misko Hevery ee8b8f52aa feat(ivy): Change static priority resolution to be same level as directive it belongs to (#34938)
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
2020-01-29 15:41:47 -08:00
Misko Hevery cf07d428a1 refactor(ivy): make assert conditions more readable (#34938)
PR Close #34938
2020-01-29 15:41:47 -08:00
Andrew Scott b8ffcf973c fix(ivy): update ViewContainerRef to get the correct parentInjector (#35013)
PR Close #35013
2020-01-29 12:46:04 -08:00
Andrew Scott 4a5467bac6 fix(ivy): ensure multi providers in ModuleWithProviders are not duplicated (#34914)
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
2020-01-29 12:22:58 -08:00
George Kalpakas 669df70da5 fix(ivy): ensure `DebugNode`/`DebugElement` are tree-shakeable in Ivy (#35003)
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
2020-01-28 15:57:57 -08:00
George Kalpakas 0f02e66170 refactor(core): order private exports alphabetically (#35003)
PR Close #35003
2020-01-28 15:57:57 -08:00
George Kalpakas 9e351b8fe1 refactor: remove unused imports (#35003)
PR Close #35003
2020-01-28 15:57:57 -08:00
Kristiyan Kostadinov 304584c291 perf(ivy): remove unused argument in hostBindings function (#34969)
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
2020-01-27 12:49:35 -08:00
Miško Hevery 9bd9590767 refactor(ivy): change styling to use programmatic API on updates (#34804)
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
2020-01-24 12:23:19 -08:00
Misko Hevery 4c7087ccdb refactor(core): Add array modification functions. (#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
2020-01-24 12:23:00 -08:00
Miško Hevery 5aabe93abe refactor(ivy): Switch styling to new reconcile algorithm (#34616)
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
2020-01-24 12:23:00 -08:00
Miško Hevery b4a711ea9f refactor(ivy): Delete all styling code. (Part of next SHA) (#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
2020-01-24 12:22:44 -08:00
Miško Hevery 69de7680f5 Revert: "feat(ivy): convert [ngStyle] and [ngClass] to use ivy styling bindings" (#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
2020-01-24 12:22:44 -08:00
Misko Hevery b7ff38b1ef refactor(ivy): Implement `computeStaticStyling` (#34418)
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
2020-01-24 12:22:44 -08:00
Misko Hevery 54af220107 refactor(ivy): create better styling parsing API (#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
2020-01-24 12:22:26 -08:00
Misko Hevery da7e362bce refactor(ivy): delete `ɵɵelementHostAttrs` instruction (#34717)
PR Close #34717
2020-01-24 12:22:25 -08:00
Miško Hevery 2227d471a4 refactor(ivy): delete `ɵɵallocHostVars` instruction (#34708)
Delete `ɵɵallocHostVars` instruction in favor of using `hostVars` declaration on `DrictiveDef` directly.

PR Close #34708
2020-01-24 12:22:10 -08:00
Miško Hevery 2961bf06c6 refactor(ivy): move `hostVars`/`hostAttrs` from instruction to `DirectiveDef` (#34683)
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
2020-01-24 12:22:10 -08:00
Miško Hevery 94504ff5c8 refactor(ivy): add `insertTStylingBinding` to keep track of bindings. (#34004)
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
2020-01-24 12:21:39 -08:00
Misko Hevery 76698d38f7 refactor(ivy): Add style reconciliation algorithm (#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
2020-01-24 12:21:39 -08:00
Kristiyan Kostadinov ef95da6d3b fix(ivy): don't detect changes on detached child embedded views (#34846)
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
2020-01-24 12:15:52 -08:00
cexbrayat 055533b6a7 feat(common): expose getLocaleCurrencyCode publicly (#34810)
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
2020-01-23 13:18:53 -08:00
Paul Gschwendtner 970b22f98e test: setup circular dependency tests for all entry points (#34774)
Sets up circular dependency tests for all entry-points in the
project (except for the ones part of a deprecated package).

PR Close #34774
2020-01-23 11:36:40 -08:00
Andrew Scott 32b72f39f0 fix(ivy): ensure eventListeners added outside angular context are not called... (#34514)
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
2020-01-22 15:38:13 -08:00
Kristiyan Kostadinov 00f13cc074 fix(ivy): DebugNode.classes not working on SVG elements (#34872)
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
2020-01-22 13:40:22 -08:00
Matias Niemelä 32489c7426 revert: refactor(ivy): remove styleSanitizer instruction in favor of an inline param (#34480) (#34910)
This reverts commit 84d24c08e1.

PR Close #34910
2020-01-22 15:59:33 -05:00
Matias Niemelä 84d24c08e1 refactor(ivy): remove styleSanitizer instruction in favor of an inline param (#34480)
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
2020-01-22 14:35:00 -05:00
crisbeto 697f6a55a5 fix(animations): not waiting for child animations to finish when removing parent in Ivy (#34702)
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
2020-01-21 13:13:20 -05:00
Pete Bacon Darwin e0ad9ecda0 fix(ivy): ensure that `LOCALE_ID` is set after app initializers (#34830)
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
2020-01-17 16:23:49 -05:00
Mike Casebolt f2df1c79d2 refactor(core): remove underscore suffix from locally scoped class expression names (#34757)
remove unnecessary underscore suffix and the corresponding TODO comments,
because the rollup bug was fixed: github.com/rollup/rollup/issues/2047

PR Close #34757
2020-01-16 12:10:01 -05:00
Miško Hevery d56cf6a92d refactor: improve assert error by including the failed condition (#34799)
PR Close #34799
2020-01-16 12:09:03 -05:00
Igor Minar 63c9c2d2be fix: work around 'noImplicityAny' incompatibility due to ts3.7 update (#34798)
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
2020-01-16 12:06:57 -05:00
Greg Magolan a28c02bf89 build: derive ts_library dep from jasmine_node_test boostrap label if it ends in `_es5` (#34736)
PR Close #34736
2020-01-15 14:58:07 -05:00
Greg Magolan 81f6da173b build: add a before-all-other bootstrap script that patches require (#34736)
This removes the churn in the existing bootstrap scripts.

PR Close #34736
2020-01-15 14:58:07 -05:00
Greg Magolan aee67f08d9 test: handle bootstrap templated_args in jasmine_node_test defaults.bzl (#34736)
PR Close #34736
2020-01-15 14:58:07 -05:00