Skip to content

Commit

Permalink
chore(null): null strictness fixes for 9/15's top 10 null:find results (
Browse files Browse the repository at this point in the history
#3359)

#### Description of changes

Another round of the top ten `yarn null:find` results, plus the extra dependent files that `yarn null:autoadd` revealed were fixed as a side effect of the first fixes.

The most interesting/notable change here is in the error handling improvements to `target-tab-finder.ts`; previously, a few edge cases the browser could return to us would result in a string error or a "cannot reference property url of undefined" error, where now they will throw more meaningful `Error` objects. Tests were updated to accomodate.

Before:
```sh
> yarn null:progress

**43%** complete (**573**/1343 non-test files)
```

After:
```sh
> yarn null:progress

**45%** complete (**599**/1343 non-test files)
```

#### Pull request checklist
<!-- If a checklist item is not applicable to this change, write "n/a" in the checkbox -->
- [x] Addresses an existing issue: #2869
- [x] Ran `yarn fastpass`
- [x] Added/updated relevant unit test(s) (and ran `yarn test`)
- [x] Verified code coverage for the changes made. Check coverage report at: `<rootDir>/test-results/unit/coverage`
- [x] PR title *AND* final merge commit title both start with a semantic tag (`fix:`, `chore:`, `feat(feature-name):`, `refactor:`). See `CONTRIBUTING.md`.
- [n/a] (UI changes only) Added screenshots/GIFs to description above
- [n/a] (UI changes only) Verified usability with NVDA/JAWS
  • Loading branch information
dbjorge authored Sep 15, 2020
1 parent 37ae844 commit 13c728b
Show file tree
Hide file tree
Showing 12 changed files with 75 additions and 28 deletions.
2 changes: 1 addition & 1 deletion src/common/components/header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ export type HeaderProps = {
navMenu?: JSX.Element;
showHeaderTitle?: boolean;
showFarItems?: boolean;
narrowModeStatus?: NarrowModeStatus;
narrowModeStatus: NarrowModeStatus;
};

