Forbid orphan components #52061

pmvald · 2023-10-05T22:48:00Z

This PR adds an option to Angular compiler options which allow to produce runtime error if orphan components found. Good for identifying such cases.

PR Checklist

Please check if your PR fulfills the following requirements:

The commit message follows our guidelines: https://github.com/angular/angular/blob/main/CONTRIBUTING.md#commit
Tests for the changes have been added (for bug fixes / features)
Docs have been added / updated (for bug fixes / features)

PR Type

What kind of change does this PR introduce?

Bugfix
Feature
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Documentation content changes
angular.io application / infrastructure changes
Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

Does this PR introduce a breaking change?

Yes
No

Other information

devversion

Some initial comments.

packages/compiler-cli/src/ngtsc/core/api/src/public_options.ts

packages/compiler/src/render3/r3_class_debug_info_compiler.ts

packages/router/src/state_manager.ts

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

devversion · 2023-10-06T10:28:27Z

packages/core/src/render3/component_ref.ts

@@ -163,6 +164,17 @@ export class ComponentFactory<T> extends AbstractComponentFactory<T> {
      injector: Injector, projectableNodes?: any[][]|undefined, rootSelectorOrNode?: any,
      environmentInjector?: NgModuleRef<any>|EnvironmentInjector|
      undefined): AbstractComponentRef<T> {
+    // Check if the component is orphan
+    if (ngDevMode && this.componentDef.debugInfo?.forbidOrphanRendering) {
+      if (depsTracker.isOrphanComponent(this.componentType)) {


There is an interesting scenario we should chat about (related to our ngDevMode/jitMode conversations). If ngJitMode is false but ngDevMode is true, this will currently always yield an error IIRC because setNgModuleScope would never be invoked?

Ah yeah, correct. For now I added ngJitMode to this if statement. Also in the very first commit I added a check to ensure that the option forbidOrphanComponents is not set without jit mode (i.e., option supportJitMode). But these are just temporary solution, and it seems we need a better flag system. I'll write design doc soon on this. My quick thought is we may need a ngDebugMode flag as well which causes more verbose and runtime checks to pop in (unless we want to redefine ngDevMode to do this, but seems like this flag is a bit limited IIRC)

Sounds good. My first thought is that ngDebugMode should not exist and ngDevMode should be sufficient. We are not adding any super verbose logging/debugging as far as I know

packages/core/src/render3/component_ref.ts

…orphan component Orphan component is an anti-pattern in Angular where a component is rendered while the NgModule declaring it is not installed. It is not easy to capture this scenario, specially in compile time. But it is possible to capture a special case in runtime where the component is being rendered without its NgModule even loaded into the browser. This change adds a flag in cli compiler option to enable such checking, and throwing a runtime exception if it happens. Note that such check is only done in dev mode. Currently the check requires some generated code that is behind ngJitMode flag (i.e., call to ɵɵsetNgModuleScope), and the new flag can be set only if JIT mode is enabled (i.e., supportJitMode=true) otherwise an error will be thrown. The orphan component is a main blocker for rolling out local compilation in g3. This option is needed for identifying and isolating such cases.

…mponent's debug info A new flag added to the component's debug info to determine whether to throw runtime error (in dev mode) if component is being rendered without its NgModule. This flag is only set for non-standalone components.

The flag `forbidOrphanRendering` is only set for non-standalone components, and indicates that the dev mode runtime should through error if the component is rendered without its ngModule loaded in the browser. This runtime error can help with further debugging.

… orphan A new method `isOrphanComponent` is added to the deps tracker API to check if the NgModule declaring this component, if exists, is loaded into the browser.

A runtime error will be thrown if a non-standalone component is being rendered without its NgModule loaded in the browser. This error is thrown only in dev mode and only if the Angular option `forbidOrphanComponents` is set to true. The error contains useful info to find the orphan component in the source code.

Now users can configure the option `forbidOrphanComponents` in the tsconfig's angularCompilerOptions part.

pmvald · 2023-10-10T12:19:54Z

Caretaker note: the ci/test failed due to //packages/core/test/bundling/defer:symbol_test which has been flaky these days. But it is fixed this morning and once rebased, this PR should pass that as well.

atscott · 2023-10-10T22:30:29Z

This PR was merged into the repository by commit 59ba2a6.

…mponent's debug info (#52061) A new flag added to the component's debug info to determine whether to throw runtime error (in dev mode) if component is being rendered without its NgModule. This flag is only set for non-standalone components. PR Close #52061

#52061) The flag `forbidOrphanRendering` is only set for non-standalone components, and indicates that the dev mode runtime should through error if the component is rendered without its ngModule loaded in the browser. This runtime error can help with further debugging. PR Close #52061

… orphan (#52061) A new method `isOrphanComponent` is added to the deps tracker API to check if the NgModule declaring this component, if exists, is loaded into the browser. PR Close #52061

A runtime error will be thrown if a non-standalone component is being rendered without its NgModule loaded in the browser. This error is thrown only in dev mode and only if the Angular option `forbidOrphanComponents` is set to true. The error contains useful info to find the orphan component in the source code. PR Close #52061

Now users can configure the option `forbidOrphanComponents` in the tsconfig's angularCompilerOptions part. PR Close #52061

angular-automatic-lock-bot · 2023-11-11T00:09:34Z

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

…orphan component (angular#52061) Orphan component is an anti-pattern in Angular where a component is rendered while the NgModule declaring it is not installed. It is not easy to capture this scenario, specially in compile time. But it is possible to capture a special case in runtime where the component is being rendered without its NgModule even loaded into the browser. This change adds a flag in cli compiler option to enable such checking, and throwing a runtime exception if it happens. Note that such check is only done in dev mode. Currently the check requires some generated code that is behind ngJitMode flag (i.e., call to ɵɵsetNgModuleScope), and the new flag can be set only if JIT mode is enabled (i.e., supportJitMode=true) otherwise an error will be thrown. The orphan component is a main blocker for rolling out local compilation in g3. This option is needed for identifying and isolating such cases. PR Close angular#52061

…mponent's debug info (angular#52061) A new flag added to the component's debug info to determine whether to throw runtime error (in dev mode) if component is being rendered without its NgModule. This flag is only set for non-standalone components. PR Close angular#52061

angular#52061) The flag `forbidOrphanRendering` is only set for non-standalone components, and indicates that the dev mode runtime should through error if the component is rendered without its ngModule loaded in the browser. This runtime error can help with further debugging. PR Close angular#52061

… orphan (angular#52061) A new method `isOrphanComponent` is added to the deps tracker API to check if the NgModule declaring this component, if exists, is loaded into the browser. PR Close angular#52061

…r#52061) A runtime error will be thrown if a non-standalone component is being rendered without its NgModule loaded in the browser. This error is thrown only in dev mode and only if the Angular option `forbidOrphanComponents` is set to true. The error contains useful info to find the orphan component in the source code. PR Close angular#52061

…ar#52061) Now users can configure the option `forbidOrphanComponents` in the tsconfig's angularCompilerOptions part. PR Close angular#52061

angular-robot bot added the detected: feature PR contains a feature commit label Oct 5, 2023

pmvald force-pushed the forbid-orphan-comp branch from 3d8d6ae to 38cdb2d Compare October 6, 2023 02:07

pmvald changed the title ~~Forbid orphan comp~~ Forbid orphan components Oct 6, 2023

pmvald force-pushed the forbid-orphan-comp branch from 38cdb2d to d7af0f9 Compare October 6, 2023 03:32

ngbot bot modified the milestone: Backlog Oct 6, 2023

pmvald requested review from devversion and crisbeto October 6, 2023 04:08

pmvald marked this pull request as ready for review October 6, 2023 04:08

devversion reviewed Oct 6, 2023

View reviewed changes

pmvald force-pushed the forbid-orphan-comp branch 2 times, most recently from 9114f48 to a539f4a Compare October 8, 2023 21:15

pmvald requested a review from devversion October 8, 2023 22:06

devversion approved these changes Oct 9, 2023

View reviewed changes

pmvald added 6 commits October 9, 2023 20:41

refactor(core): add an API to deps tracker to check if a component is…

d4050af

… orphan A new method `isOrphanComponent` is added to the deps tracker API to check if the NgModule declaring this component, if exists, is loaded into the browser.

feat(bazel): make forbidOrphanComponents option configurable

ee81a04

Now users can configure the option `forbidOrphanComponents` in the tsconfig's angularCompilerOptions part.

pmvald force-pushed the forbid-orphan-comp branch from a539f4a to ee81a04 Compare October 10, 2023 00:43

crisbeto approved these changes Oct 10, 2023

View reviewed changes

pmvald added action: merge The PR is ready for merge by the caretaker and removed action: review The PR is still awaiting reviews from at least one requested reviewer labels Oct 10, 2023

pmvald added the merge: caretaker note Alert the caretaker performing the merge to check the PR for an out of normal action needed or note label Oct 10, 2023

atscott closed this in e8201a5 Oct 10, 2023

atscott pushed a commit that referenced this pull request Oct 10, 2023

feat(bazel): make forbidOrphanComponents option configurable (#52061)

59ba2a6

Now users can configure the option `forbidOrphanComponents` in the tsconfig's angularCompilerOptions part. PR Close #52061

angular-automatic-lock-bot bot locked and limited conversation to collaborators Nov 11, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Forbid orphan components #52061

Forbid orphan components #52061

pmvald commented Oct 5, 2023 •

edited

devversion left a comment

devversion Oct 6, 2023

pmvald Oct 8, 2023

devversion Oct 9, 2023

pmvald commented Oct 10, 2023

atscott commented Oct 10, 2023

angular-automatic-lock-bot bot commented Nov 11, 2023

Forbid orphan components #52061

Forbid orphan components #52061

Conversation

pmvald commented Oct 5, 2023 • edited

PR Checklist

PR Type

What is the current behavior?

What is the new behavior?

Does this PR introduce a breaking change?

Other information

devversion left a comment

Choose a reason for hiding this comment

devversion Oct 6, 2023

Choose a reason for hiding this comment

pmvald Oct 8, 2023

Choose a reason for hiding this comment

devversion Oct 9, 2023

Choose a reason for hiding this comment

pmvald commented Oct 10, 2023

atscott commented Oct 10, 2023

angular-automatic-lock-bot bot commented Nov 11, 2023

pmvald commented Oct 5, 2023 •

edited