Fix issues with cycle detection #46029
Closed
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The Angular compiler performs cycle detection when generating imports within component files. This was previously necessary as reifying dependencies discovered via NgModules into the component output could add imports that weren't present in the original component and potentially create cycles. Doing this could cause order-of-execution issues with existing user imports, so the compiler detects this case and falls back to an alternative way of specifying component dependencies that doesn't risk creating cycles. For standalone components, Angular does not need to add new imports to the component file as the user has already explicitly referenced dependencies in the `@Component.imports`. As a result, the cycle detection can be skipped. Correctly authoring a program with import cycles is always challenging. One side of a cyclic import will always initially evaluate to `undefined`, and this can result in errors in the component definition when this happens within component `imports`. Our compiler _could_ detect the cycle and choose to wrap the component dependencies in an automatic closure instead, avoiding any issues with `undefined` during an eager evaluation. However, this commit makes an active choice not to do that as it only serves to mask the problems with cyclic imports. Future refactorings may cause the "other half" of the cycle to break. Users should instead be aware of the potential problems with cycles and explicitly defer evaluations with `forwardRef` where needed. This ensures that future implementations of Angular compilation which may not be able to automatically detect import cycles and correct accordingly can still compile such components.
This commit fixes a small issue in the logic around the calculation of template scopes for standalone components. These scopes include a `Reference` for each dependency of a standalone component, which is used to generate references to that dependency in various contexts. Previously, the `Reference` used for a dependency was the one generated from its own metadata. For example, a referenced directive used the `Reference` that was created when analyzing the directive declaration itself. This still works, as the compiler is always able to emit a reference to any valid `Reference`. However, it's not optimal. The `Reference` which should be used instead is the one generated from analyzing the standalone component's `imports` array, which has knowledge of how the dependency is referenced from within the standalone component's file itself. This allows the compiler to skip creating a new import for the dependency when emitting the standalone component, and use the existing, user-authored import instead. This saves on code size and avoids taxing the bundler with unnecessary imports.
alxhub
force-pushed
the
standalone-fix-cycle-detector
branch
from
276f02e
to
d044569
Compare
alxhub
added
target: rc
atscott
approved these changes
May 17, 2022
|
This PR was merged into the repository by commit 1e07a11. |
jessicajaniuk
pushed a commit
that referenced
this pull request
May 18, 2022
The Angular compiler performs cycle detection when generating imports within component files. This was previously necessary as reifying dependencies discovered via NgModules into the component output could add imports that weren't present in the original component and potentially create cycles. Doing this could cause order-of-execution issues with existing user imports, so the compiler detects this case and falls back to an alternative way of specifying component dependencies that doesn't risk creating cycles. For standalone components, Angular does not need to add new imports to the component file as the user has already explicitly referenced dependencies in the `@Component.imports`. As a result, the cycle detection can be skipped. Correctly authoring a program with import cycles is always challenging. One side of a cyclic import will always initially evaluate to `undefined`, and this can result in errors in the component definition when this happens within component `imports`. Our compiler _could_ detect the cycle and choose to wrap the component dependencies in an automatic closure instead, avoiding any issues with `undefined` during an eager evaluation. However, this commit makes an active choice not to do that as it only serves to mask the problems with cyclic imports. Future refactorings may cause the "other half" of the cycle to break. Users should instead be aware of the potential problems with cycles and explicitly defer evaluations with `forwardRef` where needed. This ensures that future implementations of Angular compilation which may not be able to automatically detect import cycles and correct accordingly can still compile such components. PR Close #46029
jessicajaniuk
pushed a commit
that referenced
this pull request
May 18, 2022
…46029) This commit fixes a small issue in the logic around the calculation of template scopes for standalone components. These scopes include a `Reference` for each dependency of a standalone component, which is used to generate references to that dependency in various contexts. Previously, the `Reference` used for a dependency was the one generated from its own metadata. For example, a referenced directive used the `Reference` that was created when analyzing the directive declaration itself. This still works, as the compiler is always able to emit a reference to any valid `Reference`. However, it's not optimal. The `Reference` which should be used instead is the one generated from analyzing the standalone component's `imports` array, which has knowledge of how the dependency is referenced from within the standalone component's file itself. This allows the compiler to skip creating a new import for the dependency when emitting the standalone component, and use the existing, user-authored import instead. This saves on code size and avoids taxing the bundler with unnecessary imports. PR Close #46029
jessicajaniuk
pushed a commit
that referenced
this pull request
May 18, 2022
…46029) This commit fixes a small issue in the logic around the calculation of template scopes for standalone components. These scopes include a `Reference` for each dependency of a standalone component, which is used to generate references to that dependency in various contexts. Previously, the `Reference` used for a dependency was the one generated from its own metadata. For example, a referenced directive used the `Reference` that was created when analyzing the directive declaration itself. This still works, as the compiler is always able to emit a reference to any valid `Reference`. However, it's not optimal. The `Reference` which should be used instead is the one generated from analyzing the standalone component's `imports` array, which has knowledge of how the dependency is referenced from within the standalone component's file itself. This allows the compiler to skip creating a new import for the dependency when emitting the standalone component, and use the existing, user-authored import instead. This saves on code size and avoids taxing the bundler with unnecessary imports. PR Close #46029
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.