Skip to content

Commit

Permalink
feat: remove singleFetch flag (#11522)
Browse files Browse the repository at this point in the history 
* feat: remove singleFetch flag

* remove another reference

* chore: update unit tests

* fix: spa mode tests

* update resource tests

* fix commented out test

* Remove server://singlefetch origin stub

* Remove unused headers.ts file and set-cookie-parser dependencies

* Remove entry.server.spa.tsx

* Remove/adjust a few remaining _data references in tests

* Turn off integration test debug logs

* Remove unused _data request code paths

* Revert "Remove entry.server.spa.tsx"

This reverts commit dd6a86c.

* Revert "Remove server://singlefetch origin stub"

This reverts commit d16ec89.

* Update comment for clarity

* format

* chore: ignore playgrounds from changesets
chore: add changeset

---------

Co-authored-by: Matt Brophy <[email protected]>
  • Loading branch information
@jacob-ebey @brophdawg11
jacob-ebey and brophdawg11 authored May 9, 2024
1 parent 94d9734 commit aab4ea0
Show file tree
Hide file tree
Showing 61 changed files with 2,125 additions and 12,254 deletions.
2 changes: 1 addition & 1 deletion .changeset/config.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@
"baseBranch": "dev",
"updateInternalDependencies": "patch",
"bumpVersionsWithWorkspaceProtocolOnly": true,
"ignore": ["integration", "integration-*"],
"ignore": ["integration", "integration-*", "@playground/*"],
"___experimentalUnsafeOptions_WILL_CHANGE_IN_PATCH": {
"onlyUpdatePeerDependentsWhenOutOfRange": true
}
Expand Down
11 changes: 11 additions & 0 deletions .changeset/curvy-teachers-explain.md
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
---
"@react-router/server-runtime": major
"react-router-dom": major
"@react-router/express": major
"react-router": major
"@react-router/serve": major
"@react-router/node": major
"@react-router/dev": major
---

Remove single_fetch future flag.
218 changes: 4 additions & 214 deletions integration/action-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import {
import type { Fixture, AppFixture } from "./helpers/create-fixture.js";
import { PlaywrightFixture, selectHtml } from "./helpers/playwright-fixture.js";

test.describe.skip("actions", () => {
test.describe("actions", () => {
let fixture: Fixture;
let appFixture: AppFixture;

Expand All @@ -21,6 +21,7 @@ test.describe.skip("actions", () => {

test.beforeAll(async () => {
fixture = await createFixture({
singleFetch: true,
files: {
"app/routes/urlencoded.tsx": js`
import { Form, useActionData } from "react-router-dom";
Expand Down Expand Up @@ -199,226 +200,15 @@ test.describe.skip("actions", () => {
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto(`/${THROWS_REDIRECT}`);
let responses = app.collectDataResponses();
let responses = app.collectSingleFetchResponses();
await app.clickSubmitButton(`/${THROWS_REDIRECT}`);

await page.waitForSelector(`#${REDIRECT_TARGET}`);

expect(responses.length).toBe(1);
expect(responses[0].status()).toBe(204);
expect(responses[0].status()).toBe(200);

expect(new URL(page.url()).pathname).toBe(`/${REDIRECT_TARGET}`);
expect(await app.getHtml()).toMatch(PAGE_TEXT);
});
});

// Duplicate suite of the tests above running with single fetch enabled
// TODO(v3): remove the above suite of tests and just keep these
test.describe("single fetch", () => {
test.describe("actions", () => {
let fixture: Fixture;
let appFixture: AppFixture;

let FIELD_NAME = "message";
let WAITING_VALUE = "Waiting...";
let SUBMITTED_VALUE = "Submission";
let THROWS_REDIRECT = "redirect-throw";
let REDIRECT_TARGET = "page";
let PAGE_TEXT = "PAGE_TEXT";

test.beforeAll(async () => {
fixture = await createFixture({
singleFetch: true,
files: {
"app/routes/urlencoded.tsx": js`
import { Form, useActionData } from "react-router-dom";
export let action = async ({ request }) => {
let formData = await request.formData();
return formData.get("${FIELD_NAME}");
};
export default function Actions() {
let data = useActionData()
return (
<Form method="post" id="form">
<p id="text">
{data ? <span id="action-text">{data}</span> : "${WAITING_VALUE}"}
</p>
<p>
<input type="text" defaultValue="${SUBMITTED_VALUE}" name="${FIELD_NAME}" />
<button type="submit" id="submit">Go</button>
</p>
</Form>
);
}
`,

"app/routes/request-text.tsx": js`
import { Form, useActionData } from "react-router-dom";
export let action = async ({ request }) => {
let text = await request.text();
return text;
};
export default function Actions() {
let data = useActionData()
return (
<Form method="post" id="form">
<p id="text">
{data ? <span id="action-text">{data}</span> : "${WAITING_VALUE}"}
</p>
<p>
<input name="a" defaultValue="1" />
<input name="b" defaultValue="2" />
<button type="submit" id="submit">Go</button>
</p>
</Form>
);
}
`,

[`app/routes/${THROWS_REDIRECT}.jsx`]: js`
import { redirect } from "@react-router/node";
import { Form } from "react-router-dom";
export function action() {
throw redirect("/${REDIRECT_TARGET}")
}
export default function () {
return (
<Form method="post">
<button type="submit">Go</button>
</Form>
)
}
`,

[`app/routes/${REDIRECT_TARGET}.jsx`]: js`
export default function () {
return <div id="${REDIRECT_TARGET}">${PAGE_TEXT}</div>
}
`,

"app/routes/no-action.tsx": js`
import { Form } from "react-router-dom";
export default function Component() {
return (
<Form method="post">
<button type="submit">Submit without action</button>
</Form>
);
}
`,
},
});

appFixture = await createAppFixture(fixture);
});

test.afterAll(() => {
appFixture.close();
});

let logs: string[] = [];

test.beforeEach(({ page }) => {
page.on("console", (msg) => {
logs.push(msg.text());
});
});

test.afterEach(() => {
expect(logs).toHaveLength(0);
});

test("is not called on document GET requests", async () => {
let res = await fixture.requestDocument("/urlencoded");
let html = selectHtml(await res.text(), "#text");
expect(html).toMatch(WAITING_VALUE);
});

test("is called on document POST requests", async () => {
let FIELD_VALUE = "cheeseburger";

let params = new URLSearchParams();
params.append(FIELD_NAME, FIELD_VALUE);

let res = await fixture.postDocument("/urlencoded", params);

let html = selectHtml(await res.text(), "#text");
expect(html).toMatch(FIELD_VALUE);
});

test("is called on script transition POST requests", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto(`/urlencoded`);
await page.waitForSelector(`#text:has-text("${WAITING_VALUE}")`);

await page.click("button[type=submit]");
await page.waitForSelector("#action-text");
await page.waitForSelector(`#text:has-text("${SUBMITTED_VALUE}")`);
});

test("throws a 405 when no action exists", async ({ page }) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto(`/no-action`);
await page.click("button[type=submit]");
await page.waitForSelector(`h1:has-text("405 Method Not Allowed")`);
expect(logs.length).toBe(2);
expect(logs[0]).toMatch(
'Route "routes/no-action" does not have an action'
);
// logs[1] is the raw ErrorResponse instance from the boundary but playwright
// seems to just log the name of the constructor, which in the minified code
// is meaningless so we don't bother asserting

// The rest of the tests in this suite assert no logs, so clear this out to
// avoid failures in afterEach
logs = [];
});

test("properly encodes form data for request.text() usage", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto(`/request-text`);
await page.waitForSelector(`#text:has-text("${WAITING_VALUE}")`);

await page.click("button[type=submit]");
await page.waitForSelector("#action-text");
expect(await app.getHtml("#action-text")).toBe(
'<span id="action-text">a=1&amp;b=2</span>'
);
});

test("redirects a thrown response on document requests", async () => {
let params = new URLSearchParams();
let res = await fixture.postDocument(`/${THROWS_REDIRECT}`, params);
expect(res.status).toBe(302);
expect(res.headers.get("Location")).toBe(`/${REDIRECT_TARGET}`);
});

test("redirects a thrown response on script transitions", async ({
page,
}) => {
let app = new PlaywrightFixture(appFixture, page);
await app.goto(`/${THROWS_REDIRECT}`);
let responses = app.collectSingleFetchResponses();
await app.clickSubmitButton(`/${THROWS_REDIRECT}`);

await page.waitForSelector(`#${REDIRECT_TARGET}`);

expect(responses.length).toBe(1);
expect(responses[0].status()).toBe(200);

expect(new URL(page.url()).pathname).toBe(`/${REDIRECT_TARGET}`);
expect(await app.getHtml()).toMatch(PAGE_TEXT);
});
});
});
2 changes: 1 addition & 1 deletion integration/bug-report-test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,7 @@ let appFixture: AppFixture;
////////////////////////////////////////////////////////////////////////////////

test.beforeEach(async ({ context }) => {
await context.route(/_data/, async (route) => {
await context.route(/\.data$/, async (route) => {
await new Promise((resolve) => setTimeout(resolve, 50));
route.continue();
});
Expand Down
Loading

0 comments on commit aab4ea0

Please sign in to comment.