Skip to content
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

Merged
merged 4 commits into from
Dec 8, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
271 changes: 271 additions & 0 deletions integration/fetcher-state-test.ts
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",
},
Copy link
Contributor Author

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 the submission stayed true to the state of the DOM and only the request represented the serialized URLSearchParams

Copy link
Contributor Author

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 in transition-state-test below with the same logic

Copy link
Contributor Author

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.

},
{
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",
},
]);
});
});
4 changes: 2 additions & 2 deletions packages/remix-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was out of sync with the version used in @remix-run/server-runtime which caused issues during integration tests. I don't think this would be an actual problem in apps since server-runtime had it's own copy nested in a local node_modules, however we do some tricky stuff with our integration tests builds and I think everything reads from the global repo node_modules so it was picking up on the wrong version from @remix-run/react.

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 [email protected] and the Remix 1.8.2 hotfix release that bumped to @remix-run/[email protected] merging side by side.

"react-router-dom": "6.4.5"
},
"devDependencies": {
"@remix-run/server-runtime": "1.8.2",
Expand Down
27 changes: 11 additions & 16 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down