Commit Graph

322 Commits

Author SHA1 Message Date
JoostK 4aa4e6fd03 fix(compiler): handle type references to namespaced symbols correctly (#36106)
When the compiler needs to convert a type reference to a value
expression, it may encounter a type that refers to a namespaced symbol.
Such namespaces need to be handled specially as there's various forms
available. Consider a namespace named "ns":

1. One can refer to a namespace by itself: `ns`. A namespace is only
   allowed to be used in a type position if it has been merged with a
   class, but even if this is the case it may not be possible to convert
   that type into a value expression depending on the import form. More
   on this later (case a below)
2. One can refer to a type within the namespace: `ns.Foo`. An import
   needs to be generated to `ns`, from which the `Foo` property can then
   be read.
3. One can refer to a type in a nested namespace within `ns`:
   `ns.Foo.Bar` and possibly even deeper nested. The value
   representation is similar to case 2, but includes additional property
   accesses.

The exact strategy of how to deal with these cases depends on the type
of import used. There's two flavors available:

a. A namespaced import like `import * as ns from 'ns';` that creates
   a local namespace that is irrelevant to the import that needs to be
   generated (as said import would be used instead of the original
   import).

   If the local namespace "ns" itself is referred to in a type position,
   it is invalid to convert it into a value expression. Some JavaScript
   libraries publish a value as default export using `export = MyClass;`
   syntax, however it is illegal to refer to that value using "ns".
   Consequently, such usage in a type position *must* be accompanied by
   an `@Inject` decorator to provide an explicit token.

b. An explicit namespace declaration within a module, that can be
   imported using a named import like `import {ns} from 'ns';` where the
   "ns" module declares a namespace using `declare namespace ns {}`.
   In this case, it's the namespace itself that needs to be imported,
   after which any qualified references into the namespace are converted
   into property accesses.

Before this change, support for namespaces in the type-to-value
conversion was limited and only worked  correctly for a single qualified
name using a namespace import (case 2a). All other cases were either
producing incorrect code or would crash the compiler (case 1a).

Crashing the compiler is not desirable as it does not indicate where
the issue is. Moreover, the result of a type-to-value conversion is
irrelevant when an explicit injection token is provided using `@Inject`,
so referring to a namespace in a type position (case 1) could still be
valid.

This commit introduces logic to the type-to-value conversion to be able
to properly deal with all type references to namespaced symbols.

Fixes #36006
Resolves FW-1995

PR Close #36106
2020-04-09 11:32:21 -07:00
Alex Rickabaugh 0a69a2832b style(compiler-cli): reformat of codebase with new clang-format version (#36520)
This commit reformats the packages/compiler-cli tree using the new version
of clang-format.

PR Close #36520
2020-04-08 14:51:08 -07:00
Ayaz Hafiz e893c5a330 fix(compiler-cli): pass real source spans where they are empty (#31805)
Some consumers of functions that take `ParseSourceSpan`s currently pass
empty and incorrect source spans. This fixes those cases.

PR Close #31805
2020-04-06 09:28:27 -07:00
JoostK 32ce8b1326 feat(compiler): add dependency info and ng-content selectors to metadata (#35695)
This commit augments the `FactoryDef` declaration of Angular decorated
classes to contain information about the parameter decorators used in
the constructor. If no constructor is present, or none of the parameters
have any Angular decorators, then this will be represented using the
`null` type. Otherwise, a tuple type is used where the entry at index `i`
corresponds with parameter `i`. Each tuple entry can be one of two types:

1. If the associated parameter does not have any Angular decorators,
   the tuple entry will be the `null` type.
2. Otherwise, a type literal is used that may declare at least one of
   the following properties:
   - "attribute": if `@Attribute` is present. The injected attribute's
   name is used as string literal type, or the `unknown` type if the
   attribute name is not a string literal.
   - "self": if `@Self` is present, always of type `true`.
   - "skipSelf": if `@SkipSelf` is present, always of type `true`.
   - "host": if `@Host` is present, always of type `true`.
   - "optional": if `@Optional` is present, always of type `true`.

   A property is only present if the corresponding decorator is used.

   Note that the `@Inject` decorator is currently not included, as it's
   non-trivial to properly convert the token's value expression to a
   type that is valid in a declaration file.

Additionally, the `ComponentDefWithMeta` declaration that is created for
Angular components has been extended to include all selectors on
`ng-content` elements within the component's template.

This additional metadata is useful for tooling such as the Angular
Language Service, as it provides the ability to offer suggestions for
directives/components defined in libraries. At the moment, such
tooling extracts the necessary information from the _metadata.json_
manifest file as generated by ngc, however this metadata representation
is being replaced by the information emitted into the declaration files.

Resolves FW-1870

PR Close #35695
2020-03-24 14:21:42 -07:00
ayazhafiz df890d7629 fix(compiler): record correct end of expression (#34690)
This commit fixes a bug with the expression parser wherein the end index
of an expression node was recorded as the start index of the next token,
not the end index of the current token.

Closes #33477
Closes https://github.com/angular/vscode-ng-language-service/issues/433

PR Close #34690
2020-03-20 10:19:02 -07:00
Alex Rickabaugh e3ecdc6a63 feat(bazel): transform generated shims (in Ivy) with tsickle (#35975)
Currently, when Angular code is built with Bazel and with Ivy, generated
factory shims (.ngfactory files) are not processed via the majority of
tsickle's transforms. This is a subtle effect of the build infrastructure,
but it boils down to a TsickleHost method `shouldSkipTsickleProcessing`.

For ngc_wrapped builds (Bazel + Angular), this method is defined in the
`@bazel/typescript` (aka bazel rules_typescript) implementation of
`CompilerHost`. The default behavior is to skip tsickle processing for files
which are not present in the original `srcs[]` of the build rule. In
Angular's case, this includes all generated shim files.

For View Engine factories this is probably desirable as they're quite
complex and they've never been tested with tsickle. Ivy factories however
are smaller and very straightforward, and it makes sense to treat them like
any other output.

This commit adjusts two independent implementations of
`shouldSkipTsickleProcessing` to enable transformation of Ivy shims:

* in `@angular/bazel` aka ngc_wrapped, the upstream `@bazel/typescript`
  `CompilerHost` is patched to treat .ngfactory files the same as their
  original source file, with respect to tsickle processing.

  It is currently not possible to test this change as we don't have any test
  that inspects tsickle output with bazel. It will be extensively tested in
  g3.

* in `ngc`, Angular's own implementation is adjusted to allow for the
  processing of shims when compiling with Ivy. This enables a unit test to
  be written to validate the correct behavior of tsickle when given a host
  that's appropriately configured to process factory shims.

For ngtsc-as-a-plugin, a similar fix will need to be submitted upstream in
tsc_wrapped.

PR Close #35848

PR Close #35975
2020-03-17 10:17:28 -07:00
Keen Yee Liau 31bec8ce61 feat(compiler): Propagate source span and value span to Variable AST (#36047)
This commit propagates the `sourceSpan` and `valueSpan` of a `VariableBinding`
in a microsyntax expression to `ParsedVariable`, and subsequently to
View Engine Variable AST and Ivy Variable AST.

Note that this commit does not propagate the `keySpan`, because it involves
significant changes to the template AST.

PR Close #36047
2020-03-16 10:52:57 -07:00
Andrew Kushnir 0bf6e58db2 fix(compiler): process `imports` first and `declarations` second while calculating scopes (#35850)
Prior to this commit, while calculating the scope for a module, Ivy compiler processed `declarations` field first and `imports` after that. That results in a couple issues:

* for Pipes with the same `name` and present in `declarations` and in an imported module, Pipe from imported module was selected. In View Engine the logic is opposite: Pipes from `declarations` field receive higher priority.
* for Directives with the same selector and present in `declarations` and in an imported module, we first invoked the logic of a Directive from `declarations` field and after that - imported Directive logic. In View Engine, it was the opposite and the logic of a Directive from the `declarations` field was invoked last.

In order to align Ivy and View Engine behavior, this commit updates the logic in which we populate module scope: we first process all imports and after that handle `declarations` field. As a result, in Ivy both use-cases listed above work similar to View Engine.

Resolves #35502.

PR Close #35850
2020-03-10 14:16:59 -04:00
Matias Niemelä 15482e7367 Revert "feat(bazel): transform generated shims (in Ivy) with tsickle (#35848)" (#35970)
This reverts commit 9ff9a072e6.

PR Close #35970
2020-03-09 17:00:14 -04:00
Alex Rickabaugh 9ff9a072e6 feat(bazel): transform generated shims (in Ivy) with tsickle (#35848)
Currently, when Angular code is built with Bazel and with Ivy, generated
factory shims (.ngfactory files) are not processed via the majority of
tsickle's transforms. This is a subtle effect of the build infrastructure,
but it boils down to a TsickleHost method `shouldSkipTsickleProcessing`.

For ngc_wrapped builds (Bazel + Angular), this method is defined in the
`@bazel/typescript` (aka bazel rules_typescript) implementation of
`CompilerHost`. The default behavior is to skip tsickle processing for files
which are not present in the original `srcs[]` of the build rule. In
Angular's case, this includes all generated shim files.

For View Engine factories this is probably desirable as they're quite
complex and they've never been tested with tsickle. Ivy factories however
are smaller and very straightforward, and it makes sense to treat them like
any other output.

This commit adjusts two independent implementations of
`shouldSkipTsickleProcessing` to enable transformation of Ivy shims:

* in `@angular/bazel` aka ngc_wrapped, the upstream `@bazel/typescript`
  `CompilerHost` is patched to treat .ngfactory files the same as their
  original source file, with respect to tsickle processing.

  It is currently not possible to test this change as we don't have any test
  that inspects tsickle output with bazel. It will be extensively tested in
  g3.

* in `ngc`, Angular's own implementation is adjusted to allow for the
  processing of shims when compiling with Ivy. This enables a unit test to
  be written to validate the correct behavior of tsickle when given a host
  that's appropriately configured to process factory shims.

For ngtsc-as-a-plugin, a similar fix will need to be submitted upstream in
tsc_wrapped.

PR Close #35848
2020-03-09 13:06:33 -04:00
Alex Rickabaugh 2c41bb8490 fix(compiler): type-checking error for duplicate variables in templates (#35674)
It's an error to declare a variable twice on a specific template:

```html
<div *ngFor="let i of items; let i = index">
</div>
```

This commit introduces a template type-checking error which helps to detect
and diagnose this problem.

Fixes #35186

PR Close #35674
2020-03-03 13:52:50 -08:00
JoostK 40039d8068 fix(ivy): narrow `NgIf` context variables in template type checker (#35125)
When the `NgIf` directive is used in a template, its context variables
can be used to capture the bound value. This is typically used together
with a pipe or function call, where the resulting value is captured in a
context variable. There's two syntax forms available:

1. Binding to `NgIfContext.ngIf` using the `as` syntax:
```html
<span *ngIf="(user$ | async) as user">{{user.name}}</span>
```

2. Binding to `NgIfContext.$implicit` using the `let` syntax:
```html
<span *ngIf="user$ | async; let user">{{user.name}}</span>
```

Because of the semantics of `ngIf`, it is known that the captured
context variable is non-nullable, however the template type checker
would not consider them as such and still report errors when
`strictNullTypes` is enabled.

This commit updates `NgIf`'s context guard to make the types of the
context variables non-nullable, avoiding the issue.

Fixes #34572

PR Close #35125
2020-02-28 07:39:57 -08:00
JoostK 3e3a1ef30d fix(ivy): support dynamic query tokens in AOT mode (#35307)
For view and content queries, the Ivy compiler attempts to statically
evaluate the predicate token so that string predicates containing
comma-separated reference names can be split into an array of strings
during compilation. When the predicate is a dynamic value that cannot be
statically interpreted at compile time, the compiler would previously
produce an error. This behavior breaks a use-case where an `InjectionToken`
is being used as query predicate, as the usage of the `new` keyword
prevents such predicates from being statically evaluated.

This commit changes the behavior to no longer produce an error for
dynamic values. Instead, the expression is emitted as is into the
generated code, postponing the evaluation to happen at runtime.

Fixes #34267
Resolves FW-1828

PR Close #35307
2020-02-27 16:05:21 -08:00
Alex Rickabaugh 173a1ac8e4 fix(ivy): better inference for circularly referenced directive types (#35622)
It's possible to pass a directive as an input to itself. Consider:

```html
<some-cmp #ref [value]="ref">
```

Since the template type-checker attempts to infer a type for `<some-cmp>`
using the values of its inputs, this creates a circular reference where the
type of the `value` input is used in its own inference:

```typescript
var _t0 = SomeCmp.ngTypeCtor({value: _t0});
```

Obviously, this doesn't work. To resolve this, the template type-checker
used to generate a `null!` expression when a reference would otherwise be
circular:

```typescript
var _t0 = SomeCmp.ngTypeCtor({value: null!});
```

This effectively asks TypeScript to infer a value for this context, and
works well to resolve this simple cycle. However, if the template
instead tries to use the circular value in a larger expression:

```html
<some-cmp #ref [value]="ref.prop">
```

The checker would generate:

```typescript
var _t0 = SomeCmp.ngTypeCtor({value: (null!).prop});
```

In this case, TypeScript can't figure out any way `null!` could have a
`prop` key, and so it infers `never` as the type. `(never).prop` is thus a
type error.

This commit implements a better fallback pattern for circular references to
directive types like this. Instead of generating a `null!` in place for the
reference, a type is inferred by calling the type constructor again with
`null!` as its input. This infers the widest possible type for the directive
which is then used to break the cycle:

```typescript
var _t0 = SomeCmp.ngTypeCtor(null!);
var _t1 = SomeCmp.ngTypeCtor({value: _t0.prop});
```

This has the desired effect of validating that `.prop` is legal for the
directive type (the type of `#ref`) while also avoiding a cycle.

Fixes #35372
Fixes #35603
Fixes #35522

PR Close #35622
2020-02-26 12:57:08 -08:00
Alex Rickabaugh 2d89b5d13d fix(ivy): provide a more detailed error message for NG6002/NG6003 (#35620)
NG6002/NG6003 are errors produced when an NgModule being compiled has an
imported or exported type which does not have the proper metadata (that is,
it doesn't appear to be an @NgModule, or @Directive, etc. depending on
context).

Previously this error message was a bit sparse. However, Github issues show
that this is the most common error users receive when for whatever reason
ngcc wasn't able to handle one of their libraries, or they just didn't run
it. So this commit changes the error message to offer a bit more useful
context, instructing users differently depending on whether the class in
question is from their own project, from NPM, or from a monorepo-style local
dependency.

PR Close #35620
2020-02-26 12:56:47 -08:00
Kristiyan Kostadinov 9228d7f15d perf(ivy): remove unused event argument in listener instructions (#35097)
Currently Ivy always generates the `$event` function argument, even if it isn't being used by the listener expressions. This can lead to unnecessary bytes being generated, because optimizers won't remove unused arguments by default. These changes add some logic to avoid adding the argument when it isn't required.

PR Close #35097
2020-02-20 15:22:13 -08:00
Andrew Kushnir 646655d09a fix(compiler): use FatalDiagnosticError to generate better error messages (#35244)
Prior to this commit, decorator handling logic in Ngtsc used `Error` to throw errors. This commit replaces most of these instances with `FatalDiagnosticError` class, which provider a better diagnostics error (including location of the problematic code).

PR Close #35244
2020-02-20 11:25:23 -08:00
JoostK 5de5b52beb fix(ivy): repeat template guards to narrow types in event handlers (#35193)
In Ivy's template type checker, event bindings are checked in a closure
to allow for accurate type inference of the `$event` parameter. Because
of the closure, any narrowing effects of template guards will no longer
be in effect when checking the event binding, as TypeScript assumes that
the guard outside of the closure may no longer be true once the closure
is invoked. For more information on TypeScript's Control Flow Analysis,
please refer to https://github.com/microsoft/TypeScript/issues/9998.

In Angular templates, it is known that an event binding can only be
executed when the view it occurs in is currently rendered, hence the
corresponding template guard is known to hold during the invocation of
an event handler closure. As such, it is desirable that any narrowing
effects from template guards are still in effect within the event
handler closure.

This commit tweaks the generated Type-Check Block (TCB) to repeat all
template guards within an event handler closure. This achieves the
narrowing effect of the guards even within the closure.

Fixes #35073

PR Close #35193
2020-02-07 13:06:00 -08:00
Alex Rickabaugh c35671c0a4 fix(ivy): template type-check errors from TS should not use NG error codes (#35146)
A bug previously caused the template type-checking diagnostics produced by
TypeScript for template expressions to use -99-prefixed error codes. These
codes are converted to "NG" errors instead of "TS" errors during diagnostic
printing. This commit fixes the issue.

PR Close #35146
2020-02-04 15:59:01 -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
Andrew Kushnir 6e5cfd2cd2 fix(ivy): catch FatalDiagnosticError thrown from preanalysis phase (#34801)
Component's decorator handler exposes `preanalyze` method to preload async resources (templates, stylesheets). The logic in preanalysis phase may throw `FatalDiagnosticError` errors that contain useful information regarding the origin of the problem. However these errors from preanalysis phase were not intercepted in TraitCompiler, resulting in just error message text be displayed. This commit updates the logic to handle FatalDiagnosticError and transform it before throwing, so that the result diagnostic errors contain the necessary info.

PR Close #34801
2020-01-27 10:58:27 -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 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
Alex Rickabaugh 24b2f1da2b refactor(ivy): introduce the 'core' package and split apart NgtscProgram (#34887)
Previously, NgtscProgram lived in the main @angular/compiler-cli package
alongside the legacy View Engine compiler. As a result, the main package
depended on all of the ngtsc internal packages, and a significant portion of
ngtsc logic lived in NgtscProgram.

This commit refactors NgtscProgram and moves the main logic of compilation
into a new 'core' package. The new package defines a new API which enables
implementers of TypeScript compilers (compilers built using the TS API) to
support Angular transpilation as well. It involves a new NgCompiler type
which takes a ts.Program and performs Angular analysis and transformations,
as well as an NgCompilerHost which wraps an input ts.CompilerHost and adds
any extra Angular files.

Together, these two classes are used to implement a new NgtscProgram which
adapts the legacy api.Program interface used by the View Engine compiler
onto operations on the new types. The new NgtscProgram implementation is
significantly smaller and easier to reason about.

The new NgCompilerHost replaces the previous GeneratedShimsHostWrapper which
lived in the 'shims' package.

A new 'resource' package is added to support the HostResourceLoader which
previously lived in the outer compiler package.

As a result of the refactoring, the dependencies of the outer
@angular/compiler-cli package on ngtsc internal packages are significantly
trimmed.

This refactoring was driven by the desire to build a plugin interface to the
compiler so that tsc_wrapped (another consumer of the TS compiler APIs) can
perform Angular transpilation on user request.

PR Close #34887
2020-01-24 08:59:59 -08:00
Alex Rickabaugh 5b2fa3cfd3 fix(ivy): correctly emit component when it's removed from its module (#34912)
This commit fixes a bug in the incremental rebuild engine of ngtsc, where if
a component was removed from its NgModule, it would not be properly
re-emitted.

The bug stemmed from the fact that whether to emit a file was a decision
based purely on the updated dependency graph, which captures the dependency
structure of the rebuild program. This graph has no edge from the component
to its former module (as it was removed, of course), so the compiler
erroneously decides not to emit the component.

The bug here is that the compiler does know, from the previous dependency
graph, that the component file has logically changed, since its previous
dependency (the module file) has changed. This information was not carried
forward into the set of files which need to be emitted, because it was
assumed that the updated dependency graph was a more accurate source of that
information.

With this commit, the set of files which need emit is pre-populated with the
set of logically changed files, to cover edge cases like this.

Fixes #34813

PR Close #34912
2020-01-23 13:30:10 -08:00
Alex Rickabaugh 0c8d085666 fix(ivy): use any for generic context checks when !strictTemplates (#34649)
Previously, the template type-checker would always construct a generic
template context type with correct bounds, even when strictTemplates was
disabled. This meant that type-checking of expressions involving that type
was stricter than View Engine.

This commit introduces a 'strictContextGenerics' flag which behaves
similarly to other 'strictTemplates' flags, and switches the inference of
generic type parameters on the component context based on the value of this
flag.

PR Close #34649
2020-01-23 10:31:48 -08:00
Alex Rickabaugh cb11380515 fix(ivy): disable use of aliasing in template type-checking (#34649)
FileToModuleHost aliasing supports compilation within environments that have
two properties:

1. A `FileToModuleHost` exists which defines canonical module names for any
   given TS file.
2. Dependency restrictions exist which prevent the import of arbitrary files
   even if such files are within the .d.ts transitive closure of a
   compilation ("strictdeps").

In such an environment, generated imports can only go through import paths
which are already present in the user program. The aliasing system supports
the generation and consumption of such imports at runtime.

`FileToModuleHost` aliasing does not emit re-exports in .d.ts files. This
means that it's safe to rely on alias re-exports in generated .js code (they
are guaranteed to exist at runtime) but not in template type-checking code
(since TS will not be able to follow such imports). Therefore, non-aliased
imports should be used in template type-checking code.

This commit adds a `NoAliasing` flag to `ImportFlags` and sets it when
generating imports in template type-checking code. The testing environment
is also patched to support resolution of FileToModuleHost canonical paths
within the template type-checking program, enabling testing of this change.

PR Close #34649
2020-01-23 10:31:48 -08:00
Alex Rickabaugh 22c957a93d fix(ivy): type-checking of properties which map to multiple fields (#34649)
It's possible to declare multiple inputs for a directive/component which all
map to the same property name. This is usually done in error, as only one of
any bindings to the property will "win".

In the template type-checker, an error was previously being raised as a
result of this ambiguity. Specifically, a type constructor was produced
which required a binding for each field, but only one of the fields had
a value via the binding. TypeScript would (rightfully) error on missing
values for the remaining fields. This ultimately was happening when the
code which generated the default values for "unset" inputs belonging to
directives or pipes used the final mapping from properties to fields as
a source for field names.

Instead, this commit uses the original list of fields to generate unset
input values, which correctly provides values for fields which shared a
property name but didn't receive the final binding.

PR Close #34649
2020-01-23 10:31:47 -08:00
Andrew Kushnir 39ec188003 fix(ivy): more accurate detection of pipes in host bindings (#34655)
Pipes in host binding expressions are not supported in View Engine and Ivy, but in some more complex cases (like `(value | pipe) === true`) compiler was not reporting errors. This commit extends Ivy logic to detect pipes in host binding expressions and throw in cases bindings are present. View Engine behavior remains the same.

PR Close #34655
2020-01-21 13:22:00 -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
Greg Magolan dcff76e8b9 refactor: handle breaking changes in rules_nodejs 1.0.0 (#34736)
The major one that affects the angular repo is the removal of the bootstrap attribute in nodejs_binary, nodejs_test and jasmine_node_test in favor of using templated_args --node_options=--require=/path/to/script. The side-effect of this is that the bootstrap script does not get the require.resolve patches with explicitly loading the targets _loader.js file.

PR Close #34736
2020-01-15 14:58:07 -05:00
crisbeto c3c72f689a fix(ivy): handle overloaded constructors in ngtsc (#34590)
Currently ngtsc looks for the first `ConstructorDeclaration` when figuring out what the parameters are so that it can generate the DI instructions. The problem is that if a constructor has overloads, it'll have several `ConstructorDeclaration` members with a different number of parameters. These changes tweak the logic so it looks for the constructor implementation.

PR Close #34590
2020-01-14 15:17:09 -08:00
atscott 538d0446b5 Revert "refactor: handle breaking changes in rules_nodejs 1.0.0 (#34589)" (#34730)
This reverts commit 9bb349e1c8.

PR Close #34730
2020-01-10 14:12:15 -08:00
atscott 5e60215470 Revert "test: handle bootstrap templated_args in jasmine_node_test defaults.bzl (#34589)" (#34730)
This reverts commit da4782e67f.

PR Close #34730
2020-01-10 14:12:15 -08:00
Greg Magolan da4782e67f test: handle bootstrap templated_args in jasmine_node_test defaults.bzl (#34589)
PR Close #34589
2020-01-10 08:31:59 -08:00
Greg Magolan 9bb349e1c8 refactor: handle breaking changes in rules_nodejs 1.0.0 (#34589)
The major one that affects the angular repo is the removal of the bootstrap attribute in nodejs_binary, nodejs_test and jasmine_node_test in favor of using templated_args --node_options=--require=/path/to/script. The side-effect of this is that the bootstrap script does not get the require.resolve patches with explicitly loading the targets _loader.js file.

PR Close #34589
2020-01-10 08:31:59 -08:00
JoostK e116816131 refactor(ivy): let `strictTemplates` imply `fullTemplateTypeCheck` (#34195)
Previously, it was required that both `fullTemplateTypeCheck` and
`strictTemplates` had to be enabled for strict mode to be enabled. This
is strange, as `strictTemplates` implies `fullTemplateTypeCheck`. This
commit makes setting the `fullTemplateTypeCheck` flag optional so that
strict mode can be enabled by just setting `strictTemplates`.

PR Close #34195
2020-01-06 11:07:54 -08:00
JoostK 2e82357611 refactor(ivy): verify template type check options are compatible (#34195)
It is now an error if '"fullTemplateTypeCheck"' is disabled while
`"strictTemplates"` is enabled, as enabling the latter implies that the
former is also enabled.

PR Close #34195
2020-01-06 11:07:54 -08:00
JoostK 1de49ba369 refactor(ivy): consistently translate types to `ts.TypeNode` (#34021)
The compiler has a translation mechanism to convert from an Angular
`Type` to a `ts.TypeNode`, as appropriate. Prior to this change, it
would translate certain Angular expressions into their value equivalent
in TypeScript, instead of the correct type equivalent. This was possible
as the `ExpressionVisitor` interface is not strictly typed, with `any`s
being used for return values.

For example, a literal object was translated into a
`ts.ObjectLiteralExpression`, containing `ts.PropertyAssignment` nodes
as its entries. This has worked without issues as their printed
representation is identical, however it was incorrect from a semantic
point of view. Instead, a `ts.TypeLiteralNode` is created with
`ts.PropertySignature` as its members, which corresponds with the type
declaration of an object literal.

PR Close #34021
2020-01-06 11:06:07 -08:00
crisbeto cf37c003ff feat(ivy): error in ivy when inheriting a ctor from an undecorated base (#34460)
Angular View Engine uses global knowledge to compile the following code:

```typescript
export class Base {
  constructor(private vcr: ViewContainerRef) {}
}

@Directive({...})
export class Dir extends Base {
  // constructor inherited from base
}
```

Here, `Dir` extends `Base` and inherits its constructor. To create a `Dir`
the arguments to this inherited constructor must be obtained via dependency
injection. View Engine is able to generate a correct factory for `Dir` to do
this because via metadata it knows the arguments of `Base`'s constructor,
even if `Base` is declared in a different library.

In Ivy, DI is entirely a runtime concept. Currently `Dir` is compiled with
an ngDirectiveDef field that delegates its factory to `getInheritedFactory`.
This looks for some kind of factory function on `Base`, which comes up
empty. This case looks identical to an inheritance chain with no
constructors, which works today in Ivy.

Both of these cases will now become an error in this commit. If a decorated
class inherits from an undecorated base class, a diagnostic is produced
informing the user of the need to either explicitly declare a constructor or
to decorate the base class.

PR Close #34460
2019-12-18 15:04:49 -08:00
crisbeto dcc8ff4ce7 feat(ivy): throw compilation error when providing undecorated classes (#34460)
Adds a compilation error if the consumer tries to pass in an undecorated class into the `providers` of an `NgModule`, or the `providers`/`viewProviders` arrays of a `Directive`/`Component`.

PR Close #34460
2019-12-18 15:04:49 -08:00
Alex Rickabaugh 498a2ffba3 fix(ivy): don't produce template diagnostics when scope is invalid (#34460)
Previously, ngtsc would perform scope analysis (which directives/pipes are
available inside a component's template) and template type-checking of that
template as separate steps. If a component's scope was somehow invalid (e.g.
its NgModule imported something which wasn't another NgModule), the
component was treated as not having a scope. This meant that during template
type-checking, errors would be produced for any invalid expressions/usage of
other components that should have been in the scope.

This commit changes ngtsc to skip template type-checking of a component if
its scope is erroneous (as opposed to not present in the first place). Thus,
users aren't overwhelmed with diagnostic errors for the template and are
only informed of the root cause of the problem: an invalid NgModule scope.

Fixes #33849

PR Close #34460
2019-12-18 15:04:49 -08:00
Alex Rickabaugh 763f8d470a fix(ivy): validate the NgModule declarations field (#34404)
This commit adds three previously missing validations to
NgModule.declarations:

1. It checks that declared classes are actually within the current
   compilation.

2. It checks that declared classes are directives, components, or pipes.

3. It checks that classes are declared in at most one NgModule.

PR Close #34404
2019-12-17 11:39:48 -08:00
Alex Rickabaugh 6ba5fdc208 fix(ivy): generate a better error for template var writes (#34339)
In Ivy it's illegal for a template to write to a template variable. So the
template:

```html
<ng-template let-somevar>
  <button (click)="somevar = 3">Set var to 3</button>
</ng-template>
```

is erroneous and previously would fail to compile with an assertion error
from the `TemplateDefinitionBuilder`. This error wasn't particularly user-
friendly, though, as it lacked the context of which template or where the
error occurred.

In this commit, a new check in template type-checking is added which detects
such erroneous writes and produces a true diagnostic with the appropriate
context information.

Closes #33674

PR Close #34339
2019-12-12 13:13:32 -08:00
Alex Rickabaugh 74edde0a94 perf(ivy): reuse prior analysis work during incremental builds (#34288)
Previously, the compiler performed an incremental build by analyzing and
resolving all classes in the program (even unchanged ones) and then using
the dependency graph information to determine which .js files were stale and
needed to be re-emitted. This algorithm produced "correct" rebuilds, but the
cost of re-analyzing the entire program turned out to be higher than
anticipated, especially for component-heavy compilations.

To achieve performant rebuilds, it is necessary to reuse previous analysis
results if possible. Doing this safely requires knowing when prior work is
viable and when it is stale and needs to be re-done.

The new algorithm implemented by this commit is such:

1) Each incremental build starts with knowledge of the last known good
   dependency graph and analysis results from the last successful build,
   plus of course information about the set of files changed.

2) The previous dependency graph's information is used to determine the
   set of source files which have "logically" changed. A source file is
   considered logically changed if it or any of its dependencies have
   physically changed (on disk) since the last successful compilation. Any
   logically unchanged dependencies have their dependency information copied
   over to the new dependency graph.

3) During the `TraitCompiler`'s loop to consider all source files in the
   program, if a source file is logically unchanged then its previous
   analyses are "adopted" (and their 'register' steps are run). If the file
   is logically changed, then it is re-analyzed as usual.

4) Then, incremental build proceeds as before, with the new dependency graph
   being used to determine the set of files which require re-emitting.

This analysis reuse avoids template parsing operations in many circumstances
and significantly reduces the time it takes ngtsc to rebuild a large
application.

Future work will increase performance even more, by tackling a variety of
other opportunities to reuse or avoid work.

PR Close #34288
2019-12-12 13:11:45 -08:00
JoostK b72c7a89a9 refactor(ivy): include generic type for `ModuleWithProviders` in .d.ts files (#34235)
The `ModuleWithProviders` type has an optional type parameter that
should be specified to indicate what NgModule class will be provided.
This enables the Ivy compiler to statically determine the NgModule type
from the declaration files. This type parameter will become required in
the future, however to aid in the migration the compiler will detect
code patterns where using `ModuleWithProviders` as return type is
appropriate, in which case it transforms the emitted .d.ts files to
include the generic type argument.

This should reduce the number of occurrences where `ModuleWithProviders`
is referenced without its generic type argument.

Resolves FW-389

PR Close #34235
2019-12-10 16:34:47 -08:00
JoostK 22ad701134 fix(ivy): inherit static coercion members from base classes (#34296)
For Ivy's template type checker it is possible to let a directive
specify static members to allow a wider type for some input:

```typescript
export class MatSelect {
  @Input() disabled: boolean;

  static ngAcceptInputType_disabled: boolean | string;
}
```

This allows a binding to the `MatSelect.disabled` input to be of type
boolean or string, whereas the `disabled` property itself is only of
type boolean.

Up until now, any static `ngAcceptInputType_*` property was not
inherited for subclasses of a directive class. This is cumbersome, as
the directive's inputs are inherited, so any acceptance member should as
well. To resolve this limitation, this commit extends the flattening of
directive metadata to include the acceptance members.

Fixes #33830
Resolves FW-1759

PR Close #34296
2019-12-10 16:31:23 -08:00
Kristiyan Kostadinov cca2616637 refactor(common): add defaults to new generic parameters (#34206)
This is a follow-up to #33997 where some new generic parameters were added without defaults which is technically a breaking change. These changes add the defaults.

PR Close #34206
2019-12-03 16:16:30 -08:00
crisbeto e6909bda89 fix(ivy): incorrectly validating html foreign objects inside svg (#34178)
Fixes ngtsc incorrectly logging an unknown element diagnostic for HTML elements that are inside an SVG `foreignObject` with the `xhtml` namespace.

Fixes #34171.

PR Close #34178
2019-12-03 10:29:45 -08:00
Pete Bacon Darwin e524322c43 refactor(compiler): i18n - render legacy i18n message ids (#34135)
Now that `@angular/localize` can interpret multiple legacy message ids in the
metablock of a `$localize` tagged template string, this commit adds those
ids to each i18n message extracted from component templates, but only if
the `enableI18nLegacyMessageIdFormat` is not `false`.

PR Close #34135
2019-12-03 10:15:53 -08:00