Skip to content

Commit e4f7463

Browse files
Tony RossDevtools-frontend LUCI CQ
Tony Ross
authored and
Devtools-frontend LUCI CQ
committed
Remove circular dependency between IssuesManager and SourceFrameIssuesManager
This change extracts the IssuesManager Events enum into a separate file to break the circular dependency of values between IssuesManager.ts and SourceFrameIssuesManager.ts. This helps reduce existing circular dependencies, in theory moving us slightly closer to being able to warn about such cycles at build-time to avoid unexpected errors. This particular cycle recently led to a runtime error when attempting to subclass IssuesManager to introduce additional functionality. This subclass was included in the cycle as SourceFrameIssuesManager was updated to import the subclass instead of IssuesManager. At the time the Events enum for IssuesManager was non-const, causing the circular dependency to exist post-build. The error occurred because the extends relationship of the subclass was evaluated during module initialization. Since then the Events enum has been converted to const which eliminates the runtime error. However, eliminating even this cycle is still beneficial. The compile-time only relationship is not obvious and cannot otherwise be communicated explicitly until values from const enums can be used with `import type` per microsoft/TypeScript#40344. Bug: 1229328 Change-Id: I033080c77f22584aa0527bcd0559698341d200f5 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3040178 Commit-Queue: Tony Ross <[email protected]> Reviewed-by: Sigurd Schneider <[email protected]> Reviewed-by: Tim van der Lippe <[email protected]>
1 parent feb3b63 commit e4f7463

File tree

5 files changed

+20
-11
lines changed

5 files changed

+20
-11
lines changed

config/gni/devtools_grd_files.gni

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -698,6 +698,7 @@ grd_files_debug_sources = [
698698
"front_end/models/issues_manager/Issue.js",
699699
"front_end/models/issues_manager/IssueResolver.js",
700700
"front_end/models/issues_manager/IssuesManager.js",
701+
"front_end/models/issues_manager/IssuesManagerEvents.js",
701702
"front_end/models/issues_manager/LowTextContrastIssue.js",
702703
"front_end/models/issues_manager/MarkdownIssueDescription.js",
703704
"front_end/models/issues_manager/MixedContentIssue.js",

front_end/models/issues_manager/BUILD.gn

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ devtools_module("issues_manager") {
2020
"Issue.ts",
2121
"IssueResolver.ts",
2222
"IssuesManager.ts",
23+
"IssuesManagerEvents.ts",
2324
"LowTextContrastIssue.ts",
2425
"MarkdownIssueDescription.ts",
2526
"MixedContentIssue.ts",

front_end/models/issues_manager/IssuesManager.ts

Lines changed: 3 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import {CrossOriginEmbedderPolicyIssue, isCrossOriginEmbedderPolicyIssue} from '
1212
import {DeprecationIssue} from './DeprecationIssue.js';
1313
import {HeavyAdIssue} from './HeavyAdIssue.js';
1414
import type {Issue, IssueKind} from './Issue.js';
15+
import {Events} from './IssuesManagerEvents.js';
1516
import {LowTextContrastIssue} from './LowTextContrastIssue.js';
1617
import {MixedContentIssue} from './MixedContentIssue.js';
1718
import {QuirksModeIssue} from './QuirksModeIssue.js';
@@ -22,6 +23,8 @@ import {TrustedWebActivityIssue} from './TrustedWebActivityIssue.js';
2223
import {AttributionReportingIssue} from './AttributionReportingIssue.js';
2324
import {WasmCrossOriginModuleSharingIssue} from './WasmCrossOriginModuleSharingIssue.js';
2425

26+
export {Events} from './IssuesManagerEvents.js';
27+
2528
let issuesManagerInstance: IssuesManager|null = null;
2629

2730

@@ -288,12 +291,6 @@ export class IssuesManager extends Common.ObjectWrapper.ObjectWrapper<EventTypes
288291
}
289292
}
290293

291-
export const enum Events {
292-
IssuesCountUpdated = 'IssuesCountUpdated',
293-
IssueAdded = 'IssueAdded',
294-
FullUpdateRequired = 'FullUpdateRequired',
295-
}
296-
297294
export interface IssueAddedEvent {
298295
issuesModel: SDK.IssuesModel.IssuesModel;
299296
issue: Issue;
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
// Copyright 2021 The Chromium Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
export const enum Events {
6+
IssuesCountUpdated = 'IssuesCountUpdated',
7+
IssueAdded = 'IssueAdded',
8+
FullUpdateRequired = 'FullUpdateRequired',
9+
}

front_end/models/issues_manager/SourceFrameIssuesManager.ts

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,20 @@ import * as Workspace from '../../models/workspace/workspace.js';
1111
import {ContentSecurityPolicyIssue, trustedTypesPolicyViolationCode, trustedTypesSinkViolationCode} from './ContentSecurityPolicyIssue.js';
1212
import type {Issue, IssueKind} from './Issue.js';
1313
import {toZeroBasedLocation} from './Issue.js';
14-
import * as IssuesManager from './IssuesManager.js';
14+
import type {IssueAddedEvent, IssuesManager} from './IssuesManager.js';
15+
import {Events} from './IssuesManagerEvents.js';
1516
import {getIssueTitleFromMarkdownDescription} from './MarkdownIssueDescription.js';
1617

1718
export class SourceFrameIssuesManager {
1819
private locationPool = new Bindings.LiveLocation.LiveLocationPool();
1920
private issueMessages = new Array<IssueMessage>();
2021

21-
constructor(private readonly issuesManager: IssuesManager.IssuesManager) {
22-
this.issuesManager.addEventListener(IssuesManager.Events.IssueAdded, this.onIssueAdded, this);
23-
this.issuesManager.addEventListener(IssuesManager.Events.FullUpdateRequired, this.onFullUpdateRequired, this);
22+
constructor(private readonly issuesManager: IssuesManager) {
23+
this.issuesManager.addEventListener(Events.IssueAdded, this.onIssueAdded, this);
24+
this.issuesManager.addEventListener(Events.FullUpdateRequired, this.onFullUpdateRequired, this);
2425
}
2526

26-
private onIssueAdded(event: Common.EventTarget.EventTargetEvent<IssuesManager.IssueAddedEvent>): void {
27+
private onIssueAdded(event: Common.EventTarget.EventTargetEvent<IssueAddedEvent>): void {
2728
const {issue} = event.data;
2829
this.addIssue(issue);
2930
}

0 commit comments

Comments
 (0)