Skip to content

Commit 95bb62c

Browse files
authored
Add proper TypeScript coverage for window-navigation.ts (#5559)
This removes the @ts-nocheck at the top. I used `Location` instead of `Partial<Location>` in the hopes that this would catch missing properties, but it doesn't actually seem to. Not sure why.
2 parents 30a5a27 + 47af69b commit 95bb62c

File tree

2 files changed

+29
-25
lines changed

2 files changed

+29
-25
lines changed

src/test/fixtures/mocks/window-navigation.ts

Lines changed: 28 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
/* This Source Code Form is subject to the terms of the Mozilla Public
22
* License, v. 2.0. If a copy of the MPL was not distributed with this
33
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
4-
// @ts-nocheck Complex window/history mock with extensive DOM API manipulation that requires intricate typing
54

65
/**
76
* jsdom leaves the history in place after every test, so the history will
@@ -41,10 +40,19 @@ import { coerceMatchingShape } from '../../../utils/types';
4140
// window.history can change the inner location directly.
4241
const internalLocationAssign = Symbol.for('internalLocationAssign');
4342

43+
type LocationMock = Location & {
44+
assign: (newUrl: string) => void;
45+
[internalLocationAssign]: (newUrl: string) => void;
46+
};
47+
4448
// This symbol will be used in the mock for window.history so that we can reset
4549
// it from tests.
4650
const internalHistoryReset = Symbol.for('internalHistoryReset');
4751

52+
type HistoryMock = History & {
53+
[internalHistoryReset]: () => void;
54+
};
55+
4856
/**
4957
* This mock creates a location API that allows for assigning to the location,
5058
* which we need to be able to do for certain tests.
@@ -59,15 +67,17 @@ function mockWindowLocation(location: string = 'http://localhost') {
5967
url = new URL(newUrl.toString(), url);
6068
}
6169

62-
const nativeLocation = Object.getOwnPropertyDescriptor(window, 'location');
70+
const nativeLocation = Object.getOwnPropertyDescriptor(window, 'location')!;
6371

6472
// It seems node v8 doesn't let us change the value unless we delete it before.
73+
// @ts-expect-error - property must be optional
6574
delete window.location;
6675

6776
const property = {
68-
get(): Partial<Location> {
77+
get(): LocationMock {
6978
return {
7079
toString: () => url.toString(),
80+
// @ts-expect-error - array is not a fully-featured DOMStringList
7181
ancestorOrigins: [],
7282
get href() {
7383
return url.toString();
@@ -134,7 +144,6 @@ function mockWindowLocation(location: string = 'http://localhost') {
134144
newUrl.hash = v;
135145
this.assign(newUrl.toString());
136146
},
137-
// $FlowExpectError Flow doesn't know about symbol properties sadly.
138147
[internalLocationAssign]: internalSetLocation,
139148
assign: (newUrl: string) => window.history.pushState(null, '', newUrl),
140149
reload: jest.fn(),
@@ -148,15 +157,14 @@ function mockWindowLocation(location: string = 'http://localhost') {
148157
},
149158
};
150159

151-
// $FlowExpectError because the value we pass isn't a proper Location object.
152160
Object.defineProperty(window, 'location', property);
153161

154162
// Return a function that resets the mock.
155163
return () => {
156164
// This "delete" call doesn't seem to be necessary, but better do it so that
157165
// we don't have surprises in the future.
166+
// @ts-expect-error - property not optional
158167
delete window.location;
159-
// $FlowExpectError because nativeLocation doesn't match the type expected by Flow.
160168
Object.defineProperty(window, 'location', nativeLocation);
161169
};
162170
}
@@ -165,9 +173,11 @@ function mockWindowLocation(location: string = 'http://localhost') {
165173
* This mock creates a history API that can be thrown away after every use.
166174
*/
167175
function mockWindowHistory() {
168-
const originalHistory = Object.getOwnPropertyDescriptor(window, 'history');
176+
const originalHistory = Object.getOwnPropertyDescriptor(window, 'history')!;
169177

170-
let states, urls, index;
178+
let states = [null];
179+
let urls = [window.location.href];
180+
let index = 0;
171181

172182
function reset() {
173183
states = [null];
@@ -181,7 +191,7 @@ function mockWindowHistory() {
181191
get length() {
182192
return states.length;
183193
},
184-
scrollRestoration: 'auto',
194+
scrollRestoration: 'auto' as const,
185195
get state() {
186196
return states[index] ?? null;
187197
},
@@ -190,8 +200,7 @@ function mockWindowHistory() {
190200
return;
191201
}
192202
index--;
193-
// $FlowExpectError Flow doesn't know about this internal property.
194-
window.location[internalLocationAssign](urls[index]);
203+
(window.location as LocationMock)[internalLocationAssign](urls[index]);
195204
window.dispatchEvent(new Event('popstate'));
196205
},
197206
forward() {
@@ -200,8 +209,7 @@ function mockWindowHistory() {
200209
}
201210
index++;
202211

203-
// $FlowExpectError Flow doesn't know about this internal property.
204-
window.location[internalLocationAssign](urls[index]);
212+
(window.location as LocationMock)[internalLocationAssign](urls[index]);
205213
window.dispatchEvent(new Event('popstate'));
206214
},
207215
go() {
@@ -212,8 +220,7 @@ function mockWindowHistory() {
212220
// Let's assign the URL to the window.location mock. This should also
213221
// make the URL correct if it's relative, we'll get an absolute URL when
214222
// retrieving later through window.location.href.
215-
// $FlowExpectError Flow doesn't know about this internal property.
216-
window.location[internalLocationAssign](url);
223+
(window.location as LocationMock)[internalLocationAssign](url);
217224
}
218225

219226
urls = urls.slice(0, index + 1);
@@ -226,19 +233,18 @@ function mockWindowHistory() {
226233
replaceState(newState: any, _title: string, url?: string) {
227234
if (url) {
228235
// Let's assign the URL to the window.location mock.
229-
// $FlowExpectError Flow doesn't know about this internal property.
230-
window.location[internalLocationAssign](url);
236+
(window.location as LocationMock)[internalLocationAssign](url);
231237
urls[index] = window.location.href;
232238
}
233239

234240
states[index] = newState;
235241
},
236-
// $FlowExpectError Flow doesn't know about symbol properties sadly.
237242
[internalHistoryReset]: reset,
238243
};
239244

240245
// This "delete" call doesn't seem to be necessary, but better do it so that
241246
// we don't have surprises in the future.
247+
// @ts-expect-error - property not optional
242248
delete window.history;
243249
Object.defineProperty(window, 'history', {
244250
value: coerceMatchingShape<History>(history),
@@ -250,8 +256,8 @@ function mockWindowHistory() {
250256
// For unknown reasons, we can't assign back the old descriptor without
251257
// deleting the current one first... Not deleting would keep the mock
252258
// without throwing any error.
259+
// @ts-expect-error - property not optional
253260
delete window.history;
254-
// $FlowExpectError - Flow can't handle getOwnPropertyDescriptor being used on defineProperty.
255261
Object.defineProperty(window, 'history', originalHistory);
256262
};
257263
}
@@ -274,7 +280,7 @@ export function mockFullNavigation({
274280
// window.history for each test. Take a look at the top of this file for more
275281
// information about how to use this.
276282
export function autoMockFullNavigation() {
277-
let cleanup;
283+
let cleanup: ReturnType<typeof mockFullNavigation> | null = null;
278284
beforeEach(() => {
279285
cleanup = mockFullNavigation();
280286
});
@@ -288,8 +294,6 @@ export function autoMockFullNavigation() {
288294
}
289295

290296
export function resetHistoryWithUrl(url: string = window.location.href) {
291-
// $FlowExpectError Flow doesn't know about this internal property.
292-
window.location[internalLocationAssign](url);
293-
// $FlowExpectError Flow doesn't know about this internal property.
294-
window.history[internalHistoryReset]();
297+
(window.location as LocationMock)[internalLocationAssign](url);
298+
(window.history as HistoryMock)[internalHistoryReset]();
295299
}

src/utils/types.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -113,7 +113,7 @@ export function coerce<A, B>(item: A): B {
113113
* It can be helpful to coerce one type that matches the shape of another.
114114
*/
115115
export function coerceMatchingShape<T>(item: Partial<T>): T {
116-
return item as any;
116+
return item as T;
117117
}
118118

119119
/**

0 commit comments

Comments
 (0)