export const Header = NamedFC<HeaderProps>('Header', props => {
Expand Down
5 changes: 4 additions & 1 deletion src/common/url-validator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,10 @@ import { BrowserAdapter } from './browser-adapters/browser-adapter';
export class UrlValidator {
constructor(private readonly browserAdapter: BrowserAdapter) {}

public async isSupportedUrl(url: string): Promise<boolean> {
public async isSupportedUrl(url?: string): Promise<boolean> {
if (url == null) {
return false;
}
const lowerCasedUrl: string = url.toLowerCase();
if (lowerCasedUrl.startsWith('http://') || lowerCasedUrl.startsWith('https://')) {
return this.hasSupportedPrefix(lowerCasedUrl);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,5 +13,5 @@ export class AndroidSetupActionCreator {
public saveAdbPath = (newAdbPath: string) => this.setupActions.saveAdbPath.invoke(newAdbPath);
public setSelectedDevice = (device: DeviceInfo) =>
this.setupActions.setSelectedDevice.invoke(device);
public readyToStart = () => this.setupActions.readyToStart.invoke(null, this.scope);
public readyToStart = () => this.setupActions.readyToStart.invoke(undefined, this.scope);
}
6 changes: 3 additions & 3 deletions src/electron/ipc/ipc-renderer-shim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,15 +45,15 @@ export class IpcRendererShim {
}

private onMaximize = (): void => {
this.fromBrowserWindowMaximize.invoke(null, this.invokeScope);
this.fromBrowserWindowMaximize.invoke(undefined, this.invokeScope);
};

private onEnterFullScreen = (): void => {
this.fromBrowserWindowEnterFullScreen.invoke(null, this.invokeScope);
this.fromBrowserWindowEnterFullScreen.invoke(undefined, this.invokeScope);
};

private onUnmaximize = (): void => {
this.fromBrowserWindowUnmaximize.invoke(null, this.invokeScope);
this.fromBrowserWindowUnmaximize.invoke(undefined, this.invokeScope);
};

private onClose = async (): Promise<void> => {
Expand Down
9 changes: 7 additions & 2 deletions src/popup/target-tab-finder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,12 @@ export class TargetTabFinder {
if (tabIdInUrl == null) {
return this.browserAdapter
.tabsQuery({ active: true, currentWindow: true })
.then(tabs => tabs.pop());
.then(tabs => {
if (tabs.length === 0) {
throw new Error('No active tabs found for current window');
}
return tabs.pop()!;
});
} else {
return new Promise((resolve, reject) => {
this.browserAdapter.getTab(
Expand All @@ -38,7 +43,7 @@ export class TargetTabFinder {
resolve(tab);
},
() => {
reject(`Tab with Id ${tabIdInUrl} not found`);
reject(new Error(`Tab with Id ${tabIdInUrl} not found`));
},
);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const addConfirmOnClickHandler = function (
doc: Document,
confirmCallback: ConfirmType,
): void {
const targetPageLink = doc.getElementById(linkId);
const targetPageLink = doc.getElementById(linkId)!;
targetPageLink.addEventListener('click', function (event): void {
const result = confirmCallback(
'Are you sure you want to navigate away from the Accessibility Insights report?\n' +
Expand Down
2 changes: 1 addition & 1 deletion src/scanner/custom-rules/native-widgets-default.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ export function evaluateNativeWidget(node: HTMLElement): boolean {
return true;
}

export function getNativeWidgetElementType(node: HTMLElement): string {
export function getNativeWidgetElementType(node: HTMLElement): string | undefined {
if (node.tagName === 'BUTTON' || node.tagName === 'SELECT' || node.tagName === 'TEXTAREA') {
return node.tagName.toLowerCase();
} else if (node.tagName === 'INPUT' && node.hasAttribute('list')) {
Expand Down
11 changes: 9 additions & 2 deletions src/tests/unit/tests/common/components/header.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,18 @@ import { Header, HeaderDeps } from 'common/components/header';
import { NarrowModeStatus } from 'DetailsView/components/narrow-mode-detector';

describe('Header', () => {
const stubNarrowModeStatus = {
isHeaderAndNavCollapsed: false,
} as NarrowModeStatus;

it('renders per snapshot', () => {
const applicationTitle = 'THE_APPLICATION_TITLE';
const deps = {
textContent: {
applicationTitle,
},
} as HeaderDeps;
const wrapper = shallow(<Header deps={deps} />);
const wrapper = shallow(<Header deps={deps} narrowModeStatus={stubNarrowModeStatus} />);
expect(wrapper.getElement()).toMatchSnapshot();
});

Expand All @@ -25,7 +29,9 @@ describe('Header', () => {
applicationTitle,
},
} as HeaderDeps;
const wrapper = shallow(<Header deps={deps} showHeaderTitle={false} />);
const wrapper = shallow(
<Header deps={deps} showHeaderTitle={false} narrowModeStatus={stubNarrowModeStatus} />,
);
expect(wrapper.getElement()).toMatchSnapshot();
});

Expand All @@ -41,6 +47,7 @@ describe('Header', () => {
deps={deps}
farItems={<div>THis is far items!</div>}
showFarItems={showFarItems}
narrowModeStatus={stubNarrowModeStatus}
/>,
);
expect(wrapper.getElement()).toMatchSnapshot();
Expand Down
2 changes: 2 additions & 0 deletions src/tests/unit/tests/common/url-validator.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,8 @@ describe('UrlValidatorTest', () => {
['http://domain-with-no-trailing-slash', true],
['http:/oops', false],
['oops_http://example.com', false],
[null, false],
[undefined, false],
];

beforeEach(() => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ describe(AndroidSetupActionCreator, () => {
});

it('invokes readyToStart action on readyToStart', () => {
const readyToStartMock = createActionMock<void>(null, 'AndroidSetupActionCreator');
const readyToStartMock = createActionMock<void>(undefined, 'AndroidSetupActionCreator');
androidSetupActionsMock
.setup(actions => actions.readyToStart)
.returns(() => readyToStartMock.object);
Expand Down
28 changes: 18 additions & 10 deletions src/tests/unit/tests/popup/target-tab-finder.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ describe('TargetTabFinderTest', () => {
let urlParserMock: IMock<UrlParser>;
let urlValidatorMock: IMock<UrlValidator>;
const tabId: number = 15;
let tabStub: Tab;
let tabStub: Tabs.Tab;

beforeEach(() => {
windowStub = {
Expand All @@ -28,7 +28,7 @@ describe('TargetTabFinderTest', () => {
id: tabId,
title: 'some title',
url: 'target page url',
};
} as Tabs.Tab;

browserAdapterMock = Mock.ofType<BrowserAdapter>();
urlParserMock = Mock.ofType(UrlParser);
Expand Down Expand Up @@ -72,7 +72,7 @@ describe('TargetTabFinderTest', () => {
setupGetTabCall();
} else {
setupGetTabIdParamFromUrl(null);
setupTabQueryCall();
setupTabQueryCall([tabStub]);
}

setupIsSupportedCall(testCase.isUrlSupported);
Expand All @@ -84,17 +84,25 @@ describe('TargetTabFinderTest', () => {
});
});

test('no tab found', async () => {
it('throws an error if tabId URL parameter is not found', async () => {
setupGetTabIdParamFromUrl(tabId);
browserAdapterMock
.setup(b => b.getTab(tabId, It.isAny(), It.isAny()))
.callback((id, cb, reject) => {
reject();
});
setupIsSupportedCall(true);
await testSubject
.getTargetTab()
.catch(error => expect(error).toEqual(`Tab with Id ${tabId} not found`));

await expect(testSubject.getTargetTab()).rejects.toThrowError(
`Tab with Id ${tabId} not found`,
);
});

it('throws an error there is neither a tabId URL parameter nor an active tab', async () => {
setupTabQueryCall([]);

await expect(testSubject.getTargetTab()).rejects.toThrowError(
'No active tabs found for current window',
);
});

function setupGetTabIdParamFromUrl(tabIdValue: number | null): void {
Expand All @@ -111,10 +119,10 @@ describe('TargetTabFinderTest', () => {
});
}

function setupTabQueryCall(): void {
function setupTabQueryCall(returnValue: Tabs.Tab[]): void {
browserAdapterMock
.setup(adapter => adapter.tabsQuery({ active: true, currentWindow: true }))
.returns(() => Promise.resolve([tabStub as Tabs.Tab]));
.returns(() => Promise.resolve(returnValue));
}

function setupIsSupportedCall(isSupported: boolean): void {
Expand Down
32 changes: 27 additions & 5 deletions tsconfig.strictNullChecks.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
"./src/DetailsView/components/left-nav/test-view-left-nav-link.tsx",
"./src/DetailsView/components/narrow-mode-detector.tsx",
"./src/DetailsView/components/nav-link-button.tsx",
"./src/DetailsView/components/no-content-available/no-content-available-view.tsx",
"./src/DetailsView/components/no-content-available/no-content-available.tsx",
"./src/DetailsView/components/no-displayable-preview-features-message.tsx",
"./src/DetailsView/components/overview-content/overview-heading.tsx",
Expand Down Expand Up @@ -189,6 +190,7 @@
"./src/debug-tools/components/telemetry-viewer/telemetry-viewer.tsx",
"./src/electron/common/electron-telemetry-events.ts",
"./src/electron/common/get-electron-icon-path.ts",
"./src/electron/flux/action-creator/android-setup-action-creator.ts",
"./src/electron/flux/action-creator/scan-action-creator.ts",
"./src/electron/flux/action-creator/window-frame-action-creator.ts",
"./src/electron/flux/store/scan-store.ts",
Expand All @@ -198,10 +200,6 @@
"./src/electron/flux/types/scan-store-data.ts",
"./src/electron/flux/types/window-state-store-data.ts",
"./src/electron/flux/types/window-state.ts",
"./src/electron/ipc/async-action.ts",
"./src/electron/ipc/ipc-channel-names.ts",
"./src/electron/ipc/ipc-message-dispatcher.ts",
"./src/electron/ipc/ipc-message-receiver.ts",
"./src/electron/main/main-window-config.ts",
"./src/electron/main/main-window-renderer-message-handlers.ts",
"./src/electron/platform/android/adb-wrapper.ts",
Expand All @@ -210,10 +208,13 @@
"./src/electron/platform/android/appium-adb-wrapper.ts",
"./src/electron/platform/android/device-config.ts",
"./src/electron/platform/android/live-appium-adb-creator.ts",
"./src/electron/platform/android/setup/android-port-cleaner.ts",
"./src/electron/platform/android/setup/android-service-configurator-factory.ts",
"./src/electron/platform/android/setup/android-service-configurator.ts",
"./src/electron/platform/android/setup/android-setup-deps.ts",
"./src/electron/platform/android/setup/android-setup-step-id.ts",
"./src/electron/platform/android/setup/port-cleaning-service-configurator-factory.ts",
"./src/electron/platform/android/setup/port-cleaning-service-configurator.ts",
"./src/electron/platform/android/setup/state-machine/state-machine-action-callback.ts",
"./src/electron/platform/android/setup/state-machine/state-machine-step-configs.ts",
"./src/electron/platform/android/setup/state-machine/state-machine-step.ts",
Expand All @@ -224,10 +225,21 @@
"./src/electron/views/automated-checks/components/title-bar.tsx",
"./src/electron/views/bundled-renderer-styles.ts",
"./src/electron/views/common/window-title/window-title.tsx",
"./src/electron/views/device-connect-view/components/android-setup/android-setup-spinner-step.tsx",
"./src/electron/views/device-connect-view/components/android-setup/android-setup-spinner.tsx",
"./src/electron/views/device-connect-view/components/android-setup/android-setup-step-container.tsx",
"./src/electron/views/device-connect-view/components/android-setup/android-setup-step-layout.tsx",
"./src/electron/views/device-connect-view/components/android-setup/android-setup-types.ts",
"./src/electron/views/device-connect-view/components/android-setup/configuring-port-forwarding-step.tsx",
"./src/electron/views/device-connect-view/components/android-setup/detect-adb-step.tsx",
"./src/electron/views/device-connect-view/components/android-setup/detect-devices-step.tsx",
"./src/electron/views/device-connect-view/components/android-setup/detect-permissions-step.tsx",
"./src/electron/views/device-connect-view/components/android-setup/detect-service-step.tsx",
"./src/electron/views/device-connect-view/components/android-setup/device-description.tsx",
"./src/electron/views/device-connect-view/components/android-setup/folder-picker.tsx",
"./src/electron/views/device-connect-view/components/android-setup/installing-service-step.tsx",
"./src/electron/views/device-connect-view/components/android-setup/prompt-locate-adb-step.tsx",
"./src/electron/views/device-connect-view/components/android-setup/wait-to-start-step.tsx",
"./src/electron/views/device-connect-view/components/automation-ids.ts",
"./src/electron/views/device-connect-view/components/electron-external-link.tsx",
"./src/electron/views/device-connect-view/components/electron-link.tsx",
Expand All @@ -239,6 +251,7 @@
"./src/electron/views/root-container/components/platform-body-class-modifier.tsx",
"./src/electron/views/screenshot/screenshot.tsx",
"./src/electron/window-management/platform-info.ts",
"./src/electron/window-management/window-frame-updater.ts",
"./src/injected/adapters/resolution-creator.ts",
"./src/injected/analyzers/filter-results.ts",
"./src/injected/client-utils.ts",
Expand Down Expand Up @@ -269,6 +282,7 @@
"./src/popup/components/header.tsx",
"./src/popup/components/launch-pad-item-row.tsx",
"./src/popup/incompatible-browser-renderer.tsx",
"./src/popup/target-tab-finder.ts",
"./src/reports/assessment-report.styles.ts",
"./src/reports/automated-checks-report.styles.ts",
"./src/reports/components/assessment-report-body-header.tsx",
Expand All @@ -277,6 +291,7 @@
"./src/reports/components/header-bar.tsx",
"./src/reports/components/inline-image.tsx",
"./src/reports/components/instance-outcome-type.ts",
"./src/reports/components/new-tab-link-confirmation-dialog.tsx",
"./src/reports/components/report-head.tsx",
"./src/reports/components/report-sections/body-section.tsx",
"./src/reports/components/report-sections/collapsible-script-provider.tsx",
Expand All @@ -285,6 +300,7 @@
"./src/reports/components/report-sections/no-failed-instances-congrats.tsx",
"./src/reports/components/report-sections/report-footer.tsx",
"./src/reports/components/report-sections/summary-report-header-section.tsx",
"./src/reports/components/report-sections/summary-results-table.tsx",
"./src/reports/components/report-sections/title-section.tsx",
"./src/reports/components/report-sections/tool-link.tsx",
"./src/reports/components/summary-report-head.tsx",
Expand All @@ -307,9 +323,11 @@
"./src/scanner/custom-rules/header-rule.ts",
"./src/scanner/custom-rules/heading-rule.ts",
"./src/scanner/custom-rules/link-purpose.ts",
"./src/scanner/custom-rules/native-widgets-default.ts",
"./src/scanner/custom-rules/page-title.ts",
"./src/scanner/custom-rules/text-contrast.ts",
"./src/scanner/custom-rules/unique-landmark.ts",
"./src/scanner/custom-rules/widget-function.ts",
"./src/scanner/document-utils.ts",
"./src/scanner/help-url-getter.ts",
"./src/scanner/iruleresults.d.ts",
Expand Down Expand Up @@ -338,6 +356,7 @@
"./src/tests/end-to-end/common/playwright-option-types.ts",
"./src/tests/end-to-end/common/prepare-test-result-file-path.ts",
"./src/tests/end-to-end/common/pretty-print-axe-violations.ts",
"./src/tests/end-to-end/common/scan-for-accessibility-issues.ts",
"./src/tests/end-to-end/common/screenshot-on-error.ts",
"./src/tests/end-to-end/common/test-resources.ts",
"./src/tests/end-to-end/common/timeouts.ts",
Expand Down Expand Up @@ -365,7 +384,9 @@
"./src/tests/unit/tests/injected/visualization/drawer-utils-mock-builder.ts",
"./src/tests/unit/tests/scanner/custom-rules-configuration-stub.ts",
"./src/tests/unit/tests/scanner/mock-axe-utils.ts",
"./src/views/content/guidance-title.tsx"
"./src/views/content/content-view.tsx",
"./src/views/content/guidance-title.tsx",
"./src/views/page/page.tsx"
],
"include": [
"src/Devtools/**/*",
Expand All @@ -392,6 +413,7 @@
"src/electron/auto-update/**/*",
"src/electron/electron-builder/**/*",
"src/electron/flux/action/**/*",
"src/electron/ipc/**/*",
"src/electron/resources/**/*",
"src/icons/**/*",
"src/issue-filing/common/markup/**/*",
Expand Down

0 comments on commit 13c728b

Please sign in to comment.