Skip to content

Commit e2789c1

Browse files
authored
chore: upgrade jest and jsdom (#10453)
This lets us remove a polyfill and some other hacks, and brings along some additional jsdom features which will facilitate testing fixes to submitter serialization. Note: * jsdom is making it harder to stub things, so we inject a jest-friendly window into the router. I'm not a huge fan of DI when used solely for the sake of testing, but this seems less terrible than other approaches (e.g. (patch-package-ing jsdom, or monkey-patching Object.defineProperty) 🙃 * We use a yarn resolution to get the latest jsdom, despite the latest jest pinning it at ^20.0.0. Per the maintainers, this is fine as jest doesn't need any code changes to work with new jsdom, and there are no concrete plans yet to ship a major version.
1 parent 3e6628c commit e2789c1

File tree

11 files changed

+900
-774
lines changed

11 files changed

+900
-774
lines changed

.changeset/stale-birds-travel.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
"@remix-run/router": patch
3+
---
4+
5+
upgrade jest and jsdom

package.json

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,11 +32,15 @@
3232
"jest": {
3333
"projects": [
3434
"<rootDir>/packages/*"
35+
],
36+
"reporters": [
37+
"default"
3538
]
3639
},
3740
"resolutions": {
3841
"@types/react": "^18.0.0",
39-
"@types/react-dom": "^18.0.0"
42+
"@types/react-dom": "^18.0.0",
43+
"jsdom": "22.0.0"
4044
},
4145
"dependencies": {
4246
"@ampproject/filesize": "^4.3.0",

packages/react-router-dom-v5-compat/jest.config.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,4 @@ module.exports = {
2121
"^react-router$": "<rootDir>/../react-router/index.ts",
2222
"^react-router-dom-v5-compat$": "<rootDir>/index.ts",
2323
},
24-
reporters: ["default"],
2524
};

packages/react-router-dom/__tests__/polyfills/SubmitEvent.submitter.ts

Lines changed: 0 additions & 29 deletions
This file was deleted.

packages/react-router-dom/__tests__/setup.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,6 @@ import {
55
import { fetch, Request, Response, Headers } from "@remix-run/web-fetch";
66
import { AbortController as NodeAbortController } from "abort-controller";
77

8-
import "./polyfills/SubmitEvent.submitter";
9-
108
// https://reactjs.org/blog/2022/03/08/react-18-upgrade-guide.html#configuring-your-testing-environment
119
globalThis.IS_REACT_ACT_ENVIRONMENT = true;
1210

packages/react-router-dom/jest.config.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,5 +21,4 @@ module.exports = {
2121
"^react-router$": "<rootDir>/../react-router/index.ts",
2222
"^react-router-dom$": "<rootDir>/index.tsx",
2323
},
24-
reporters: ["default"],
2524
};

packages/react-router-native/jest.config.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,4 @@ module.exports = {
2424
"^react-router$": "<rootDir>/../react-router/index.ts",
2525
"^react-router-native$": "<rootDir>/index.tsx",
2626
},
27-
reporters: ["default"],
2827
};

packages/router/__tests__/router-test.ts

Lines changed: 20 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -428,6 +428,18 @@ function setup({
428428
});
429429
}
430430

431+
// jsdom is making more and more properties non-configurable, so we inject
432+
// our own jest-friendly window.
433+
let testWindow = {
434+
...window,
435+
location: {
436+
...window.location,
437+
assign: jest.fn(),
438+
replace: jest.fn(),
439+
},
440+
} as unknown as Window;
441+
// ^ Spread makes TS sad - `window.NaN` conflicts with `[index: number]: Window`
442+
431443
let history = createMemoryHistory({ initialEntries, initialIndex });
432444
jest.spyOn(history, "push");
433445
jest.spyOn(history, "replace");
@@ -437,6 +449,7 @@ function setup({
437449
routes: enhanceRoutes(routes),
438450
hydrationData,
439451
future,
452+
window: testWindow,
440453
}).initialize();
441454

