Advance the state of ngtsc's template type-checking to the point where it can handle AIO. #29698

alxhub · 2019-04-04T01:56:36Z

No description provided.

JoostK

Truly awesome work! 🎉

packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts

packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts

packages/compiler-cli/src/ngtsc/typecheck/src/environment.ts

packages/compiler-cli/src/ngtsc/program.ts

packages/compiler-cli/src/ngtsc/typecheck/src/context.ts

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_file.ts

packages/bazel/src/ng_module.bzl

packages/compiler-cli/src/ngtsc/typecheck/test/type_check_block_spec.ts

packages/compiler-cli/src/ngtsc/typecheck/src/type_check_block.ts

packages/compiler-cli/ngcc/src/analysis/decoration_analyzer.ts

packages/compiler-cli/src/ngtsc/metadata/src/api.ts

packages/compiler-cli/src/ngtsc/metadata/src/util.ts

packages/compiler-cli/test/ngtsc/template_typecheck_spec.ts

packages/compiler-cli/src/ngtsc/typecheck/src/expression.ts

petebacondarwin · 2019-04-16T16:39:28Z

The commit message

perf(ivy): template type-check the entire program in 1 file if possible

has a typo:

parameters are present, to the approach taken is actually a hybrid. These

-> so the approach

petebacondarwin

Amazing work @alxhub - quite an opus. Looks good - a few minor nits and it needs rebase conflicts resolving.

alxhub · 2019-04-18T23:06:02Z

I cannot set the google3 status to successful:

"This SHA and context has reached the maximum number of statuses."

…29698) This commit adds support for the generation of type-checking expressions for forms which were previously unsupported: * array literals * map literals * keyed property accesses * non-null assertions Testing strategy: TCB tests included. Fixes #29327 FW-1218 #resolve PR Close #29698

…9698) Previously the template type-checking engine processed templates in a linear manner, and could not handle '#' references within a template. One reason for this is that '#' references are non-linear - a reference can be used before its declaration. Consider the template: ```html {{ref.value}} <input #ref> ``` Accommodating this required refactoring the type-checking code generator to be able to produce Type Check Block (TCB) code non-linearly. Now, each template is processed and a list of TCB operations (`TcbOp`s) are created. Non-linearity is modeled via dependencies between operations, with the appropriate protection in place for circular dependencies. Testing strategy: TCB tests included. PR Close #29698

Previously, bindings to [class] and [style] were treated like any other property binding. That is, they would result in type-checking code that attempted to write directly to .class or .style on the element node. This is incorrect, however - the mapping from Angular's [class] and [style] onto the DOM properties is non-trivial. For now, this commit avoids the issue by only checking the expressions themselves and not the assignment to the element properties. Testing strategy: TCB tests included. PR Close #29698

Previously, metadata registration (the recording of collected metadata during analysis of directives, pipes, and NgModules) was only used to produce the `LocalModuleScope`, and thus was handled by the `LocalModuleScopeRegistry`. However, the template type-checker also needs information about registered directives, outside of the NgModule scope determinations. Rather than reuse the scope registry for an unintended purpose, this commit introduces new abstractions for metadata registration and lookups in a separate 'metadata' package, which the scope registry implements. This paves the way for a future commit to make use of this metadata for the template type-checking system. Testing strategy: this commit is a refactoring which introduces no new functionality, so existing tests are sufficient. PR Close #29698

…29698) Previously the template type-checking code only considered the metadata of directive classes actually referenced in the template. If those directives had base classes, any inputs/outputs/etc of the base classes were not tracked when generating the TCB. This resulted in bindings to those inputs being incorrectly attributed to the host component or element. This commit uses the new metadata package to follow directive inheritance chains and use the full metadata for a directive for TCB generation. Testing strategy: Template type-checking tests included. PR Close #29698

