Part 2 of Standalone Components #44973

alxhub · 2022-02-04T16:38:24Z

Part 1 implemented the standalone flag in components/directives/pipes, and imports for components. In this PR, NgModules gain support for importing standalone types.

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

atscott

As mentioned by Pawel, this could generally use more test coverage for the various diagnostics and types of standalone entities.

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

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

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

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

packages/compiler-cli/src/ngtsc/testing/src/utils.ts

Tests added!

JoostK · 2022-03-24T19:13:14Z

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

+        if (errors[0].relatedInformation !== undefined) {
+          messageText += errors[0].relatedInformation.map(info => info.messageText).join('\n');
+        }


This looks like it could use ts.flattenDiagnosticMessageText

I don't think so, because relatedInformation is not the same as using a DiagnosticMessageChain for messageText. But I might be misunderstanding the TS APIs here?

Oh, you are correct. It could be used within the .map(info => ...) transform here to avoid an [object object] string in the case of a message chain, but we can adjust once we need it (since this is just testing code).

JoostK · 2022-03-24T19:13:14Z

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

+        const dirMeta = this.fullReader.getDirectiveMetadata(decl);
+        const pipeMeta = this.fullReader.getPipeMetadata(decl);
+        if (dirMeta !== null || pipeMeta !== null) {
+          const isStandalone = (dirMeta?.isStandalone || pipeMeta?.isStandalone)!;


nit: a ?? would be slightly more appropriate here, also allowing to remove one ?. in favor of !.

Suggested change

const isStandalone = (dirMeta?.isStandalone || pipeMeta?.isStandalone)!;

const isStandalone = dirMeta?.isStandalone ?? pipeMeta!.isStandalone;

I think this is too magical actually (both the original and the suggested) - I've changed it to be more explicit.

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

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

packages/core/src/render3/definition.ts

Before the `SemanticSymbol` system which now powers incremental compilation, the compiler previously needed to track which NgModules contributed to the scope of a component in order to recompile correctly if something changed. This commit removes that legacy field (which had no consumers) as well as the logic to populate it.

This commit improves the error messages generated by the compiler when NgModule scope analysis finds structural issues within a compilation. In particular, errors are now shown on a node within the metadata of the NgModule which produced the error, as opposed to the node of the erroneous declaration/import/ export. For example, if an NgModule declares `declarations: [FooCmp]` and `FooCmp` is not annotated as a directive, component, or pipe, the error is now shown on the reference to `FooCmp` in the `declarations` array expression. Previously, the error would have been shown on `FooCmp` itself, with a mention in the error text of the NgModule name. Additional error context in some cases has been moved to related information attached to the diagnostic, which further improves the legibility of such errors. Error text has also been adjusted to be more succinct, since more info about the error is now delivered through context.

This commit moves the error for declaring a standalone directive/component/ pipe to the `LocalModuleScopeRegistry`. Previously the error was produced by the `NgModuleHandler` directly. Producing the error in the scope registry allows the scope to be marked as poisoned when the error occurs, preventing spurious downstream errors from surfacing.

This just helps organize the standalone tests a little bit.

This commit implements the next step of Angular's "standalone" functionality, by allowing directives/components/pipes declared as `standalone` to be imported into NgModules. Errors are raised when such a type is not standalone but is included in an NgModule's imports.

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

This commit carries the `standalone` flag forward from a directive/pipe into its generated directive/pipe definition, allowing the runtime to recognize standalone entities.

JoostK · 2022-03-29T18:04:14Z

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

+        if (errors[0].relatedInformation !== undefined) {
+          messageText += errors[0].relatedInformation.map(info => info.messageText).join('\n');
+        }


Oh, you are correct. It could be used within the .map(info => ...) transform here to avoid an [object object] string in the case of a message chain, but we can adjust once we need it (since this is just testing code).

pkozlowski-opensource

LGTM

dylhunn · 2022-03-29T20:06:28Z

This PR was merged into the repository by commit 2142ffd.

…es (#44973) This commit improves the error messages generated by the compiler when NgModule scope analysis finds structural issues within a compilation. In particular, errors are now shown on a node within the metadata of the NgModule which produced the error, as opposed to the node of the erroneous declaration/import/ export. For example, if an NgModule declares `declarations: [FooCmp]` and `FooCmp` is not annotated as a directive, component, or pipe, the error is now shown on the reference to `FooCmp` in the `declarations` array expression. Previously, the error would have been shown on `FooCmp` itself, with a mention in the error text of the NgModule name. Additional error context in some cases has been moved to related information attached to the diagnostic, which further improves the legibility of such errors. Error text has also been adjusted to be more succinct, since more info about the error is now delivered through context. PR Close #44973

…44973) This commit moves the error for declaring a standalone directive/component/ pipe to the `LocalModuleScopeRegistry`. Previously the error was produced by the `NgModuleHandler` directly. Producing the error in the scope registry allows the scope to be marked as poisoned when the error occurs, preventing spurious downstream errors from surfacing. PR Close #44973

…sts (#44973) This just helps organize the standalone tests a little bit. PR Close #44973

…44973) This commit implements the next step of Angular's "standalone" functionality, by allowing directives/components/pipes declared as `standalone` to be imported into NgModules. Errors are raised when such a type is not standalone but is included in an NgModule's imports. PR Close #44973

This commit carries the `standalone` flag forward from a directive/pipe into its generated directive/pipe definition, allowing the runtime to recognize standalone entities. PR Close #44973

atscott self-requested a review Feb 4, 2022

pkozlowski-opensource self-requested a review Feb 4, 2022

pkozlowski-opensource reviewed Feb 4, 2022

View changes

packages/compiler-cli/test/ngtsc/standalone_spec.ts Outdated Show resolved Hide resolved

pkozlowski-opensource reviewed Feb 4, 2022

View changes

packages/compiler-cli/test/ngtsc/standalone_spec.ts Show resolved Hide resolved

dylhunn added the comp: compiler label Feb 4, 2022

ngbot bot added this to the Backlog milestone Feb 4, 2022

alxhub added target: major and removed comp: compiler labels Feb 4, 2022

ngbot bot removed this from the Backlog milestone Feb 4, 2022

atscott previously requested changes Feb 7, 2022

View changes

alxhub added the comp: compiler label Feb 17, 2022

ngbot bot added this to the Backlog milestone Feb 17, 2022

alxhub force-pushed the ngtsc/sac/impl.2 branch 7 times, most recently from 9e0bdbb to f054987 Compare Mar 23, 2022

alxhub marked this pull request as ready for review Mar 24, 2022

JoostK reviewed Mar 24, 2022

View changes

alxhub added 4 commits Mar 28, 2022

test(compiler-cli): split standalone tests into component & module tests

c0aaf09

This just helps organize the standalone tests a little bit.

alxhub force-pushed the ngtsc/sac/impl.2 branch from f054987 to e781cff Compare Mar 28, 2022

alxhub force-pushed the ngtsc/sac/impl.2 branch from e781cff to f3235b1 Compare Mar 28, 2022

ahasall reviewed Mar 29, 2022

View changes

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

alxhub force-pushed the ngtsc/sac/impl.2 branch from f3235b1 to c236a25 Compare Mar 29, 2022

feat(compiler-cli): propagate standalone flag to runtime

8dcff2a

This commit carries the `standalone` flag forward from a directive/pipe into its generated directive/pipe definition, allowing the runtime to recognize standalone entities.

alxhub force-pushed the ngtsc/sac/impl.2 branch from c236a25 to 8dcff2a Compare Mar 29, 2022

JoostK approved these changes Mar 29, 2022

View changes

pkozlowski-opensource approved these changes Mar 29, 2022

View changes

alxhub added target: minor action: merge and removed target: major labels Mar 29, 2022

dylhunn closed this in angular/angular@f027bfb Mar 29, 2022

dylhunn added a commit that referenced this issue Mar 29, 2022

test(compiler-cli): split standalone tests into component & module te…

459766b

…sts (#44973) This just helps organize the standalone tests a little bit. PR Close #44973

angular / angular Public

Part 2 of Standalone Components #44973

Part 2 of Standalone Components #44973

alxhub commented Feb 4, 2022

atscott left a comment

JoostK Mar 24, 2022

alxhub Mar 28, 2022

JoostK Mar 29, 2022

JoostK Mar 24, 2022

alxhub Mar 28, 2022

JoostK Mar 29, 2022

pkozlowski-opensource left a comment

dylhunn commented Mar 29, 2022

	const isStandalone = (dirMeta?.isStandalone \|\| pipeMeta?.isStandalone)!;
	const isStandalone = dirMeta?.isStandalone ?? pipeMeta!.isStandalone;

angular / angular Public

Part 2 of Standalone Components #44973

Part 2 of Standalone Components #44973

Conversation

alxhub commented Feb 4, 2022

atscott left a comment

JoostK Mar 24, 2022

Choose a reason for hiding this comment

alxhub Mar 28, 2022

Choose a reason for hiding this comment

JoostK Mar 29, 2022

Choose a reason for hiding this comment

JoostK Mar 24, 2022

Choose a reason for hiding this comment

alxhub Mar 28, 2022

Choose a reason for hiding this comment

JoostK Mar 29, 2022

Choose a reason for hiding this comment

pkozlowski-opensource left a comment

dylhunn commented Mar 29, 2022