fix(compiler-cli): report when NgModule imports or exports itself #58231
Conversation
| if (decl.node === ref.node) { | ||
| diagnostics.push( | ||
| makeDiagnostic( | ||
| ErrorCode.NGMODULE_INVALID_IMPORT, |
There was a problem hiding this comment.
I was on the fence whether to introduce a new error code or reuse the existing one.
There was a problem hiding this comment.
I think having just one code is fine.
f8c7388 to
9608a03
Compare
There was a problem hiding this comment.
I was contemplating whether situations like the following would work correctly:
A > B > C
^---/
as then the cycle would still infinitely recurse when lookupRoot is A, but a quick test didn't show this failure mode.
We could avoid that potential problem by inserting a sentinel value into the cache prior to recursing, such that any cycle can be detected by asserting that the cache entry is not set to the cycle sentinel.
| if (decl.node === ref.node) { | ||
| diagnostics.push( | ||
| makeDiagnostic( | ||
| ErrorCode.NGMODULE_INVALID_IMPORT, |
There was a problem hiding this comment.
I think having just one code is fine.
|
I don't quite follow the |
No, just C depending on B. @NgModule({ imports: [forwardRef(() => ModB)] })
class ModC {}
@NgModule({ imports: [ModC] })
class ModB {}
@NgModule({ imports: [ModB] })
class ModA {}(I haven't attempted with the exact code as above, only with NgModules across different modules). The problem won't be observed if the scope of either So perhaps a repro might be (haven't tested): @NgModule({ imports: [forwardRef(() => ModB)] })
class ModA {}
@NgModule({ imports: [forwardRef(() => ModC)] })
class ModB {}
@NgModule({ imports: [ModB] })
class ModC {} |
Reports a diagnostic if an NgModule refers to itself in its `imports` or `exports`. Previously the compiler would go into an infinite loop. Fixes angular#58224.
9608a03 to
071df45
Compare
|
Ah, well spotted 👍 I've pushed a fixup commit. |
|
This PR was merged into the repository by commit d0c74f3. The changes were merged into the following branches: main, 18.2.x |
|
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. |
Reports a diagnostic if an NgModule refers to itself in its
importsorexports. Previously the compiler would go into an infinite loop.Fixes #58224.