Commit Graph

2369 Commits

Author SHA1 Message Date
Paul Gschwendtner 59607dc495 fix(core): undecorated-classes-with-di migration should handle libraries generated with CLI versions past v6.2.0 (#35824)
The options for `flatModuleId` and `flatModuleOutFile` had been removed in the CLI
from generated libraries with 718ee15b9a.

This has been done because `ng-packagr` (which is used to build the
libraries) automatically set these options in-memory when it compiles the library.
No migration has been created for this because there was no actual need to get rid of
this. Keeping the options in the library `tsconfig` does not cause any problems unless
the `tsconfig` is used outside of `ng-packagr`. This was not anticipated, but is now
commonly done in `ng update` migrations.

The `ng update` migrations try to create an instance of the `AngularCompilerProgram` by
simply parsing the `tsconfig`. The migrations make the valid assumption that `tsconfig` files
are not incomplete/invalid. They _definitely_ are in the file system though. It just works for
libraries because `ng-packagr` in-memory completes the invalid `tsconfig` files, so that they
can be passed to the `@angular/compiler-cli`.

We can't have this logic in the `ng update` migrations because it's
out-of-scope for individual migrations to distinguish between libraries
and applications. Also it would be out-of-scope to parse the
`ng-packagr` configuration and handle the tsconfig in-memory completion.

As a workaround though, we can remove the flat-module bundle options
in-memory when creating the compiler program. This is acceptable since
we don't emit the program and the flat module bundles are not needed.

Fixes #34985.

PR Close #35824
2020-03-06 12:40:18 -05:00
Andrew Scott d543b13e5c Revert "fix(core): log error instead of warning for unknown properties and elements (#35798)" (#35845)
This reverts commit 00f3c58bb9.
Rolling back because it could be breaking e2e tests that assert that
there are no errors in the console after the assertions have run. We can
re-add this in v10.

PR Close #35845
2020-03-03 16:02:58 -08:00
crisbeto 00f3c58bb9 fix(core): log error instead of warning for unknown properties and elements (#35798)
Changes the Ivy unknown element/property messages from being logged with `console.warn` to `console.error`. This should make them a bit more visible without breaking existing apps. Furthermore, a lot of folks filter out warning messages in the dev tools' console, whereas errors are usually still shown.

Fixes #35699.

PR Close #35798
2020-03-03 13:55:22 -08:00
crisbeto 1f8a243b67 fix(core): unable to NgModuleRef.injector in module constructor (#35731)
This is a follow up to #35637 which resolved a similar issue for `ComponentFactoryResolver`, but not the root cause. When a `NgModuleRef` is created, it instantiates an `Injector` internally which in turn resolves all of injector types. This can result in a circular call that results in an error, because the module is one of the injector types being resolved.

These changes work around the issue by allowing the constructor to run before resolving the injector types.

Fixes #35677.
Fixes #35639.

PR Close #35731
2020-03-03 13:51:51 -08:00
Andrew Scott c7d0567d37 Revert "docs: add comment markers for internal goog.define replacement (#32795)" (#35830)
This reverts commit 32b6c2285e as this is
no longer used.

PR Close #35830
2020-03-03 09:34:17 -08:00
Paul Gschwendtner 5615928df9 build: no longer run tslint from within gulp task (#35800)
Switches our tslint setup to the standard `tslint.json` linter excludes.
The set of files that need to be linted is specified through a Yarn script.

For IDEs, open files are linted with the closest tslint configuration, if the
tslint IDE extension is set up, and the source file is not excluded.

We cannot use the language service plugin for tslint as we have multiple nested
tsconfig files, and we don't want to add the plugin to each tsconfig. We
could reduce that bloat by just extending from a top-level tsconfig that
defines the language service plugin, but unfortunately the tslint plugin does
not allow the use of tslint configs which are not part of the tsconfig project.

This is problematic since the tslint configuration is at the project root, and we
don't want to copy tslint configurations next to each tsconfig file.

Additionally, linting of `d.ts` files has been re-enabled. This has been
disabled in the past and a TODO has been left. This commit fixes the
lint issues and re-enables linting.

PR Close #35800
2020-03-03 09:20:49 -08:00
Paul Gschwendtner 5349e46b46 build: update tslint to latest version (#35800)
The old version we currently use does not properly implement the
option to exclude files from within the `tslint.json` file.

PR Close #35800
2020-03-03 09:20:48 -08:00
crisbeto 0653db14e7 perf(core): add micro benchmark for destroy hook invocation (#35784)
Adds a micro benchmark which excercises the logic that invokes `ngOnDestroy` hooks.

PR Close #35784
2020-03-03 08:57:58 -08:00
Doug Parker c195d22f68 fix(core): remove side effects from `ɵɵgetInheritedFactory()` (#35769)
`ɵɵgetInheritedFactory()` is called from generated code for a component which extends another class. This function is detected by Closure to have a side effect and is not able to tree shake the component as a result. Marking it with `noSideEffects()` tells Closure it can remove this function under the relevant tree shaking conditions.

PR Close #35769
2020-03-03 08:50:03 -08:00
Doug Parker 9cf85d2177 fix(core): remove side effects from `ɵɵNgOnChangesFeature()` (#35769)
`ɵɵNgOnChangesFeature()` would set `ngInherit`, which is a side effect and also not necessary. This was pulled out to module scope so the function itself can be pure. Since it only curries another function, the call is entirely unnecessary. Updated the compiler to only generate a reference to this function, rather than a call to it, and removed the extra curry indirection.

PR Close #35769
2020-03-03 08:50:03 -08:00
Doug Parker ba3612774f fix(core): add `noSideEffects()` to `ɵɵdefineComponent()` (#35769)
This marks the function are "pure" and eligible to be tree shaken by Closure. Without this, initializing `ngDevMode` is considered a side effect which prevents this function from being tree shaken and also any component which calls it.

PR Close #35769
2020-03-03 08:50:03 -08:00
Doug Parker dc6a7918e3 fix(core): add `noSideEffects()` to `make*Decorator()` functions (#35769)
This causes all the `make*Decorator()` functions to be considered pure and to be eligible for associated tree shaking by Closure.

PR Close #35769
2020-03-03 08:50:03 -08:00
Doug Parker 4052dd8188 refactor(core): update `noSideEffects()` to return the result of the inner function (#35769)
This is useful for propagating return values without them being converted to a string. It still provides the same guarantees to Closure, which will assume that the function invoked is pure and can be tree-shaken accordingly.

PR Close #35769
2020-03-03 08:50:03 -08:00
Pawel Kozlowski 5652fb1018 fix(core): allow null / undefined values in query results (#35796)
Before this change ngIvy implementation of queries would throw upon
encountering null / undefined query result collected from an embedded
view. It turns out that we might have a provider that explicitly provides
a null / undefined value in a place of a token queried for.

This commit removes a check from the ngIvy query implementation that was
asserting on a query result to be defined.

Fixes #35673

PR Close #35796
2020-03-02 11:16:22 -08:00
Miško Hevery a153b61098 fix(core): treat `[class]` and `[className]` as unrelated bindings (#35668)
Before this change `[class]` and `[className]` were both converted into `ɵɵclassMap`. The implication of this is that at runtime we could not differentiate between the two and as a result we treated `@Input('class')` and `@Input('className)` as equivalent.

This change makes `[class]` and `[className]` distinct. The implication of this is that `[class]` becomes `ɵɵclassMap` instruction but  `[className]` becomes `ɵɵproperty' instruction. This means that `[className]` will no longer participate in styling and will overwrite the DOM `class` value.

Fix #35577

PR Close #35668
2020-03-02 08:18:59 -08:00
Miško Hevery a4e956a7cf style: Reformat test comment for line breaks (#35668)
PR Close #35668
2020-03-02 08:18:59 -08:00
Andrew Kushnir 40da51f641 fix(compiler): support i18n attributes on `<ng-template>` tags (#35681)
Prior to this commit, i18n attributes defined on `<ng-template>` tags were not processed by the compiler. This commit adds the necessary logic to handle i18n attributes in the same way how these attrs are processed for regular elements.

PR Close #35681
2020-03-02 08:18:06 -08:00
Andrew Kushnir 5bc39f8c8d perf(core): use multiple directives in host bindings micro benchmark (#35736)
This commit updates the host bindings micro benchmark to run tests with mutliple directives (where each directive contains host bindings). The number of directives is configurable as a constant in the micro benchmark file. This change is needed to have an ability to measure/compare perf in different scenarios.

PR Close #35736
2020-02-28 12:26:43 -08:00
Andrew Kushnir ef75875ca3 fix(core): handle `<ng-template>` with local refs in i18n blocks (#35758)
This commit extends the range of tNode types that may have local refs to include `TNodeType.Container` to account for `<ng-template>`s. Original changes in https://github.com/angular/angular/pull/33415 didn't include that type and as a result, an error is thrown at runtime in case an i18n block contains an `<ng-template>` with local refs.

PR Close #35758
2020-02-28 12:25:11 -08:00
Misko Hevery 40035380e7 fix(core): Remove `debugger` statement when assert is thrown (#35763)
Fix #35470
Fix FW-1925

PR Close #35763
2020-02-28 12:23:50 -08:00
Miško Hevery 8f38eb7acb fix(core): Remove `debugger` statement (#35763)
Fix #35470

PR Close #35763
2020-02-28 12:23:50 -08:00
Andrew Kushnir 7b13977c3a fix(core): use proper configuration to compile Injectable in JIT (#35706)
Prior to this change, the logic that compiles Injectable in JIT mode used incorrect configuration that triggers a problem when `ChangeDetectorRef` is used as a dependency. This commit updates the logic to generate correct inject instruction to add the `ChangeDetectorRef` dependency in case it's requested in @Injectable class.

PR Close #35706
2020-02-27 14:01:15 -08:00
Andrew Kushnir 8fed1fe792 perf(core): adding micro benchmark for host bindings (#35705)
This commit adds micro benchmark for host bindings, so that we can assess the impact of changes related to host bindings (for example PR #35568).

PR Close #35705
2020-02-27 13:59:56 -08:00
crisbeto 0bc35a71e2 fix(ivy): injecting incorrect provider when re-providing injectable with useClass (#34574)
If an injectable has a `useClass`, Ivy injects the token in `useClass`, rather than the original injectable, if the injectable is re-provided under a different token. The correct behavior is that it should inject the re-provided token, no matter whether it has `useClass`.

Fixes #34110.

PR Close #34574
2020-02-26 13:00:21 -08:00
Misko Hevery 3af103aa61 fix(core): support sanitizer value in the [style] bindings (#35564)
When binding to `[style]` we correctly sanitized/unwrapped properties but we did not do it for the object itself.

```
@HostBinding("style")
style: SafeStyle = this.sanitizer.bypassSecurityTrustStyle(
    "background: red; color: white; display: block;"
  );
```

Above code would fail since the `[style]` would not unwrap the `SafeValue` and would treat it as object resulting in incorrect behavior.

Fix #35476 (FW-1875)

PR Close #35564
2020-02-26 12:56:09 -08:00
KostyaTretyak 2a8dd4758c test(core): check dependency in extended child (#34767)
Related to https://github.com/angular/angular/issues/31337

PR Close #34767
2020-02-26 12:50:52 -08:00
Zac a7ed74f573 docs(core): correct insertBefore's description (#34547)
Update the description of `refChild`
Based on https://developer.mozilla.org/en-US/docs/Web/API/Node/insertBefore
PR Close #34547
2020-02-26 12:46:10 -08:00
Zac 8fbb966ca7 docs(core): describe your change... (#34513)
According to https://angular.io/guide/lifecycle-hooks#ondestroy
"Put cleanup logic in ngOnDestroy(), the logic that must run before Angular destroys the directive."
PR Close #34513
2020-02-26 12:45:41 -08:00
crisbeto 835618c678 fix(ivy): error when accessing NgModuleRef.componentFactoryResolver in constructor (#35637)
Currently we resolve the `NgModuleRef.componentFactoryResolver` by going through the injector, but the problem is that `ComponentFactoryResolver` has a dependency on `NgModuleRef`, which means that if the module that's attached to the ref tries to inject  `ComponentFactoryResolver` in its constructor, we'll create a circular dependency which throws at runtime.

These changes resolve the issue by creating the `ComponentFactoryResolver` manually ahead of time without going through the injector. We can do this safely, because the only dependency for the resolver is the current module ref which is providing it.

Aside from fixing the issue, another advantage to this approach is that it should reduce the amount of generated JS, because it removes a getter and a provider definitio.

Fixes #35580.

PR Close #35637
2020-02-25 13:19:13 -08:00
Felix Becker 1e20b2ca36 build(packaging): add repository.directory field to package.jsons (#27544)
PR Close #27544
2020-02-25 13:12:45 -08:00
Andrew Kushnir 0a1a989fa9 perf(core): avoid recursive scope recalculation when TestBed.overrideModule is used (#35454)
Currently if TestBed detects that TestBed.overrideModule was used for module X, transitive scopes are recalculated recursively for all modules that X imports and previously calculated data (stored in cache) is ignored. This behavior was introduced in https://github.com/angular/angular/pull/33787 to fix stale transitive scopes issue (cache was not updated if module overrides are present).

The perf issue comes from a "diamond" problem, where module X is overridden which imports modules A and B, which both import module C. Under previous logic, module C gets its transitive deps recomputed multiple times, during the recompute for both A and B. For deep graphs and big common/shared modules this can be super costly.

This commit updates the logic to recalculate ransitive scopes for the overridden module, while keeping previously calculated scopes of other modules untouched.

PR Close #35454
2020-02-25 13:11:42 -08:00
Greg Magolan 3887be464b test: disable broken saucelabs tests with “fixme-saucelabs-ivy” & “fixme-saucelabs-ve” tags (#35516)
PR Close #35516
2020-02-24 17:27:21 -08:00
Greg Magolan dde68ff954 build: add npm_integration_test && angular_integration_test (#33927)
* it's tricky to get out of the runfiles tree with `bazel test` as `BUILD_WORKSPACE_DIRECTORY` is not set but I employed a trick to read the `DO_NOT_BUILD_HERE` file that is one level up from `execroot` and that contains the workspace directory. This is experimental and if `bazel test //:test.debug` fails than `bazel run` is still guaranteed to work as  `BUILD_WORKSPACE_DIRECTORY` will be set in that context

* test //integration:bazel_test and //integration:bazel-schematics_test exclusively

* run "exclusive" and "manual" bazel-in-bazel integration tests in their own CI job as they take 8m+ to execute

```
//integration:bazel-schematics_test                                      PASSED in 317.2s
//integration:bazel_test                                                 PASSED in 167.8s
```

* Skip all integration tests that are now handled by angular_integration_test except the tests that are tracked for payload size; these are:
- cli-hello-world*
- hello_world__closure

* add & pin @babel deps as newer versions of babel break //packages/localize/src/tools/test:test

@babel/core dep had to be pinned to 7.6.4 or else //packages/localize/src/tools/test:test failed. Also //packages/localize uses @babel/generator, @babel/template, @babel/traverse & @babel/types so these deps were added to package.json as they were not being hoisted anymore from @babel/core transitive.

NB: integration/hello_world__systemjs_umd test must run with systemjs 0.20.0
NB: systemjs must be at 0.18.10 for legacy saucelabs job to pass
NB: With Bazel 2.0, the glob for the files to test `"integration/bazel/**"` is empty if integation/bazel is in .bazelignore. This glob worked under these conditions with 1.1.0. I did not bother testing with 1.2.x as not having integration/bazel in .bazelignore is correct.

PR Close #33927
2020-02-24 08:59:18 -08:00
crisbeto e17bde99f8 fix(ivy): error in AOT when pipe inherits constructor from injectable that uses DI (#35468)
When a pipe inherits its constructor, and as a result its factory, from an injectable in AOT mode, it can end up throwing an error, because the inject implementation hasn't been set yet. These changes ensure that the implementation is set before the pipe's factory is invoked.

Note that this isn't a problem in JIT mode, because the factory inheritance works slightly differently, hence why this test isn't going through `TestBed`.

Fixes #35277.

PR Close #35468
2020-02-21 12:37:08 -08:00
crisbeto 22786c8e88 fix(ivy): incorrectly generating shared pure function between null and object literal (#35481)
In #33705 we made it so that we generate pure functions for object/array literals in order to avoid having them be shared across elements/views. The problem this introduced is that further down the line the `ContantPool` uses the generated literal in order to figure out whether to share an existing factory or to create a new one. `ConstantPool` determines whether to share a factory by creating a key from the AST node and using it to look it up in the factory cache, however the key generation function didn't handle function invocations and replaced them with `null`. This means that the key for `{foo: pureFunction0(...)}` and `{foo: null}` are the same.

These changes rework the logic so that instead of generating a `null` key
for function invocations, we generate a variable called `<unknown>` which
shouldn't be able to collide with anything.

Fixes #35298.

PR Close #35481
2020-02-20 15:23:58 -08:00
JiaLiPassion 17b862cf82 feat: add an tickOptions parameter with property processNewMacroTasksSynchronously. (#33838)
This option will control whether to invoke the new macro tasks when ticking.

Close #33799

PR Close #33838
2020-02-20 15:14:59 -08:00
Miško Hevery 2562a3b1b0 fix(ivy): Add `style="{{exp}}"` based interpolation (#34202)
Fixes #33575

Add support for interpolation in styles as shown:
```
<div style="color: {{exp1}}; width: {{exp2}};">
```

PR Close #34202
2020-02-20 15:13:10 -08:00
zhuyujie a756161dc2 fix(core): make subclass inherit developer-defined data (#35105)
PR Close #35105
2020-02-19 12:50:48 -08:00
Andrew Kushnir f95b8ce07e fix(ivy): add attributes and classes to host elements based on selector (#34481)
In View Engine, host element of dynamically created component received attributes and classes extracted from component's selector. For example, if component selector is `[attr] .class`, the `attr` attribute and `.class` class will be add to host element. This commit adds similar logic to Ivy, to make sure this behavior is aligned with View Engine.

PR Close #34481
2020-02-18 17:18:13 -08:00
Pawel Kozlowski 3f4e02b8c7 fix(ivy): queries should match elements inside ng-container with the descendants: false option (#35384)
Before this change content queries with the `descendants: false` option, as implemented in ivy,
would not descendinto `<ng-container>` elements. This behaviour was different from the way the
View Engine worked. This change alligns ngIvy and VE behaviours when it comes to queries and the
`<ng-container>` elements and fixes a common bugs where a query target was placed inside the
`<ng-container>` element with a * directive on it.

Before:

```html
<needs-target>
  <ng-container *ngIf="condition">
    <div #target>...</div>  <!-- this node would NOT match -->
  </ng-container>
</needs-target>
```

After:

```html
<needs-target>
  <ng-container *ngIf="condition">
    <div #target>...</div>  <!-- this node WILL match -->
  </ng-container>
</needs-target>
```

Fixes #34768

PR Close #35384
2020-02-18 17:17:46 -08:00
crisbeto 5fbfe6996a fix(ivy): wrong context passed to ngOnDestroy when resolved multiple times (#35249)
When the same provider is resolved multiple times on the same node, the first invocation had the correct context, but all subsequent ones were incorrect because we were registering the hook multiple times under different indexes in `destroyHooks`.

Fixes #35167.

PR Close #35249
2020-02-18 17:17:05 -08:00
Andrew Kushnir c013dd4ca6 fix(core): better handing of ICUs outside of i18n blocks (#35347)
Currently the logic that handles ICUs located outside of i18n blocks may throw exceptions at runtime. The problem is caused by the fact that we store incorrect TNode index for previous TNode (index that includes HEADER_OFFSET) and do not store a flag whether this TNode is a parent or a sibling node. As a result, the logic that assembles the final output uses incorrect TNodes and in some cases throws exceptions (when incompatible structure is extracted from tView.data due to the incorrect index). This commit adjusts the index and captures whether TNode is a parent to make sure underlying logic manipulates correct TNode.

PR Close #35347
2020-02-18 12:43:44 -08:00
Misko Hevery 8c75f2155d fix(core): correctly concatenate static and dynamic binding to `class` when shadowed (#35350)
Given:
```
<div class="s1" [class]="null" [ngClass]="exp">
```
Notice that `[class]` binding is not a `string`. As a result the existing logic would not concatenate `[class]` with `class="s1"`. The resulting falsy value would than be sent to `ngClass` which would promptly clear all styles on the `<div>`

The new logic correctly handles falsy values for `[class]` bindings.

Fix #35335

PR Close #35350
2020-02-14 15:33:14 -08:00
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