…ng (#29698) View Engine's implementation of naive template type-checking is less advanced than the current Ivy implementation. As a result, Ivy catches lots of typing bugs which VE does not. As a result, it's necessary to tone down the Ivy template type-checker in the default case. This commit introduces a mechanism for doing that, by passing a config to the template type-checking engine. Through this configuration, particular checks can be loosened or disabled entirely. Testing strategy: TCB tests included. PR Close #29698

…#29698) This commit adds support in the template type-checking engine for handling the logical not operation and the safe navigation operation. Safe navigation in particular is tricky, as the View Engine implementation has a rather inconvenient flaw. View Engine checks a safe navigation operation `a?.b` as: ```typescript (a != null ? a!.b : null as any) ``` The type of this expression is always 'any', as the false branch of the ternary has type 'any'. Thus, using null-safe navigation throws away the type of the result, and breaks type-checking for the rest of the expression. A flag is introduced in the type-checking configuration to allow Ivy to mimic this behavior when needed. Testing strategy: TCB tests included. PR Close #29698

…le (#29698) The template type-checking engine previously would assemble a type-checking program by inserting Type Check Blocks (TCBs) into existing user files. This approach proved expensive, as TypeScript has to re-parse and re-type-check those files when processing the type-checking program. Instead, a far more performant approach is to augment the program with a single type-checking file, into which all TCBs are generated. Additionally, type constructors are also inlined into this file. This is not always possible - both TCBs and type constructors can sometimes require inlining into user code, particularly if bound generic type parameters are present, so the approach taken is actually a hybrid. These operations are inlined if necessary, but are otherwise generated in a single file. It is critically important that the original program also include an empty version of the type-checking file, otherwise the shape of the two programs will be different and TypeScript will throw away all the old program information. This leads to a painfully slow type checking pass, on the same order as the original program creation. A shim to generate this file in the original program is therefore added. Testing strategy: this commit is largely a refactor with no externally observable behavioral differences, and thus no tests are needed. PR Close #29698

This commit adds support for template type-checking a pipe binding which previously was not handled by the type-checking engine. In compatibility mode, the arguments to transform() are not checked and the type returned by a pipe is 'any'. In full type-checking mode, the transform() method's type signature is used to check the pipe usage and infer the return type of the pipe. Testing strategy: TCB tests included. PR Close #29698

Previously, Template.templateAttrs was introduced to capture attribute bindings which originated from microsyntax (e.g. bindings in *ngFor="..."). This means that a Template node can have two different structures, depending on whether it originated from microsyntax or from a literal <ng-template>. In the literal case, the node behaves much like an Element node, it has attributes, inputs, and outputs which determine which directives apply. In the microsyntax case, though, only the templateAttrs should be used to determine which directives apply. Previously, both the t2_binder and the TemplateDefinitionBuilder were using the wrong set of attributes to match directives - combining the attributes, inputs, outputs, and templateAttrs of the Template node regardless of its origin. In the TDB's case this wasn't a problem, since the TDB collects a global Set of directives used in the template, so it didn't matter whether the directive was also recognized on the <ng-template>. t2_binder's API distinguishes between directives on specific nodes, though, so it's more sensitive to mismatching. In particular, this showed up as an assertion failure in template type- checking in certain cases, when a directive was accidentally matched on a microsyntax template element and also had a binding which referenced a variable declared in the microsyntax. This resulted in the type-checker attempting to generate a reference to a variable that didn't exist in that scope. The fix is to distinguish between the two cases and select the appropriate set of attributes to match on accordingly. Testing strategy: tested in the t2_binder tests. PR Close #29698

This commit adds support in the template type-checking engine for the $any cast operation. Testing strategy: TCB tests included. PR Close #29698

…#29698) Template type-checking is enabled by default in the View Engine compiler. The feature in Ivy is not quite ready for this yet, so this flag will temporarily control whether templates are type-checked in ngtsc. The goal is to remove this flag after rolling out template type-checking in google3 in Ivy mode, and making sure the feature is as compatible with the View Engine implementation as possible. Initially, the default value of the flag will leave checking disabled. PR Close #29698

This commit adds a test suite for the Type Check Block generation which doesn't require running the entire compiler (specifically, it doesn't even require the creation of a ts.Program). PR Close angular#29698

…ngular#29698) This commit adds support for the generation of type-checking expressions for forms which were previously unsupported: * array literals * map literals * keyed property accesses * non-null assertions Testing strategy: TCB tests included. Fixes angular#29327 FW-1218 #resolve PR Close angular#29698

…gular#29698) Previously the template type-checking engine processed templates in a linear manner, and could not handle '#' references within a template. One reason for this is that '#' references are non-linear - a reference can be used before its declaration. Consider the template: ```html {{ref.value}} <input #ref> ``` Accommodating this required refactoring the type-checking code generator to be able to produce Type Check Block (TCB) code non-linearly. Now, each template is processed and a list of TCB operations (`TcbOp`s) are created. Non-linearity is modeled via dependencies between operations, with the appropriate protection in place for circular dependencies. Testing strategy: TCB tests included. PR Close angular#29698

Previously, bindings to [class] and [style] were treated like any other property binding. That is, they would result in type-checking code that attempted to write directly to .class or .style on the element node. This is incorrect, however - the mapping from Angular's [class] and [style] onto the DOM properties is non-trivial. For now, this commit avoids the issue by only checking the expressions themselves and not the assignment to the element properties. Testing strategy: TCB tests included. PR Close angular#29698

…#29698) Previously, metadata registration (the recording of collected metadata during analysis of directives, pipes, and NgModules) was only used to produce the `LocalModuleScope`, and thus was handled by the `LocalModuleScopeRegistry`. However, the template type-checker also needs information about registered directives, outside of the NgModule scope determinations. Rather than reuse the scope registry for an unintended purpose, this commit introduces new abstractions for metadata registration and lookups in a separate 'metadata' package, which the scope registry implements. This paves the way for a future commit to make use of this metadata for the template type-checking system. Testing strategy: this commit is a refactoring which introduces no new functionality, so existing tests are sufficient. PR Close angular#29698

…ngular#29698) Previously the template type-checking code only considered the metadata of directive classes actually referenced in the template. If those directives had base classes, any inputs/outputs/etc of the base classes were not tracked when generating the TCB. This resulted in bindings to those inputs being incorrectly attributed to the host component or element. This commit uses the new metadata package to follow directive inheritance chains and use the full metadata for a directive for TCB generation. Testing strategy: Template type-checking tests included. PR Close angular#29698

…ng (angular#29698) View Engine's implementation of naive template type-checking is less advanced than the current Ivy implementation. As a result, Ivy catches lots of typing bugs which VE does not. As a result, it's necessary to tone down the Ivy template type-checker in the default case. This commit introduces a mechanism for doing that, by passing a config to the template type-checking engine. Through this configuration, particular checks can be loosened or disabled entirely. Testing strategy: TCB tests included. PR Close angular#29698

…angular#29698) This commit adds support in the template type-checking engine for handling the logical not operation and the safe navigation operation. Safe navigation in particular is tricky, as the View Engine implementation has a rather inconvenient flaw. View Engine checks a safe navigation operation `a?.b` as: ```typescript (a != null ? a!.b : null as any) ``` The type of this expression is always 'any', as the false branch of the ternary has type 'any'. Thus, using null-safe navigation throws away the type of the result, and breaks type-checking for the rest of the expression. A flag is introduced in the type-checking configuration to allow Ivy to mimic this behavior when needed. Testing strategy: TCB tests included. PR Close angular#29698

…le (angular#29698) The template type-checking engine previously would assemble a type-checking program by inserting Type Check Blocks (TCBs) into existing user files. This approach proved expensive, as TypeScript has to re-parse and re-type-check those files when processing the type-checking program. Instead, a far more performant approach is to augment the program with a single type-checking file, into which all TCBs are generated. Additionally, type constructors are also inlined into this file. This is not always possible - both TCBs and type constructors can sometimes require inlining into user code, particularly if bound generic type parameters are present, so the approach taken is actually a hybrid. These operations are inlined if necessary, but are otherwise generated in a single file. It is critically important that the original program also include an empty version of the type-checking file, otherwise the shape of the two programs will be different and TypeScript will throw away all the old program information. This leads to a painfully slow type checking pass, on the same order as the original program creation. A shim to generate this file in the original program is therefore added. Testing strategy: this commit is largely a refactor with no externally observable behavioral differences, and thus no tests are needed. PR Close angular#29698

…29698) This commit adds support for template type-checking a pipe binding which previously was not handled by the type-checking engine. In compatibility mode, the arguments to transform() are not checked and the type returned by a pipe is 'any'. In full type-checking mode, the transform() method's type signature is used to check the pipe usage and infer the return type of the pipe. Testing strategy: TCB tests included. PR Close angular#29698

) Previously, Template.templateAttrs was introduced to capture attribute bindings which originated from microsyntax (e.g. bindings in *ngFor="..."). This means that a Template node can have two different structures, depending on whether it originated from microsyntax or from a literal <ng-template>. In the literal case, the node behaves much like an Element node, it has attributes, inputs, and outputs which determine which directives apply. In the microsyntax case, though, only the templateAttrs should be used to determine which directives apply. Previously, both the t2_binder and the TemplateDefinitionBuilder were using the wrong set of attributes to match directives - combining the attributes, inputs, outputs, and templateAttrs of the Template node regardless of its origin. In the TDB's case this wasn't a problem, since the TDB collects a global Set of directives used in the template, so it didn't matter whether the directive was also recognized on the <ng-template>. t2_binder's API distinguishes between directives on specific nodes, though, so it's more sensitive to mismatching. In particular, this showed up as an assertion failure in template type- checking in certain cases, when a directive was accidentally matched on a microsyntax template element and also had a binding which referenced a variable declared in the microsyntax. This resulted in the type-checker attempting to generate a reference to a variable that didn't exist in that scope. The fix is to distinguish between the two cases and select the appropriate set of attributes to match on accordingly. Testing strategy: tested in the t2_binder tests. PR Close angular#29698

This commit adds support in the template type-checking engine for the $any cast operation. Testing strategy: TCB tests included. PR Close angular#29698

…angular#29698) Template type-checking is enabled by default in the View Engine compiler. The feature in Ivy is not quite ready for this yet, so this flag will temporarily control whether templates are type-checked in ngtsc. The goal is to remove this flag after rolling out template type-checking in google3 in Ivy mode, and making sure the feature is as compatible with the View Engine implementation as possible. Initially, the default value of the flag will leave checking disabled. PR Close angular#29698

angular-automatic-lock-bot · 2019-09-14T23:52:04Z

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

_{This action has been performed automatically by a bot.}

googlebot added the cla: yes label Apr 4, 2019

alxhub force-pushed the ngtsc/ttc/aio branch from a4ff3c0 to 0d7c3e5 Compare April 4, 2019 16:59

jasonaden added the area: core Issues related to the framework runtime label Apr 4, 2019

ngbot bot added this to the needsTriage milestone Apr 4, 2019

alxhub force-pushed the ngtsc/ttc/aio branch 3 times, most recently from 25119b3 to 0d91f57 Compare April 4, 2019 22:57

JoostK reviewed Apr 6, 2019

View reviewed changes

alxhub force-pushed the ngtsc/ttc/aio branch 4 times, most recently from 4c4e352 to cf4c6bd Compare April 10, 2019 22:42

alxhub marked this pull request as ready for review April 10, 2019 22:48

alxhub requested review from a team as code owners April 10, 2019 22:48

alexeagle reviewed Apr 10, 2019

View reviewed changes

packages/bazel/src/ng_module.bzl Show resolved Hide resolved