Standalone Compiler Updates #45672

alxhub · 2022-04-18T19:38:30Z

This PR bundles together a number of commits that round out the implementation of standalone components/directives/pipes in the AOT compiler and linker.

packages/compiler/src/render3/partial/directive.ts

packages/core/src/render3/interfaces/definition.ts

packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts

atscott · 2022-04-19T17:14:50Z

packages/compiler-cli/src/ngtsc/scope/src/standalone.ts

+      private dtsModuleReader: DtsModuleScopeResolver) {}
+
+  getScopeForComponent(clazz: ClassDeclaration): StandaloneScope|null {
+    if (!this.cache.has(clazz)) {


Suggested change

if (!this.cache.has(clazz)) {

if (this.cache.has(clazz)) {

return this.cache.get(clazz)!;

}

This is the typical pattern for memoized computations in the compiler. It's basically:

if (!cache.has(key)) { const result = computeThing(); cache.set(key, result); } return cache.get(key)!;

This structure helps avoid a common pitfall of returning the result without putting it in the cache, since there's one clear return point and it always reads from the canonical cached location.

JoostK

LGTM overall. We should not forget about the .d.ts emit side of standalone components.

JoostK · 2022-04-19T18:19:31Z

packages/compiler/src/render3/partial/api.ts

+    R3DeclareDirectiveDependencyMetadata|R3DeclarePipeDependencyMetadata;
+
+export interface R3DeclareDirectiveDependencyMetadata {
+  kind: 'directive'|'component';


FYI, we use symbolic references to enum types in other places, e.g. the target type of a factory function:

target: i0.ɵɵFactoryTarget.Component

The current approach with string literals does avoid the need for a stable symbol so is perhaps more desirable, but I'm mentioning it here if we want to unify these for consistency.

Good to know - I think I'll keep the string constants for now as it's more reliable.

JoostK · 2022-04-19T18:20:31Z

packages/compiler/src/render3/partial/api.ts

+   * List of dependencies which matched in the template, including sufficient
+   * metadata for each
+   */
+  dependencies?: R3DeclareTemplateDependencyMetadata[];


We should arguably bump the minVersion of components because of this change (in a followup commit/PR is fine).

👍 agreed - and for directives/pipes due to the need to compile the standalone flag.

packages/compiler/src/render3/view/api.ts

JoostK · 2022-04-19T18:27:04Z

packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts

      const cyclesFromPipes = new Map<UsedPipe, Cycle>();
-      for (const usedPipe of usedPipes) {
-        const cycle = this._checkForCyclicImport(usedPipe.importedFile, usedPipe.type, context);
+      for (const usedDep of declarations) {


In the future, I believe we should be able to skip the cyclic dep check for standalone components as any reference we may need to generate an import from is already part of the import graph from the standalone component.

Agreed - I need to think through the logic fully but I believe you're right and we can skip it.

JoostK · 2022-04-19T18:30:03Z

packages/compiler/src/render3/partial/api.ts

@@ -249,6 +249,12 @@ export interface R3DeclarePipeDependencyMetadata {
  type: o.Expression|(() => o.Expression);
 }

+export interface R3DeclareNgModuleDependencyMetadata {
+  kind: 'ngmodule';


My preference would be ngModule here.

packages/compiler/src/render3/partial/component.ts

JoostK · 2022-04-19T18:35:56Z

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

+    const dependencies = scope.kind === ComponentScopeKind.NgModule ?
+        scope.compilation.dependencies :
+        scope.dependencies;


I'd move this below checking the cache to make it more apparent when this is actually needed.

JoostK · 2022-04-19T18:52:12Z

packages/compiler-cli/src/ngtsc/scope/src/standalone.ts

+      // A standalone component always has itself in scope, so add `clazzMeta` during
+      // initialization.


Noting this here; it's a bit unclear to me what the status of the JIT compilation is at this point, but we should ensure that we account for this situation in JIT as well.

AndrewKushnir

Awesome work @alxhub 👍

Just left a few minor comments, none of them are blocking and up to you to address or ignore (I'm ok either way :)).

packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts

packages/compiler/src/jit_compiler_facade.ts

AndrewKushnir · 2022-04-19T18:57:40Z

packages/compiler/src/jit_compiler_facade.ts

+
+function convertDirectiveDeclarationToMetadata(
+    declaration: R3DeclareDirectiveDependencyFacade,
+    isComponent: true|null = null): R3DirectiveDependencyMetadata {


Can we use boolean here instead?

Suggested change

isComponent: true|null = null): R3DirectiveDependencyMetadata {

isComponent: boolean = false): R3DirectiveDependencyMetadata {

This is actually pretty specifically designed - passing false for an isComponent parameter is a clear statement that "no, this is not a component." I would expect isComponent = false to override any other sources of component-ness.

Using null communicates "there's no information about whether this is a component", so the caller can either say "yes it's a component" or "idk if it's a component, you figure it out".

Sounds good. Can we copy'n'paste your comment into the code? :) Not a blocker though, feel free to ignore.

packages/compiler/src/render3/partial/api.ts

AndrewKushnir · 2022-04-19T19:01:07Z

packages/compiler/src/render3/partial/api.ts

+export interface R3DeclareNgModuleDependencyMetadata {
+  kind: 'ngmodule';
+
+  type: o.Expression|(() => o.Expression);


nit: may be copy the field description from the R3DeclarePipeDependencyMetadata above for completeness?

packages/compiler/src/render3/partial/component.ts

AndrewKushnir

Reviewed-for: size-tracking

pkozlowski-opensource

Going over this PR again - it LGTM. While my understanding of the compiler is limited I also don't see any "red flags" here.

Again, the breakdown into small, focused commits is great and helps a lot! Thnx @alxhub !

`PipeSymbol` contains logic to detect changes in the public API surface of pipes, which includes the pipe name. However, the pipe handler inadvertently uses the pipe class name instead of the actual pipe name to initialize the `PipeSymbol`, which breaks incremental compilation when pipe names change. There is a test which attempts to verify that this logic is working, but the test actually passes for a different reason. The test swaps the names of 2 pipes that are both used in a component, and asserts that the component is re-emitted, theoretically because the public APIs of the pipes is changed. However, the emit order of the references to the pipes depends on the order in which they match in the template, which changes when the names are swapped. This ordering dependency is picked up by the semantic dependency tracking system, and is what actually causes the component to be re-emitted and therefore the pipe test to pass in spite of the bug with name tracking. This commit fixes the `PipeSymbol` initialization to use the correct pipe name. The test is still flawed in that it's sensitive to the ordering of pipe emits, but this ordering is due to change soon as a result of the standalone components work, so this issue will be resolved in a future commit.

Previously, the compiler would represent template dependencies of a component in its component definition through separate fields (`directives`, `pipes`). This commit refactors the compiler/runtime interface to use a single field (`dependencies`). The runtime component definition object still has separate `directiveDefs` and `pipeDefs`, which are calculated from the `dependencies` when the definition is evaluated. This change is also reflected in partially compiled declarations. To ensure compatibility with partially compiled code already on NPM, the linker will still honor the old form of declaration (with separate fields).

Previously, the compiler tracked directives and pipes in template scopes separately. This commit refactors the scope system to unify them into a single data structure, disambiguated by a `kind` field.

…onents This commit expands on the unified dependency tracking in the previous commit and adds tracking of NgModule dependencies. These are not used for standard components, but are emitted for standalone components to allow the runtime to roll up providers from those NgModules into standalone injectors.

This commit adds an emit for standalone components of the `StandaloneFeature`, which will support creation of standalone injectors and any other mechanisms necessary for standalone component functionality at runtime. Using a feature allows for standalone functionality to be tree-shaken in applications that aren't using them.

Standalone component scopes were first implemented in the `ComponentDecoratorHandler` itself, due to an assumption that "standalone" allowed for a localized analysis of the component's dependencies. However, this is not strictly true. Other compiler machinery also needs to understand component scopes, including standalone component scopes. A good example is the template type-checking engine, which uses a `ComponentScopeReader` to build full metadata objects (that is, metadata that considers the entire inheritance chain) for type-checking purposes. Therefore, the `ComponentScopeReader` should be able to give the scope for a standalone component. To achieve this, a new `StandaloneComponentScopeReader` is implemented, and the return type of `ComponentScopeReader.getScopeForComponent` is expanded to express standalone scopes. This cleanly integrates the "standalone" concept into the existing machinery.

This commit propagates the `isStandalone` flag for a component, directive, or pipe during partial compilation of a standalone declaration. This flag allows the linker to properly process a standalone declaration that it encounters.

The ngcc integration test is in an awkward state: it's attempting to test that the current ngcc can process @angular/core at v12. We need to make a forwards-incompatible change to the typings of @angular/core, which means that the compiled typings from the current ngcc won't be compatible with core as of v12. To get around this and allow the integration test to have some value, we're disabling library checking for the time being.

This commit adds a type field to .d.ts metadata for directives, components, and pipes which carries a boolean literal indicating whether the given type is standalone or not. For backwards compatibility, this flag defaults to `false`. Tests are added to validate that standalone types coming from .d.ts files can be correctly imported into new standalone components.

This commit bundles tests for standalone components that are possible after previous implementation commits. Most new tests are compliance tests, but a test is also included to validate that the template type-checking system can work with standalone components as well.

alxhub · 2022-04-20T12:45:58Z

This PR was merged into the repository by commit b8d3389.

Previously, the compiler would represent template dependencies of a component in its component definition through separate fields (`directives`, `pipes`). This commit refactors the compiler/runtime interface to use a single field (`dependencies`). The runtime component definition object still has separate `directiveDefs` and `pipeDefs`, which are calculated from the `dependencies` when the definition is evaluated. This change is also reflected in partially compiled declarations. To ensure compatibility with partially compiled code already on NPM, the linker will still honor the old form of declaration (with separate fields). PR Close #45672

…5672) Previously, the compiler tracked directives and pipes in template scopes separately. This commit refactors the scope system to unify them into a single data structure, disambiguated by a `kind` field. PR Close #45672

…onents (#45672) This commit expands on the unified dependency tracking in the previous commit and adds tracking of NgModule dependencies. These are not used for standard components, but are emitted for standalone components to allow the runtime to roll up providers from those NgModules into standalone injectors. PR Close #45672

…nts (#45672) This commit adds an emit for standalone components of the `StandaloneFeature`, which will support creation of standalone injectors and any other mechanisms necessary for standalone component functionality at runtime. Using a feature allows for standalone functionality to be tree-shaken in applications that aren't using them. PR Close #45672

…ler (#45672) Standalone component scopes were first implemented in the `ComponentDecoratorHandler` itself, due to an assumption that "standalone" allowed for a localized analysis of the component's dependencies. However, this is not strictly true. Other compiler machinery also needs to understand component scopes, including standalone component scopes. A good example is the template type-checking engine, which uses a `ComponentScopeReader` to build full metadata objects (that is, metadata that considers the entire inheritance chain) for type-checking purposes. Therefore, the `ComponentScopeReader` should be able to give the scope for a standalone component. To achieve this, a new `StandaloneComponentScopeReader` is implemented, and the return type of `ComponentScopeReader.getScopeForComponent` is expanded to express standalone scopes. This cleanly integrates the "standalone" concept into the existing machinery. PR Close #45672

#45672) This commit propagates the `isStandalone` flag for a component, directive, or pipe during partial compilation of a standalone declaration. This flag allows the linker to properly process a standalone declaration that it encounters. PR Close #45672

The ngcc integration test is in an awkward state: it's attempting to test that the current ngcc can process @angular/core at v12. We need to make a forwards-incompatible change to the typings of @angular/core, which means that the compiled typings from the current ngcc won't be compatible with core as of v12. To get around this and allow the integration test to have some value, we're disabling library checking for the time being. PR Close #45672

…45672) This commit adds a type field to .d.ts metadata for directives, components, and pipes which carries a boolean literal indicating whether the given type is standalone or not. For backwards compatibility, this flag defaults to `false`. Tests are added to validate that standalone types coming from .d.ts files can be correctly imported into new standalone components. PR Close #45672

…45672) This commit bundles tests for standalone components that are possible after previous implementation commits. Most new tests are compliance tests, but a test is also included to validate that the template type-checking system can work with standalone components as well. PR Close #45672

angular-automatic-lock-bot · 2022-05-21T16:06:32Z

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.}

alxhub force-pushed the standalone-compiler branch from 1fa9814 to c472cea Compare April 18, 2022 19:40

alxhub added target: minor This PR is targeted for the next minor release area: core Issues related to the framework runtime labels Apr 18, 2022

ngbot bot added this to the Backlog milestone Apr 18, 2022

alxhub force-pushed the standalone-compiler branch 5 times, most recently from 68ae271 to 5832170 Compare April 19, 2022 14:19

pkozlowski-opensource reviewed Apr 19, 2022

View reviewed changes

packages/compiler/src/render3/partial/directive.ts Show resolved Hide resolved

pkozlowski-opensource reviewed Apr 19, 2022

View reviewed changes

packages/core/src/render3/interfaces/definition.ts Show resolved Hide resolved

pkozlowski-opensource reviewed Apr 19, 2022

View reviewed changes

packages/compiler-cli/src/ngtsc/annotations/component/src/handler.ts Outdated Show resolved Hide resolved

alxhub force-pushed the standalone-compiler branch 3 times, most recently from 1be2a1e to f8d4113 Compare April 19, 2022 17:11

pullapprove bot requested review from AndrewKushnir, atscott, dylhunn and jessicajaniuk April 19, 2022 17:11

atscott reviewed Apr 19, 2022

View reviewed changes

pullapprove bot requested a review from atscott April 19, 2022 17:16

JoostK approved these changes Apr 19, 2022

View reviewed changes

AndrewKushnir approved these changes Apr 19, 2022

View reviewed changes

pullapprove bot requested a review from AndrewKushnir April 19, 2022 19:17

AndrewKushnir approved these changes Apr 19, 2022

View reviewed changes

pkozlowski-opensource approved these changes Apr 19, 2022

View reviewed changes

alxhub force-pushed the standalone-compiler branch 3 times, most recently from 9f6d15a to 2b568dd Compare April 19, 2022 23:07

alxhub added the action: merge The PR is ready for merge by the caretaker label Apr 19, 2022

dylhunn added action: cleanup The PR is in need of cleanup, either due to needing a rebase or in response to comments from reviews and removed merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note labels Apr 20, 2022

alxhub added 10 commits April 20, 2022 04:14

refactor(compiler-cli): unify tracked template scope dependencies

8427f3e

Previously, the compiler tracked directives and pipes in template scopes separately. This commit refactors the scope system to unify them into a single data structure, disambiguated by a `kind` field.

alxhub force-pushed the standalone-compiler branch from b2c1f61 to 270dcfb Compare April 20, 2022 11:15

alxhub closed this in 046dad1 Apr 20, 2022

sync-by-unito bot mentioned this pull request May 1, 2022

build(deps-dev): bump @angular/language-service from 12.2.4 to 13.3.5 numbersprotocol/capture-lite#1575

Closed

angular-automatic-lock-bot bot locked and limited conversation to collaborators May 21, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Standalone Compiler Updates #45672

Standalone Compiler Updates #45672

alxhub commented Apr 18, 2022

atscott Apr 19, 2022

alxhub Apr 19, 2022

JoostK left a comment

JoostK Apr 19, 2022

alxhub Apr 19, 2022

JoostK Apr 19, 2022

alxhub Apr 19, 2022

JoostK Apr 19, 2022

alxhub Apr 19, 2022

JoostK Apr 19, 2022

JoostK Apr 19, 2022

JoostK Apr 19, 2022 •

edited

Loading

AndrewKushnir left a comment

AndrewKushnir Apr 19, 2022

alxhub Apr 19, 2022

AndrewKushnir Apr 19, 2022

AndrewKushnir Apr 19, 2022

AndrewKushnir left a comment

pkozlowski-opensource left a comment

alxhub commented Apr 20, 2022

angular-automatic-lock-bot bot commented May 21, 2022

-    if (!this.cache.has(clazz)) {
+    if (this.cache.has(clazz)) {
+      return this.cache.get(clazz)!;
+    }

		// A standalone component always has itself in scope, so add `clazzMeta` during
		// initialization.

	isComponent: true\|null = null): R3DirectiveDependencyMetadata {
	isComponent: boolean = false): R3DirectiveDependencyMetadata {

Standalone Compiler Updates #45672

Standalone Compiler Updates #45672

Conversation

alxhub commented Apr 18, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoostK left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

JoostK Apr 19, 2022 • edited Loading

Choose a reason for hiding this comment

AndrewKushnir left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

AndrewKushnir left a comment

Choose a reason for hiding this comment

pkozlowski-opensource left a comment

Choose a reason for hiding this comment

alxhub commented Apr 20, 2022

angular-automatic-lock-bot bot commented May 21, 2022

JoostK Apr 19, 2022 •

edited

Loading