-
-
Notifications
You must be signed in to change notification settings - Fork 10.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix URL creation with memory history #9814
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
- Loading branch information
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,107 @@ | ||
/** | ||
* @jest-environment node | ||
*/ | ||
Comment on lines
+1
to
+3
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Because tests by default run in jsdom, we didn't catch this regression since |
||
|
||
import { createMemoryHistory, createRouter } from "../index"; | ||
|
||
// This suite of tests specifically runs in the node jest environment to catch | ||
// issues when window is not present | ||
describe("a memory router", () => { | ||
it("initializes properly", async () => { | ||
let router = createRouter({ | ||
routes: [ | ||
{ | ||
path: "/", | ||
}, | ||
], | ||
history: createMemoryHistory(), | ||
}); | ||
expect(router.state).toEqual({ | ||
historyAction: "POP", | ||
loaderData: {}, | ||
actionData: null, | ||
errors: null, | ||
location: { | ||
hash: "", | ||
key: expect.any(String), | ||
pathname: "/", | ||
search: "", | ||
state: null, | ||
}, | ||
matches: [ | ||
{ | ||
params: {}, | ||
pathname: "/", | ||
pathnameBase: "/", | ||
route: { | ||
id: "0", | ||
path: "/", | ||
}, | ||
}, | ||
], | ||
initialized: true, | ||
navigation: { | ||
location: undefined, | ||
state: "idle", | ||
}, | ||
preventScrollReset: false, | ||
restoreScrollPosition: null, | ||
revalidation: "idle", | ||
fetchers: new Map(), | ||
}); | ||
router.dispose(); | ||
}); | ||
|
||
it("can create Requests without window", async () => { | ||
let loaderSpy = jest.fn(); | ||
let router = createRouter({ | ||
routes: [ | ||
{ | ||
path: "/", | ||
}, | ||
{ | ||
path: "/a", | ||
loader: loaderSpy, | ||
}, | ||
], | ||
history: createMemoryHistory(), | ||
}); | ||
|
||
router.navigate("/a"); | ||
expect(loaderSpy.mock.calls[0][0].request.url).toBe("unknown://unknown/a"); | ||
router.dispose(); | ||
}); | ||
|
||
it("can create URLs without window", async () => { | ||
let shouldRevalidateSpy = jest.fn(); | ||
|
||
let router = createRouter({ | ||
routes: [ | ||
{ | ||
path: "/", | ||
loader: () => "ROOT", | ||
shouldRevalidate: shouldRevalidateSpy, | ||
children: [ | ||
{ | ||
index: true, | ||
}, | ||
{ | ||
path: "a", | ||
}, | ||
], | ||
}, | ||
], | ||
history: createMemoryHistory(), | ||
hydrationData: { loaderData: { "0": "ROOT" } }, | ||
}); | ||
|
||
router.navigate("/a"); | ||
expect(shouldRevalidateSpy.mock.calls[0][0].currentUrl.toString()).toBe( | ||
"unknown://unknown/" | ||
); | ||
expect(shouldRevalidateSpy.mock.calls[0][0].nextUrl.toString()).toBe( | ||
"unknown://unknown/a" | ||
); | ||
router.dispose(); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,4 +1,9 @@ | ||
import { | ||
TextEncoder as NodeTextEncoder, | ||
TextDecoder as NodeTextDecoder, | ||
} from "util"; | ||
import { fetch, Request, Response } from "@remix-run/web-fetch"; | ||
import { AbortController as NodeAbortController } from "abort-controller"; | ||
|
||
if (!globalThis.fetch) { | ||
// Built-in lib.dom.d.ts expects `fetch(Request | string, ...)` but the web | ||
|
@@ -13,3 +18,14 @@ if (!globalThis.fetch) { | |
// @ts-expect-error | ||
globalThis.Response = Response; | ||
} | ||
|
||
if (!globalThis.AbortController) { | ||
// @ts-expect-error | ||
globalThis.AbortController = NodeAbortController; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Needed for the new node test run |
||
} | ||
|
||
if (!globalThis.TextEncoder || !globalThis.TextDecoder) { | ||
globalThis.TextEncoder = NodeTextEncoder; | ||
// @ts-expect-error | ||
globalThis.TextDecoder = NodeTextDecoder; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Copied over from the deleted custom environment |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unsure why we had 2 ways of doing this but I moved the custom environment stuff into
setup.ts