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-24T18:53:52Z

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:04:24Z

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:02:10Z

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

…ular#44973) 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. PR Close angular#44973

…es (angular#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 angular#44973

…ngular#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 angular#44973

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

…ngular#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 angular#44973

…4973) 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 angular#44973

angular-automatic-lock-bot · 2022-04-29T16:08:10Z

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

atscott self-requested a review February 4, 2022 17:24

pkozlowski-opensource self-requested a review February 4, 2022 17:26

pkozlowski-opensource reviewed Feb 4, 2022

View reviewed changes

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

pkozlowski-opensource reviewed Feb 4, 2022

View reviewed changes

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

dylhunn added the area: compiler Issues related to `ngc`, Angular's template compiler label Feb 4, 2022

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

alxhub added target: major This PR is targeted for the next major release and removed area: compiler Issues related to `ngc`, Angular's template compiler labels Feb 4, 2022

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

atscott previously requested changes Feb 7, 2022

View reviewed changes

alxhub added the area: compiler Issues related to `ngc`, Angular's template 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 March 23, 2022 19:26

alxhub marked this pull request as ready for review March 24, 2022 17:07

JoostK reviewed Mar 24, 2022

View reviewed changes

alxhub added 4 commits March 28, 2022 12:39

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 March 28, 2022 20:00

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

ahasall reviewed Mar 29, 2022

View reviewed 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 March 29, 2022 17:49

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 March 29, 2022 17:52

JoostK approved these changes Mar 29, 2022

View reviewed changes

pkozlowski-opensource approved these changes Mar 29, 2022

View reviewed changes

alxhub added target: minor This PR is targeted for the next minor release action: merge The PR is ready for merge by the caretaker and removed target: major This PR is targeted for the next major release labels Mar 29, 2022

dylhunn closed this in f027bfb Mar 29, 2022

dylhunn pushed a commit that referenced this pull request 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-automatic-lock-bot bot locked and limited conversation to collaborators Apr 29, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

angular-automatic-lock-bot bot commented Apr 29, 2022

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

Part 2 of Standalone Components #44973

Part 2 of Standalone Components #44973

Conversation

alxhub commented Feb 4, 2022

atscott left a comment

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

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

Choose a reason for hiding this comment

dylhunn commented Mar 29, 2022

angular-automatic-lock-bot bot commented Apr 29, 2022