-
Notifications
You must be signed in to change notification settings - Fork 2.6k
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
Add fetcher state/type tests #4803
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,271 @@ | ||
import { test, expect } from "@playwright/test"; | ||
import { ServerMode } from "@remix-run/server-runtime/mode"; | ||
|
||
import { createAppFixture, createFixture, js } from "./helpers/create-fixture"; | ||
import type { Fixture, AppFixture } from "./helpers/create-fixture"; | ||
import { PlaywrightFixture } from "./helpers/playwright-fixture"; | ||
|
||
// idle, done, actionReload are tested during the testing of these flows | ||
const TYPES = { | ||
actionSubmission: "actionSubmission", | ||
loaderSubmission: "loaderSubmission", | ||
actionRedirect: "actionRedirect", | ||
normalLoad: "normalLoad", | ||
}; | ||
|
||
test.describe("fetcher states", () => { | ||
let fixture: Fixture; | ||
let appFixture: AppFixture; | ||
|
||
test.beforeAll(async () => { | ||
fixture = await createFixture({ | ||
files: { | ||
"app/root.jsx": js` | ||
import { useMemo, useRef } from "react"; | ||
import { Outlet, Scripts, useFetchers } from "@remix-run/react"; | ||
|
||
export default function Comp() { | ||
// Only gonna use a single fetcher in any given test but this way | ||
// we can route away from the child route and preserve the info | ||
const [fetcher] = useFetchers(); | ||
const fetcherRef = useRef(); | ||
const states = useMemo(() => { | ||
if (!fetcher) return | ||
const savedStates = fetcherRef.current || []; | ||
savedStates.push({ | ||
state: fetcher.state, | ||
type: fetcher.type, | ||
submission: fetcher.submission ? { | ||
...fetcher.submission, | ||
formData: Object.fromEntries(fetcher.submission.formData.entries()), | ||
key: undefined | ||
}: undefined, | ||
data: fetcher.data, | ||
}); | ||
fetcherRef.current = savedStates; | ||
return savedStates; | ||
}, [fetcher]); | ||
|
||
return ( | ||
<html lang="en"> | ||
<body> | ||
<Outlet /> | ||
{fetcher && fetcher.state != "idle" && ( | ||
<p id="loading">Loading...</p> | ||
)} | ||
<p> | ||
<code id="states"> | ||
{JSON.stringify(states, null, 2)} | ||
</code> | ||
</p> | ||
<Scripts /> | ||
</body> | ||
</html> | ||
); | ||
} | ||
`, | ||
"app/routes/page.jsx": js` | ||
import { redirect } from "@remix-run/node"; | ||
import { useFetcher } from "@remix-run/react"; | ||
export function loader() { | ||
return { from: 'loader' } | ||
} | ||
export async function action({ request }) { | ||
let fd = await request.formData() | ||
if (fd.has('redirect')) { | ||
return redirect('/redirect'); | ||
} | ||
return { from: 'action' } | ||
} | ||
export default function() { | ||
const fetcher = useFetcher(); | ||
return ( | ||
<> | ||
{fetcher.type === 'init' ? | ||
<pre id="initial-state"> | ||
{ | ||
JSON.stringify({ | ||
state: fetcher.state, | ||
type: fetcher.type, | ||
submission: fetcher.submission, | ||
data: fetcher.data, | ||
}) | ||
} | ||
</pre> : | ||
null} | ||
<fetcher.Form method="post"> | ||
${TYPES.actionSubmission} | ||
<button type="submit" name="key" value="value" id="${TYPES.actionSubmission}"> | ||
Submit ${TYPES.actionSubmission} | ||
</button> | ||
</fetcher.Form> | ||
<fetcher.Form method="get"> | ||
${TYPES.loaderSubmission} | ||
<button type="submit" name="key" value="value" id="${TYPES.loaderSubmission}"> | ||
Submit ${TYPES.loaderSubmission} | ||
</button> | ||
</fetcher.Form> | ||
<fetcher.Form method="post"> | ||
${TYPES.actionRedirect} | ||
<button type="submit" name="redirect" value="yes" id="${TYPES.actionRedirect}"> | ||
Submit ${TYPES.actionRedirect} | ||
</button> | ||
</fetcher.Form> | ||
<button id="${TYPES.normalLoad}" onClick={() => fetcher.load('/page')}> | ||
Submit ${TYPES.normalLoad} | ||
</button> | ||
</> | ||
); | ||
} | ||
`, | ||
"app/routes/redirect.jsx": js` | ||
import { useFetcher } from "@remix-run/react"; | ||
export function loader() { | ||
return { from: 'redirect loader' } | ||
} | ||
export default function() { | ||
return <h1>Redirect</h1>; | ||
} | ||
`, | ||
}, | ||
}); | ||
|
||
appFixture = await createAppFixture(fixture, ServerMode.Development); | ||
}); | ||
|
||
test.afterAll(async () => { | ||
await appFixture.close(); | ||
}); | ||
|
||
test("represents a initial fetcher", async ({ page }) => { | ||
let app = new PlaywrightFixture(appFixture, page); | ||
await app.goto("/page", true); | ||
let text = (await app.getElement("#initial-state")).text(); | ||
expect(JSON.parse(text)).toEqual({ | ||
state: "idle", | ||
type: "init", | ||
}); | ||
}); | ||
|
||
test("represents an actionSubmission fetcher", async ({ page }) => { | ||
let app = new PlaywrightFixture(appFixture, page); | ||
await app.goto("/page", true); | ||
await app.clickElement(`#${TYPES.actionSubmission}`); | ||
await page.waitForSelector("#loading", { state: "hidden" }); | ||
let text = (await app.getElement("#states")).text(); | ||
expect(JSON.parse(text)).toEqual([ | ||
{ | ||
state: "submitting", | ||
type: "actionSubmission", | ||
submission: { | ||
formData: { key: "value" }, | ||
action: "/page", | ||
method: "POST", | ||
encType: "application/x-www-form-urlencoded", | ||
}, | ||
}, | ||
{ | ||
state: "loading", | ||
type: "actionReload", | ||
submission: { | ||
formData: { key: "value" }, | ||
action: "/page", | ||
method: "POST", | ||
encType: "application/x-www-form-urlencoded", | ||
}, | ||
data: { | ||
from: "action", | ||
}, | ||
}, | ||
{ | ||
state: "idle", | ||
type: "done", | ||
data: { | ||
from: "action", | ||
}, | ||
}, | ||
]); | ||
}); | ||
|
||
test("represents a loaderSubmission fetcher", async ({ page }) => { | ||
let app = new PlaywrightFixture(appFixture, page); | ||
await app.goto("/page", true); | ||
await app.clickElement(`#${TYPES.loaderSubmission}`); | ||
await page.waitForSelector("#loading", { state: "hidden" }); | ||
let text = (await app.getElement("#states")).text(); | ||
expect(JSON.parse(text)).toEqual([ | ||
{ | ||
state: "submitting", | ||
type: "loaderSubmission", | ||
submission: { | ||
formData: { key: "value" }, | ||
// TODO: This feels potentially wrong - the formAction is just /page, | ||
// the request URL is /page?key=value | ||
action: "/page?key=value", | ||
method: "GET", | ||
encType: "application/x-www-form-urlencoded", | ||
}, | ||
}, | ||
{ | ||
state: "idle", | ||
type: "done", | ||
data: { | ||
from: "loader", | ||
}, | ||
}, | ||
]); | ||
}); | ||
|
||
test("represents an actionRedirect fetcher", async ({ page }) => { | ||
let app = new PlaywrightFixture(appFixture, page); | ||
await app.goto("/page", true); | ||
await app.clickElement(`#${TYPES.actionRedirect}`); | ||
await page.waitForSelector("#loading", { state: "hidden" }); | ||
let text = (await app.getElement("#states")).text(); | ||
expect(JSON.parse(text)).toEqual([ | ||
{ | ||
state: "submitting", | ||
type: "actionSubmission", | ||
submission: { | ||
formData: { redirect: "yes" }, | ||
action: "/page", | ||
method: "POST", | ||
encType: "application/x-www-form-urlencoded", | ||
}, | ||
}, | ||
{ | ||
state: "loading", | ||
type: "actionRedirect", | ||
submission: { | ||
formData: { redirect: "yes" }, | ||
action: "/page", | ||
method: "POST", | ||
encType: "application/x-www-form-urlencoded", | ||
}, | ||
}, | ||
{ | ||
state: "idle", | ||
type: "done", | ||
}, | ||
]); | ||
}); | ||
|
||
test("represents a normalLoad fetcher", async ({ page }) => { | ||
let app = new PlaywrightFixture(appFixture, page); | ||
await app.goto("/page", true); | ||
await app.clickElement(`#${TYPES.normalLoad}`); | ||
await page.waitForSelector("#loading", { state: "hidden" }); | ||
let text = (await app.getElement("#states")).text(); | ||
expect(JSON.parse(text)).toEqual([ | ||
{ | ||
state: "loading", | ||
type: "normalLoad", | ||
}, | ||
{ | ||
data: { from: "loader" }, | ||
state: "idle", | ||
type: "done", | ||
}, | ||
]); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,8 +16,8 @@ | |
"typings": "dist/index.d.ts", | ||
"module": "dist/esm/index.js", | ||
"dependencies": { | ||
"@remix-run/router": "1.0.4", | ||
"react-router-dom": "6.4.4" | ||
"@remix-run/router": "1.0.5", | ||
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. This was out of sync with the version used in Generally speaking we'll upgrade these in lock step going forward so I expect this is largely a non-issue - this mismatch is just a function of the PR to update to |
||
"react-router-dom": "6.4.5" | ||
}, | ||
"devDependencies": { | ||
"@remix-run/server-runtime": "1.8.2", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2069,11 +2069,6 @@ | |
"@changesets/types" "^5.0.0" | ||
dotenv "^8.1.0" | ||
|
||
"@remix-run/[email protected]": | ||
version "1.0.4" | ||
resolved "https://registry.npmjs.org/@remix-run/router/-/router-1.0.4.tgz#cbfbec6735711e7c2fc93b9b40adf70ef5a39990" | ||
integrity sha512-gTL8H5USTAKOyVA4xczzDJnC3HMssdFa3tRlwBicXynx9XfiXwneHnYQogwSKpdCkjXISrEKSTtX62rLpNEVQg== | ||
|
||
"@remix-run/[email protected]": | ||
version "1.0.5" | ||
resolved "https://registry.npmjs.org/@remix-run/router/-/router-1.0.5.tgz#d5c65626add4c3c185a89aa5bd38b1e42daec075" | ||
|
@@ -10727,20 +10722,20 @@ react-is@^17.0.1: | |
resolved "https://registry.npmjs.org/react-is/-/react-is-17.0.2.tgz" | ||
integrity sha512-w2GsyukL62IJnlaff/nRegPQR94C/XXamvMWmSHRJ4y7Ts/4ocGRmTHvOs8PSE6pB3dWOrD/nueuU5sduBsQ4w== | ||
|
||
[email protected].4: | ||
version "6.4.4" | ||
resolved "https://registry.npmjs.org/react-router-dom/-/react-router-dom-6.4.4.tgz#4271ec66333c440d1754477e4e6a3a5acb5487f8" | ||
integrity sha512-0Axverhw5d+4SBhLqLpzPhNkmv7gahUwlUVIOrRLGJ4/uwt30JVajVJXqv2Qr/LCwyvHhQc7YyK1Do8a9Jj7qA== | ||
[email protected].5: | ||
version "6.4.5" | ||
resolved "https://registry.npmjs.org/react-router-dom/-/react-router-dom-6.4.5.tgz#4fdb12efef4f3848c693a76afbeaed1f6ca02047" | ||
integrity sha512-a7HsgikBR0wNfroBHcZUCd9+mLRqZS8R5U1Z1mzLWxFXEkUT3vR1XXmSIVoVpxVX8Bar0nQYYYc9Yipq8dWwAA== | ||
dependencies: | ||
"@remix-run/router" "1.0.4" | ||
react-router "6.4.4" | ||
"@remix-run/router" "1.0.5" | ||
react-router "6.4.5" | ||
|
||
[email protected].4: | ||
version "6.4.4" | ||
resolved "https://registry.npmjs.org/react-router/-/react-router-6.4.4.tgz#8e7794f55ccc7050cb03937c87ff3720ce9f8b60" | ||
integrity sha512-SA6tSrUCRfuLWeYsTJDuriRqfFIsrSvuH7SqAJHegx9ZgxadE119rU8oOX/rG5FYEthpdEaEljdjDlnBxvfr+Q== | ||
[email protected].5: | ||
version "6.4.5" | ||
resolved "https://registry.npmjs.org/react-router/-/react-router-6.4.5.tgz#73f382af2c8b9a86d74e761a7c5fc3ce7cb0024d" | ||
integrity sha512-1RQJ8bM70YEumHIlNUYc6mFfUDoWa5EgPDenK/fq0bxD8DYpQUi/S6Zoft+9DBrh2xmtg92N5HMAJgGWDhKJ5Q== | ||
dependencies: | ||
"@remix-run/router" "1.0.4" | ||
"@remix-run/router" "1.0.5" | ||
|
||
react@^18.2.0: | ||
version "18.2.0" | ||
|
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.
@jacob-ebey What do you think here? IIRC we specifically tested against this in
@remix-run/router
so thesubmission
stayed true to the state of the DOM and only therequest
represented the serializedURLSearchParams
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.
Same behavior happens for
loaderSubmission
transitions - updated the test intransition-state-test
below with the same logicThere 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.
If we think this is wrong we can probably just let this test pass with a note that it'll break as a bug fix when we layer in 6.4, instead of attempting to fix it in the current transition manager.