442455
function getRouteHelpers(
@@ -843,6 +856,7 @@ function setup({
843856
}
844857

845858
return {
859+
window: testWindow,
846860
history,
847861
router: currentRouter,
848862
navigate,
@@ -6545,15 +6559,6 @@ describe("a router", () => {
65456559
];
65466560

65476561
for (let url of urls) {
6548-
// This is gross, don't blame me, blame SO :)
6549-
// https://stackoverflow.com/a/60697570
6550-
let oldLocation = window.location;
6551-
const location = new URL(window.location.href) as unknown as Location;
6552-
location.assign = jest.fn();
6553-
location.replace = jest.fn();
6554-
delete (window as any).location;
6555-
window.location = location as unknown as Location;
6556-
65576562
let t = setup({ routes: REDIRECT_ROUTES });
65586563

65596564
let A = await t.navigate("/parent/child", {
@@ -6562,10 +6567,8 @@ describe("a router", () => {
65626567
});
65636568

65646569
await A.actions.child.redirectReturn(url);
6565-
expect(window.location.assign).toHaveBeenCalledWith(url);
6566-
expect(window.location.replace).not.toHaveBeenCalled();
6567-
6568-
window.location = oldLocation;
6570+
expect(t.window.location.assign).toHaveBeenCalledWith(url);
6571+
expect(t.window.location.replace).not.toHaveBeenCalled();
65696572
}
65706573
});
65716574

@@ -6578,15 +6581,6 @@ describe("a router", () => {
65786581
];
65796582

65806583
for (let url of urls) {
6581-
// This is gross, don't blame me, blame SO :)
6582-
// https://stackoverflow.com/a/60697570
6583-
let oldLocation = window.location;
6584-
const location = new URL(window.location.href) as unknown as Location;
6585-
location.assign = jest.fn();
6586-
location.replace = jest.fn();
6587-
delete (window as any).location;
6588-
window.location = location as unknown as Location;
6589-
65906584
let t = setup({ routes: REDIRECT_ROUTES });
65916585

65926586
let A = await t.navigate("/parent/child", {
@@ -6596,10 +6590,8 @@ describe("a router", () => {
65966590
});
65976591

65986592
await A.actions.child.redirectReturn(url);
6599-
expect(window.location.replace).toHaveBeenCalledWith(url);
6600-
expect(window.location.assign).not.toHaveBeenCalled();
6601-
6602-
window.location = oldLocation;
6593+
expect(t.window.location.replace).toHaveBeenCalledWith(url);
6594+
expect(t.window.location.assign).not.toHaveBeenCalled();
66036595
}
66046596
});
66056597

@@ -6654,15 +6646,6 @@ describe("a router", () => {
66546646
});
66556647

66566648
it("treats same-origin absolute URLs as external if they don't match the basename", async () => {
6657-
// This is gross, don't blame me, blame SO :)
6658-
// https://stackoverflow.com/a/60697570
6659-
let oldLocation = window.location;
6660-
const location = new URL(window.location.href) as unknown as Location;
6661-
location.assign = jest.fn();
6662-
location.replace = jest.fn();
6663-
delete (window as any).location;
6664-
window.location = location as unknown as Location;
6665-
66666649
let t = setup({ routes: REDIRECT_ROUTES, basename: "/base" });
66676650

66686651
let A = await t.navigate("/base/parent/child", {
@@ -6672,10 +6655,8 @@ describe("a router", () => {
66726655

66736656
let url = "http://localhost/not/the/same/basename";
66746657
await A.actions.child.redirectReturn(url);
6675-
expect(window.location.assign).toHaveBeenCalledWith(url);
6676-
expect(window.location.replace).not.toHaveBeenCalled();
6677-
6678-
window.location = oldLocation;
6658+
expect(t.window.location.assign).toHaveBeenCalledWith(url);
6659+
expect(t.window.location.replace).not.toHaveBeenCalled();
66796660
});
66806661

66816662
describe("redirect status code handling", () => {

packages/router/jest.config.js

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,5 +16,4 @@ module.exports = {
1616
"@web3-storage/multipart-parser"
1717
),
1818
},
19-
reporters: ["default"],
2019
};

packages/router/router.ts

Lines changed: 16 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -352,6 +352,7 @@ export interface RouterInit {
352352
mapRouteProperties?: MapRoutePropertiesFunction;
353353
future?: Partial<FutureConfig>;
354354
hydrationData?: HydrationState;
355+
window?: Window;
355356
}
356357

357358
/**
@@ -661,12 +662,6 @@ export const IDLE_BLOCKER: BlockerUnblocked = {
661662

662663
const ABSOLUTE_URL_REGEX = /^(?:[a-z][a-z0-9+.-]*:|\/\/)/i;
663664

664-
const isBrowser =
665-
typeof window !== "undefined" &&
666-
typeof window.document !== "undefined" &&
667-
typeof window.document.createElement !== "undefined";
668-
const isServer = !isBrowser;
669-
670665
const defaultMapRouteProperties: MapRoutePropertiesFunction = (route) => ({
671666
hasErrorBoundary: Boolean(route.hasErrorBoundary),
672667
});
@@ -681,6 +676,17 @@ const defaultMapRouteProperties: MapRoutePropertiesFunction = (route) => ({
681676
* Create a router and listen to history POP navigations
682677
*/
683678
export function createRouter(init: RouterInit): Router {
679+
const routerWindow = init.window
680+
? init.window
681+
: typeof window !== "undefined"
682+
? window
683+
: undefined;
684+
const isBrowser =
685+
typeof routerWindow !== "undefined" &&
686+
typeof routerWindow.document !== "undefined" &&
687+
typeof routerWindow.document.createElement !== "undefined";
688+
const isServer = !isBrowser;
689+
684690
invariant(
685691
init.routes.length > 0,
686692
"You must provide a non-empty routes array to createRouter"
@@ -2087,19 +2093,15 @@ export function createRouter(init: RouterInit): Router {
20872093
"Expected a location on the redirect navigation"
20882094
);
20892095
// Check if this an absolute external redirect that goes to a new origin
2090-
if (
2091-
ABSOLUTE_URL_REGEX.test(redirect.location) &&
2092-
isBrowser &&
2093-
typeof window?.location !== "undefined"
2094-
) {
2096+
if (ABSOLUTE_URL_REGEX.test(redirect.location) && isBrowser) {
20952097
let url = init.history.createURL(redirect.location);
20962098
let isDifferentBasename = stripBasename(url.pathname, basename) == null;
20972099

2098-
if (window.location.origin !== url.origin || isDifferentBasename) {
2100+
if (routerWindow.location.origin !== url.origin || isDifferentBasename) {
20992101
if (replace) {
2100-
window.location.replace(redirect.location);
2102+
routerWindow.location.replace(redirect.location);
21012103
} else {
2102-
window.location.assign(redirect.location);
2104+
routerWindow.location.assign(redirect.location);
21032105
}
21042106
return;
21052107
}

0 commit comments

Comments
 (0)