Skip to content

Commit bee2463

Browse files
authored
refactor: replace NodeSelector with Document in preparation for Typescript upgrade (#1065)
Updating from Typescript 2.9 to 3.5 results in a variety of errors that look like this: ``` ERROR in Q:/repos/accessibility-insights-web/src/common/document-manipulator.ts(4,42): TS2304: Cannot find name 'NodeSelector'. ``` It looks like microsoft/TypeScript-DOM-lib-generator#647 was the point where it was removed, merged into a separate `ParentNode` interface. We considered using `ParentNode` instead here, but `Document` seemed closer to the intended usage in most cases and led to the tests working more similarly to the product code than anything else, so that's what I went with. Replacing the usages with Document is a no-op in Typescript 2.9 (so nice to do as a separate PR, to make the eventual Typescript upgrade PR smaller). It also forces the affected tests to be a little bit better (they create test documents for the cases where the product passes real documents, rather than creating test nodes within a global document and hoping that they act similarly and that no other test touches global document state).
1 parent cbf992b commit bee2463

22 files changed

+185
-232
lines changed

src/DetailsView/details-view-renderer.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ import { PreviewFeatureFlagsHandler } from './handlers/preview-feature-flags-han
1919

2020
export class DetailsViewRenderer {
2121
private renderer: typeof ReactDOM.render;
22-
private dom: NodeSelector & Node;
22+
private dom: Document;
2323
private scopingActionMessageCreator: ScopingActionMessageCreator;
2424
private inspectActionMessageCreator: InspectActionMessageCreator;
2525
private issuesSelection: ISelection;
@@ -34,7 +34,7 @@ export class DetailsViewRenderer {
3434

3535
constructor(
3636
private readonly deps: DetailsViewContainerDeps,
37-
dom: NodeSelector & Node,
37+
dom: Document,
3838
renderer: typeof ReactDOM.render,
3939
scopingActionMessageCreator: ScopingActionMessageCreator,
4040
inspectActionMessageCreator: InspectActionMessageCreator,

src/common/document-manipulator.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
33
export class DocumentManipulator {
4-
constructor(private document: Node & NodeSelector) {}
4+
constructor(private document: Document) {}
55

66
public setShortcutIcon(href: string): void {
77
const icon = this.document.querySelector('head link[rel="shortcut icon"]');

src/injected/visualization/base-drawer.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { DrawerUtils } from './drawer-utils';
88
import { Formatter } from './formatter';
99

1010
export abstract class BaseDrawer implements Drawer {
11-
protected dom: NodeSelector & Node;
11+
protected dom: Document;
1212
protected formatter: Formatter;
1313
protected isEnabled = false;
1414
protected containerClass: string;
@@ -23,7 +23,7 @@ export abstract class BaseDrawer implements Drawer {
2323
private shadowUtils: ShadowUtils;
2424

2525
constructor(
26-
dom: NodeSelector & Node,
26+
dom: Document,
2727
containerClass: string,
2828
windowUtils: WindowUtils,
2929
shadowUtils: ShadowUtils,

src/injected/visualization/drawer-utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Licensed under the MIT License.
33
import { ClientRectOffset } from '../client-utils';
44
export class DrawerUtils {
5-
private dom: NodeSelector & Node;
5+
private dom: Document;
66
public clientWindowOffsetThreshold: number = 5;
77

88
constructor(dom) {
@@ -18,7 +18,7 @@ export class DrawerUtils {
1818
}
1919

2020
public getDocumentElement(): Document {
21-
return this.dom.ownerDocument || (this.dom as Document);
21+
return this.dom.ownerDocument || this.dom;
2222
}
2323

2424
public getContainerWidth(

src/injected/visualization/highlight-box-drawer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,7 @@ export class HighlightBoxDrawer extends BaseDrawer {
3131
};
3232

3333
constructor(
34-
dom: NodeSelector & Node,
34+
dom: Document,
3535
containerClass: string,
3636
windowUtils: WindowUtils,
3737
shadowUtils: ShadowUtils,

src/injected/visualization/svg-drawer.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ export class SVGDrawer extends BaseDrawer {
2626
private centerPositionCalculator: CenterPositionCalculator;
2727

2828
constructor(
29-
dom: NodeSelector & Node,
29+
dom: Document,
3030
containerClass: string,
3131
windowUtils: WindowUtils,
3232
shadowUtils: ShadowUtils,

src/popup/components/diagnostic-view-toggle-factory.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,11 +22,11 @@ export class DiagnosticViewToggleFactory {
2222
private commandStore: BaseStore<CommandStoreData>;
2323
private actionMessageCreator: PopupActionMessageCreator;
2424
private clickHandler: DiagnosticViewClickHandler;
25-
private dom: NodeSelector & Node;
25+
private dom: Document;
2626

2727
constructor(
2828
private readonly deps: DiagnosticViewToggleDeps,
29-
dom: NodeSelector & Node,
29+
dom: Document,
3030
visualizationTypes: VisualizationType[],
3131
visualizationConfigurationFactory: VisualizationConfigurationFactory,
3232
visualizationStore: BaseStore<VisualizationStoreData>,

src/popup/components/diagnostic-view-toggle.tsx

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ export interface DiagnosticViewToggleProps {
2727
clickHandler: DiagnosticViewClickHandler;
2828
shortcutCommands: chrome.commands.Command[];
2929
telemetrySource: TelemetryEventSource;
30-
dom: NodeSelector & Node;
30+
dom: Document;
3131
}
3232

3333
export type DiagnosticViewToggleDeps = ContentLinkDeps;
@@ -39,7 +39,7 @@ export interface DiagnosticViewToggleState {
3939
export class DiagnosticViewToggle extends React.Component<DiagnosticViewToggleProps, DiagnosticViewToggleState> {
4040
private configuration: VisualizationConfiguration;
4141
private toggle: React.RefObject<IToggle> = React.createRef<IToggle>();
42-
private dom: NodeSelector & Node;
42+
private dom: Document;
4343
private userEventListenerAdded: boolean;
4444

4545
// Must be consistent with internal react naming for same property to work

src/popup/incompatible-browser-renderer.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@ import { NewTabLink } from '../common/components/new-tab-link';
88
import { Header } from './components/header';
99

1010
export class IncompatibleBrowserRenderer {
11-
constructor(private readonly renderer: typeof ReactDOM.render, private readonly dom: NodeSelector & Node) {}
11+
constructor(private readonly renderer: typeof ReactDOM.render, private readonly dom: Document) {}
1212

1313
public render(): void {
1414
const container = this.dom.querySelector('#popup-container');

src/popup/main-renderer.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ export class MainRenderer {
1919
private readonly deps: MainRendererDeps,
2020
private readonly popupHandlers: IPopupHandlers,
2121
private readonly renderer: typeof ReactDOM.render,
22-
private readonly dom: NodeSelector & Node,
22+
private readonly dom: Document,
2323
private readonly popupWindow: Window,
2424
private readonly targetTabUrl: string,
2525
private readonly hasAccess: boolean,

src/scanner/axe-options.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ export interface AxeOptions {
66
runOnly?: Axe.RunOnly;
77
restoreScroll?: Boolean; // missing from axe options.
88
}
9-
export type AxeScanContext = string | NodeSelector & Node | IncludeExcludeOptions | NodeList;
9+
export type AxeScanContext = string | Document | IncludeExcludeOptions | NodeList;
1010
export interface IncludeExcludeOptions {
1111
include: string[][];
1212
exclude: string[][];

src/scanner/launcher.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export class Launcher {
1010
constructor(
1111
private axe: typeof Axe,
1212
private scanParameterGenerator: ScanParameterGenerator,
13-
private dom: NodeSelector & Node,
13+
private dom: Document,
1414
private options: ScanOptions,
1515
) {}
1616

src/scanner/scan-options.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22
// Licensed under the MIT License.
33
export interface ScanOptions {
44
testsToRun?: string[];
5-
dom?: NodeSelector & Node | NodeList;
5+
dom?: Document | NodeList;
66
selector?: string;
77
include?: string[][];
88
exclude?: string[][];

src/scanner/scan-parameter-generator.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ export class ScanParameterGenerator {
2424
return result;
2525
}
2626

27-
public getContext(dom: NodeSelector & Node, options: ScanOptions): AxeScanContext {
27+
public getContext(dom: Document, options: ScanOptions): AxeScanContext {
2828
if (options == null) {
2929
return dom;
3030
}
Lines changed: 5 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,10 @@
11
// Copyright (c) Microsoft Corporation. All rights reserved.
22
// Licensed under the MIT License.
3+
import { JSDOM } from 'jsdom';
4+
35
export class TestDocumentCreator {
4-
public static createTestDocument(html: string): Node & NodeSelector {
5-
const element = document.createElement('html');
6-
element.innerHTML = html;
7-
return element;
6+
public static createTestDocument(html: string = ''): Document {
7+
const jsdom = new JSDOM(`<html>${html}</html>`);
8+
return jsdom.window.document;
89
}
910
}

src/tests/unit/tests/DetailsView/details-view-renderer.test.tsx

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import { AssessmentInstanceTableHandler } from '../../../../DetailsView/handlers
1919
import { DetailsViewToggleClickHandlerFactory } from '../../../../DetailsView/handlers/details-view-toggle-click-handler-factory';
2020
import { PreviewFeatureFlagsHandler } from '../../../../DetailsView/handlers/preview-feature-flags-handler';
2121
import { CreateTestAssessmentProvider } from '../../common/test-assessment-provider';
22+
import { TestDocumentCreator } from '../../common/test-document-creator';
2223

2324
describe('DetailsViewRendererTest', () => {
2425
test('render', () => {
@@ -27,8 +28,7 @@ describe('DetailsViewRendererTest', () => {
2728
const scopingActionMessageCreatorStrictMock = Mock.ofType<ScopingActionMessageCreator>(null, MockBehavior.Strict);
2829
const inspectActionMessageCreatorStrictMock = Mock.ofType<InspectActionMessageCreator>(null, MockBehavior.Strict);
2930

30-
const dom = document.createElement('div');
31-
const detailsViewContainer = document.createElement('div');
31+
const fakeDocument = TestDocumentCreator.createTestDocument('<div id="details-container"></div>');
3232

3333
const renderMock: IMock<typeof ReactDOM.render> = Mock.ofInstance(() => null);
3434
const selectionMock = Mock.ofType<ISelection>(Selection);
@@ -46,9 +46,6 @@ describe('DetailsViewRendererTest', () => {
4646
const documentManipulatorMock = Mock.ofType(DocumentManipulator);
4747
documentManipulatorMock.setup(des => des.setShortcutIcon('../' + expectedIcon16)).verifiable();
4848

49-
detailsViewContainer.setAttribute('id', 'details-container');
50-
dom.appendChild(detailsViewContainer);
51-
5249
renderMock
5350
.setup(r =>
5451
r(
@@ -71,14 +68,14 @@ describe('DetailsViewRendererTest', () => {
7168
/>
7269
</>,
7370
),
74-
detailsViewContainer,
71+
fakeDocument.getElementById('details-container'),
7572
),
7673
)
7774
.verifiable();
7875

7976
const renderer = new DetailsViewRenderer(
8077
deps,
81-
dom,
78+
fakeDocument,
8279
renderMock.object,
8380
scopingActionMessageCreatorStrictMock.object,
8481
inspectActionMessageCreatorStrictMock.object,

0 commit comments

Comments
 (0)