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 future flag for Single Fetch #8773

Merged
merged 62 commits into from
Mar 12, 2024
Merged
Show file tree
Hide file tree
Changes from 23 commits
Commits
Show all changes
62 commits
Select commit Hold shift + click to select a range
29a1e30
Bump to RR experimental
brophdawg11 Feb 6, 2024
78c011e
Initial implementation of single fetch for loaders
brophdawg11 Feb 6, 2024
e837499
Add future flag
brophdawg11 Feb 6, 2024
05ef1de
Bump RR experimental to allow boolean loaders
brophdawg11 Feb 6, 2024
59758b3
Handle clientLoaders with single fetch enabled
brophdawg11 Feb 6, 2024
1c5978b
Use turbo-stream for single fetch responses
brophdawg11 Feb 8, 2024
195cd18
POC of streamiong loader data down in action response
brophdawg11 Feb 8, 2024
cc8a03f
Move back to separate action and revalidation requests
brophdawg11 Feb 9, 2024
918d340
WIP POC of granular revalidation
brophdawg11 Feb 9, 2024
18378f1
Support fine-grained revalidation
brophdawg11 Feb 13, 2024
ce9b27d
Fix action bodies
brophdawg11 Feb 13, 2024
a4631bd
Bump RR experimental
brophdawg11 Feb 14, 2024
dd0203b
Move single fetch implementation out of browser.tsx
brophdawg11 Feb 14, 2024
77ac53e
Fix css/loading parallelization issues by passing singleFetch through…
brophdawg11 Feb 14, 2024
a0fffd7
Bump RR Experimental
brophdawg11 Feb 14, 2024
a2dc7a4
Streamline routes code and fix a few e2e tests bugs
brophdawg11 Feb 14, 2024
49b46be
Bump turbo-stream
brophdawg11 Feb 15, 2024
bbc0728
Switch from X-Remix-Routes header to _routes query param
brophdawg11 Feb 15, 2024
c962cf7
Update to latest RR
brophdawg11 Feb 15, 2024
3d7c4eb
Proxy action status back through DecodedResponse
brophdawg11 Feb 16, 2024
36887bb
Bump RR experimental
brophdawg11 Feb 16, 2024
308b010
RR experimental
brophdawg11 Feb 16, 2024
d5bed65
Fix unit tests
brophdawg11 Feb 16, 2024
5cfdec3
Bump RR Experimental
brophdawg11 Feb 16, 2024
509863d
Bump RR Experimental
brophdawg11 Feb 29, 2024
45f07c8
Minor updates and fixes from E2E test runs
brophdawg11 Feb 29, 2024
b5fb702
Duplicate a bunch of E2E tests to run with single fetch enabled
brophdawg11 Feb 29, 2024
1e6d13d
Merge branch 'dev' into brophdawg11/single-fetch
brophdawg11 Feb 29, 2024
38ff342
Fix exports test
brophdawg11 Feb 29, 2024
3670378
Fix lint issue
brophdawg11 Feb 29, 2024
729336c
Switch from DecodedResponse to HandlerResult
brophdawg11 Mar 1, 2024
7a80a6b
bump RR experimental
brophdawg11 Mar 1, 2024
c6750a4
Leverage turnbo-stream for document streaming when single fetch is en…
brophdawg11 Mar 1, 2024
4f9ea7b
Ficx up some E2E tests - still have some errors in error-sanitization
brophdawg11 Mar 1, 2024
1d39835
E2E test updates
brophdawg11 Mar 4, 2024
522f9e4
More E2E tests - need to do transition test next
brophdawg11 Mar 4, 2024
98ebcd5
Bump router experimental and fix lint issues
brophdawg11 Mar 5, 2024
782af76
fix unit tests
brophdawg11 Mar 5, 2024
23ca0b8
Add more E2E tests
brophdawg11 Mar 5, 2024
47a2a3b
Enable single fetch prefetching
brophdawg11 Mar 5, 2024
291bb3b
Update defer E2E tests
brophdawg11 Mar 5, 2024
14de37c
Fix up defer and a few lingering E2E tests
brophdawg11 Mar 6, 2024
903d483
Add changeset
brophdawg11 Mar 6, 2024
dce624d
Merge branch 'dev' into brophdawg11/single-fetch
brophdawg11 Mar 6, 2024
be316ac
Update pnpm-lock
brophdawg11 Mar 6, 2024
6cce524
Add remix-run/router as a dep to the integration package
brophdawg11 Mar 6, 2024
439a6cb
Merge branch 'dev' into brophdawg11/single-fetch
brophdawg11 Mar 6, 2024
e72932b
Fix prefetching unit test
brophdawg11 Mar 6, 2024
3cd4512
Fix E2E test
brophdawg11 Mar 7, 2024
9bc58ce
Merge branch 'dev' into brophdawg11/single-fetch
brophdawg11 Mar 7, 2024
4123e1f
Updates from code review
brophdawg11 Mar 8, 2024
674ac4a
Bump RR version
brophdawg11 Mar 11, 2024
b81780a
Updates for latest RR version
brophdawg11 Mar 11, 2024
2939707
Silly pnpm deps
brophdawg11 Mar 11, 2024
395078d
Update response signature ofr internal server handlers
brophdawg11 Mar 11, 2024
faf262e
Lift redirects to the root for single fetch loaders
brophdawg11 Mar 11, 2024
2d20749
Merge branch 'dev' into brophdawg11/single-fetch
brophdawg11 Mar 11, 2024
00e414c
Revert "Lift redirects to the root for single fetch loaders"
brophdawg11 Mar 11, 2024
ac3133e
Lift redirect to the top via Symbol
brophdawg11 Mar 11, 2024
8e53388
Fix E2E tests
brophdawg11 Mar 11, 2024
f672242
Fix EntryContext typings
brophdawg11 Mar 12, 2024
8d4c012
Single fetch specific E2E tests
brophdawg11 Mar 12, 2024
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
1 change: 1 addition & 0 deletions packages/remix-dev/__tests__/readConfig-test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ describe("readConfig", () => {
"entryServerFile": "entry.server.tsx",
"entryServerFilePath": Any<String>,
"future": {
"unstable_singleFetch": false,
"v3_fetcherPersist": false,
"v3_relativeSplatPath": false,
"v3_throwAbortReason": false,
Expand Down
2 changes: 2 additions & 0 deletions packages/remix-dev/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ interface FutureConfig {
v3_fetcherPersist: boolean;
v3_relativeSplatPath: boolean;
v3_throwAbortReason: boolean;
unstable_singleFetch: boolean;
}

type NodeBuiltinsPolyfillOptions = Pick<
Expand Down Expand Up @@ -600,6 +601,7 @@ export async function resolveConfig(
v3_fetcherPersist: appConfig.future?.v3_fetcherPersist === true,
v3_relativeSplatPath: appConfig.future?.v3_relativeSplatPath === true,
v3_throwAbortReason: appConfig.future?.v3_throwAbortReason === true,
unstable_singleFetch: appConfig.future?.unstable_singleFetch === true,
};

if (appConfig.future) {
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-dev/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@
"@mdx-js/mdx": "^2.3.0",
"@npmcli/package-json": "^4.0.1",
"@remix-run/node": "2.6.0",
"@remix-run/router": "1.15.0",
"@remix-run/router": "0.0.0-experimental-cbcd94b7",
"@remix-run/server-runtime": "2.6.0",
"@types/mdx": "^2.0.5",
"@vanilla-extract/integration": "^6.2.0",
Expand Down
18 changes: 12 additions & 6 deletions packages/remix-react/browser.tsx
Original file line number Diff line number Diff line change
@@ -1,9 +1,5 @@
import {
createBrowserHistory,
createRouter,
type HydrationState,
type Router,
} from "@remix-run/router";
import type { HydrationState, Router } from "@remix-run/router";
import { createBrowserHistory, createRouter } from "@remix-run/router";
import type { ReactElement } from "react";
import * as React from "react";
import { UNSAFE_mapRouteProperties as mapRouteProperties } from "react-router";
Expand All @@ -19,6 +15,7 @@ import {
createClientRoutesWithHMRRevalidationOptOut,
shouldHydrateRouteLoader,
} from "./routes";
import { getSingleFetchDataStrategy } from "./single-fetch";

/* eslint-disable prefer-let/prefer-let */
declare global {
Expand Down Expand Up @@ -275,9 +272,18 @@ export function RemixBrowser(_props: RemixBrowserProps): ReactElement {
v7_partialHydration: true,
v7_prependBasename: true,
v7_relativeSplatPath: window.__remixContext.future.v3_relativeSplatPath,
// Single fetch enables this underlying behavior
v7_skipActionErrorRevalidation:
window.__remixContext.future.unstable_singleFetch === true,
},
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Single fetch opts into this behavior automatically

hydrationData,
mapRouteProperties,
unstable_dataStrategy: window.__remixContext.future.unstable_singleFetch
? getSingleFetchDataStrategy(
window.__remixManifest,
window.__remixRouteModules
)
: undefined,
});

// We can call initialize() immediately if the router doesn't have any
Expand Down
2 changes: 1 addition & 1 deletion packages/remix-react/components.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ function getActiveMatches(
}

if (errors) {
let errorIdx = matches.findIndex((m) => errors[m.route.id]);
let errorIdx = matches.findIndex((m) => errors[m.route.id] !== undefined);
return matches.slice(0, errorIdx + 1);
}

Expand Down
59 changes: 33 additions & 26 deletions packages/remix-react/data.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ export function isNetworkErrorResponse(response: any): response is Response {
// If we reach the Remix server, we can safely identify response types via the
// X-Remix-Error/X-Remix-Catch headers. However, if we never reach the Remix
// server, and instead receive a 4xx/5xx from somewhere in between (like
// Cloudflare), then we get a false negative n the isErrorResponse check and
// Cloudflare), then we get a false negative in the isErrorResponse check and
// we incorrectly assume that the user returns the 4xx/5xx response and
// consider it successful. To alleviate this, we add X-Remix-Response to any
// non-Error/non-Catch responses coming back from the server. If we don't
Expand Down Expand Up @@ -73,37 +73,13 @@ export async function fetchData(
let url = new URL(request.url);
url.searchParams.set("_data", routeId);

let init: RequestInit = { signal: request.signal };

if (request.method !== "GET") {
init.method = request.method;

let contentType = request.headers.get("Content-Type");

// Check between word boundaries instead of startsWith() due to the last
// paragraph of https://httpwg.org/specs/rfc9110.html#field.content-type
if (contentType && /\bapplication\/json\b/.test(contentType)) {
init.headers = { "Content-Type": contentType };
init.body = JSON.stringify(await request.json());
} else if (contentType && /\btext\/plain\b/.test(contentType)) {
init.headers = { "Content-Type": contentType };
init.body = await request.text();
} else if (
contentType &&
/\bapplication\/x-www-form-urlencoded\b/.test(contentType)
) {
init.body = new URLSearchParams(await request.text());
} else {
init.body = await request.formData();
}
}

if (retry > 0) {
// Retry up to 3 times waiting 50, 250, 1250 ms
// between retries for a total of 1550 ms before giving up.
await new Promise((resolve) => setTimeout(resolve, 5 ** retry * 10));
}

let init = await createRequestInit(request);
let revalidation = window.__remixRevalidation;
let response = await fetch(url.href, init).catch((error) => {
if (
Expand Down Expand Up @@ -134,6 +110,37 @@ export async function fetchData(
return response;
}

export async function createRequestInit(
request: Request
): Promise<RequestInit> {
let init: RequestInit = { signal: request.signal };

if (request.method !== "GET") {
init.method = request.method;

let contentType = request.headers.get("Content-Type");

// Check between word boundaries instead of startsWith() due to the last
// paragraph of https://httpwg.org/specs/rfc9110.html#field.content-type
if (contentType && /\bapplication\/json\b/.test(contentType)) {
init.headers = { "Content-Type": contentType };
init.body = JSON.stringify(await request.json());
} else if (contentType && /\btext\/plain\b/.test(contentType)) {
init.headers = { "Content-Type": contentType };
init.body = await request.text();
} else if (
contentType &&
/\bapplication\/x-www-form-urlencoded\b/.test(contentType)
) {
init.body = new URLSearchParams(await request.text());
} else {
init.body = await request.formData();
}
}

return init;
}

const DEFERRED_VALUE_PLACEHOLDER_PREFIX = "__deferred_promise:";
export async function parseDeferredReadableStream(
stream: ReadableStream<Uint8Array>
Expand Down
1 change: 1 addition & 0 deletions packages/remix-react/entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export interface EntryContext extends RemixContextObject {
export interface FutureConfig {
v3_fetcherPersist: boolean;
v3_relativeSplatPath: boolean;
unstable_singleFetch: boolean;
}

export interface AssetsManifest {
Expand Down
7 changes: 4 additions & 3 deletions packages/remix-react/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@
"typings": "dist/index.d.ts",
"module": "dist/esm/index.js",
"dependencies": {
"@remix-run/router": "1.15.0",
"@remix-run/router": "0.0.0-experimental-cbcd94b7",
"@remix-run/server-runtime": "2.6.0",
"react-router": "6.22.0",
"react-router-dom": "6.22.0"
"react-router": "0.0.0-experimental-cbcd94b7",
"react-router-dom": "0.0.0-experimental-cbcd94b7",
"turbo-stream": "^1.2.1"
},
"devDependencies": {
"@testing-library/jest-dom": "^5.17.0",
Expand Down
84 changes: 56 additions & 28 deletions packages/remix-react/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -196,7 +196,7 @@ export function createClientRoutesWithHMRRevalidationOptOut(
);
}

function preventInvalidServerHandlerCall(
export function preventInvalidServerHandlerCall(
type: "action" | "loader",
route: Omit<EntryRoute, "children">,
isSpaMode: boolean
Expand All @@ -221,7 +221,7 @@ function preventInvalidServerHandlerCall(
}
}

function noActionDefinedError(
export function noActionDefinedError(
type: "action" | "clientAction",
routeId: string
) {
Expand Down Expand Up @@ -249,16 +249,34 @@ export function createClientRoutes(
return (routesByParentId[parentId] || []).map((route) => {
let routeModule = routeModulesCache[route.id];

async function fetchServerLoader(request: Request) {
async function fetchServerLoader(
request: Request,
unwrap: boolean,
singleFetch: unknown
) {
if (!route.hasLoader) return null;
return fetchServerHandler(request, route);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead of always hitting the server here, if single fetch is enabled we will have passed the singleFetch method down from dataStrategy so use that to ensure we make the right HTTP call to the server

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By proxying this all the way though and branching here as deep as possible - we avoid having to re-implement any of the logic around critical/lazy routes, revalidation, client data etc. in data
strategy and get to leverage all of the existing battle-tested flows.

if (typeof singleFetch === "function") {
let result = await singleFetch();
return result;
}
let result = await fetchServerHandler(request, route);
return unwrap ? unwrapServerResponse(result) : result;
}

async function fetchServerAction(request: Request) {
async function fetchServerAction(
request: Request,
unwrap: boolean,
singleFetch: unknown
) {
if (!route.hasAction) {
throw noActionDefinedError("action", route.id);
}
return fetchServerHandler(request, route);
if (typeof singleFetch === "function") {
let result = await singleFetch();
return result;
}
let result = await fetchServerHandler(request, route);
return unwrap ? unwrapServerResponse(result) : result;
}

async function prefetchStylesAndCallHandler(
Expand Down Expand Up @@ -306,7 +324,10 @@ export function createClientRoutes(
needsRevalidation == null &&
(routeModule.clientLoader?.hydrate === true || !route.hasLoader);

dataRoute.loader = async ({ request, params }: LoaderFunctionArgs) => {
dataRoute.loader = async (
{ request, params }: LoaderFunctionArgs,
singleFetch?: unknown
) => {
try {
let result = await prefetchStylesAndCallHandler(async () => {
invariant(
Expand All @@ -316,7 +337,7 @@ export function createClientRoutes(
if (!routeModule.clientLoader) {
if (isSpaMode) return null;
// Call the server when no client loader exists
return fetchServerLoader(request);
return fetchServerLoader(request, false, singleFetch);
}

return routeModule.clientLoader({
Expand All @@ -334,9 +355,7 @@ export function createClientRoutes(
}

// Call the server loader for client-side navigations
let result = await fetchServerLoader(request);
let unwrapped = await unwrapServerResponse(result);
return unwrapped;
return fetchServerLoader(request, true, singleFetch);
},
});
});
Expand All @@ -355,7 +374,10 @@ export function createClientRoutes(
isSpaMode
);

dataRoute.action = ({ request, params }: ActionFunctionArgs) => {
dataRoute.action = (
{ request, params }: ActionFunctionArgs,
singleFetch?: unknown
) => {
return prefetchStylesAndCallHandler(async () => {
invariant(
routeModule,
Expand All @@ -365,17 +387,15 @@ export function createClientRoutes(
if (isSpaMode) {
throw noActionDefinedError("clientAction", route.id);
}
return fetchServerAction(request);
return fetchServerAction(request, false, singleFetch);
}

return routeModule.clientAction({
request,
params,
async serverAction() {
preventInvalidServerHandlerCall("action", route, isSpaMode);
let result = await fetchServerAction(request);
let unwrapped = await unwrapServerResponse(result);
return unwrapped;
return fetchServerAction(request, true, singleFetch);
},
});
});
Expand All @@ -385,19 +405,25 @@ export function createClientRoutes(
// the server loader/action in parallel with the module load so we add
// loader/action as static props on the route
if (!route.hasClientLoader) {
dataRoute.loader = ({ request }: LoaderFunctionArgs) =>
dataRoute.loader = (
{ request }: LoaderFunctionArgs,
singleFetch?: unknown
) =>
prefetchStylesAndCallHandler(() => {
if (isSpaMode) return Promise.resolve(null);
return fetchServerLoader(request);
return fetchServerLoader(request, false, singleFetch);
});
}
if (!route.hasClientAction) {
dataRoute.action = ({ request }: ActionFunctionArgs) =>
dataRoute.action = (
{ request }: ActionFunctionArgs,
singleFetch?: unknown
) =>
prefetchStylesAndCallHandler(() => {
if (isSpaMode) {
throw noActionDefinedError("clientAction", route.id);
}
return fetchServerAction(request);
return fetchServerAction(request, false, singleFetch);
});
}

Expand All @@ -411,28 +437,30 @@ export function createClientRoutes(
let lazyRoute: Partial<DataRouteObject> = { ...mod };
if (mod.clientLoader) {
let clientLoader = mod.clientLoader;
lazyRoute.loader = (args) =>
lazyRoute.loader = (
args: LoaderFunctionArgs,
singleFetch?: unknown
) =>
clientLoader({
...args,
async serverLoader() {
preventInvalidServerHandlerCall("loader", route, isSpaMode);
let response = await fetchServerLoader(args.request);
let result = await unwrapServerResponse(response);
return result;
return fetchServerLoader(args.request, true, singleFetch);
},
});
}

if (mod.clientAction) {
let clientAction = mod.clientAction;
lazyRoute.action = (args) =>
lazyRoute.action = (
args: ActionFunctionArgs,
singleFetch?: unknown
) =>
clientAction({
...args,
async serverAction() {
preventInvalidServerHandlerCall("action", route, isSpaMode);
let response = await fetchServerAction(args.request);
let result = await unwrapServerResponse(response);
return result;
return fetchServerAction(args.request, true, singleFetch);
},
});
}
Expand Down
Loading
Loading