-
-
Notifications
You must be signed in to change notification settings - Fork 10.3k
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(router): fix URL creation in Cloudflare Pages #9682
Changes from 2 commits
072311f
03a362a
0b6ade2
06f4674
d518c72
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,5 @@ | ||
--- | ||
"@remix-run/router": patch | ||
--- | ||
|
||
Fix URL creation in Cloudflare Pages or other non-browser-environment |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -447,6 +447,20 @@ export function createHashHistory( | |
//#region UTILS | ||
//////////////////////////////////////////////////////////////////////////////// | ||
|
||
/** | ||
* @private | ||
*/ | ||
export function invariant(value: boolean, message?: string): asserts value; | ||
export function invariant<T>( | ||
value: T | null | undefined, | ||
message?: string | ||
): asserts value is T; | ||
export function invariant(value: any, message?: string) { | ||
if (value === false || value === null || typeof value === "undefined") { | ||
throw new Error(message); | ||
} | ||
} | ||
|
||
function warning(cond: any, message: string) { | ||
if (!cond) { | ||
// eslint-disable-next-line no-console | ||
|
@@ -544,7 +558,7 @@ export function parsePath(path: string): Partial<Path> { | |
return parsedPath; | ||
} | ||
|
||
export function createURL(location: Location | string): URL { | ||
export function createClientSideURL(location: Location | string): URL { | ||
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. Make this more apparent that it's not intended to be used in |
||
// window.location.origin is "null" (the literal string value) in Firefox | ||
// under certain conditions, notably when serving from a local HTML file | ||
// See https://bugzilla.mozilla.org/show_bug.cgi?id=878297 | ||
|
@@ -553,9 +567,13 @@ export function createURL(location: Location | string): URL { | |
typeof window.location !== "undefined" && | ||
window.location.origin !== "null" | ||
? window.location.origin | ||
: "unknown://unknown"; | ||
: null; | ||
let href = typeof location === "string" ? location : createPath(location); | ||
return new URL(href, base); | ||
invariant( | ||
base, | ||
`No window.location.origin available to create URL for href: ${href}` | ||
); | ||
return new URL(href, window.location.origin); | ||
brophdawg11 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
export interface UrlHistory extends History {} | ||
|
@@ -643,7 +661,9 @@ function getUrlBasedHistory( | |
}, | ||
encodeLocation(to) { | ||
// Encode a Location the same way window.location would | ||
let url = createURL(typeof to === "string" ? to : createPath(to)); | ||
let url = createClientSideURL( | ||
typeof to === "string" ? to : createPath(to) | ||
); | ||
return { | ||
pathname: url.pathname, | ||
search: url.search, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,7 +3,8 @@ import { | |
Action as HistoryAction, | ||
createLocation, | ||
createPath, | ||
createURL, | ||
createClientSideURL, | ||
invariant, | ||
parsePath, | ||
} from "./history"; | ||
import type { | ||
|
@@ -28,7 +29,6 @@ import { | |
ResultType, | ||
convertRoutesToDataRoutes, | ||
getPathContributingMatches, | ||
invariant, | ||
isRouteErrorResponse, | ||
joinPaths, | ||
matchRoutes, | ||
|
@@ -913,7 +913,7 @@ export function createRouter(init: RouterInit): Router { | |
|
||
// Create a controller/Request for this navigation | ||
pendingNavigationController = new AbortController(); | ||
let request = createRequest( | ||
let request = createClientSideRequest( | ||
location, | ||
pendingNavigationController.signal, | ||
opts && opts.submission | ||
|
@@ -954,7 +954,7 @@ export function createRouter(init: RouterInit): Router { | |
loadingNavigation = navigation; | ||
|
||
// Create a GET request for the loaders | ||
request = createRequest(request.url, request.signal); | ||
request = new Request(request.url, { signal: request.signal }); | ||
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. When we already have a valid request, we can just inline the request creation. |
||
} | ||
|
||
// Call loaders | ||
|
@@ -1299,7 +1299,11 @@ export function createRouter(init: RouterInit): Router { | |
|
||
// Call the action for the fetcher | ||
let abortController = new AbortController(); | ||
let fetchRequest = createRequest(path, abortController.signal, submission); | ||
let fetchRequest = createClientSideRequest( | ||
path, | ||
abortController.signal, | ||
submission | ||
); | ||
fetchControllers.set(key, abortController); | ||
|
||
let actionResult = await callLoaderOrAction( | ||
|
@@ -1346,7 +1350,7 @@ export function createRouter(init: RouterInit): Router { | |
// Start the data load for current matches, or the next location if we're | ||
// in the middle of a navigation | ||
let nextLocation = state.navigation.location || state.location; | ||
let revalidationRequest = createRequest( | ||
let revalidationRequest = createClientSideRequest( | ||
nextLocation, | ||
abortController.signal | ||
); | ||
|
@@ -1501,7 +1505,7 @@ export function createRouter(init: RouterInit): Router { | |
|
||
// Call the loader for this fetcher route match | ||
let abortController = new AbortController(); | ||
let fetchRequest = createRequest(path, abortController.signal); | ||
let fetchRequest = createClientSideRequest(path, abortController.signal); | ||
fetchControllers.set(key, abortController); | ||
let result: DataResult = await callLoaderOrAction( | ||
"loader", | ||
|
@@ -1675,7 +1679,7 @@ export function createRouter(init: RouterInit): Router { | |
...fetchersToLoad.map(([, href, match, fetchMatches]) => | ||
callLoaderOrAction( | ||
"loader", | ||
createRequest(href, request.signal), | ||
createClientSideRequest(href, request.signal), | ||
match, | ||
fetchMatches, | ||
router.basename | ||
|
@@ -2120,7 +2124,7 @@ export function unstable_createStaticHandler( | |
if (!actionMatch.route.action) { | ||
let error = getInternalRouterError(405, { | ||
method: request.method, | ||
pathname: createURL(request.url).pathname, | ||
pathname: new URL(request.url).pathname, | ||
routeId: actionMatch.route.id, | ||
}); | ||
if (isRouteRequest) { | ||
|
@@ -2206,7 +2210,7 @@ export function unstable_createStaticHandler( | |
} | ||
|
||
// Create a GET request for the loaders | ||
let loaderRequest = createRequest(request.url, request.signal); | ||
let loaderRequest = new Request(request.url, { signal: request.signal }); | ||
let context = await loadRouteData(loaderRequest, matches); | ||
|
||
return { | ||
|
@@ -2240,7 +2244,7 @@ export function unstable_createStaticHandler( | |
if (isRouteRequest && !routeMatch?.route.loader) { | ||
throw getInternalRouterError(400, { | ||
method: request.method, | ||
pathname: createURL(request.url).pathname, | ||
pathname: new URL(request.url).pathname, | ||
routeId: routeMatch?.route.id, | ||
}); | ||
} | ||
|
@@ -2531,9 +2535,9 @@ function shouldRevalidateLoader( | |
isRevalidationRequired: boolean, | ||
actionResult: DataResult | undefined | ||
) { | ||
let currentUrl = createURL(currentLocation); | ||
let currentUrl = createClientSideURL(currentLocation); | ||
let currentParams = currentMatch.params; | ||
let nextUrl = createURL(location); | ||
let nextUrl = createClientSideURL(location); | ||
let nextParams = match.params; | ||
|
||
// This is the default implementation as to when we revalidate. If the route | ||
|
@@ -2624,16 +2628,22 @@ async function callLoaderOrAction( | |
); | ||
|
||
// Check if this an external redirect that goes to a new origin | ||
let external = createURL(location).origin !== createURL("/").origin; | ||
let currentUrl = new URL(request.url); | ||
let currentOrigin = currentUrl.origin; | ||
let newOrigin = new URL(location, currentOrigin).origin; | ||
let external = newOrigin !== currentOrigin; | ||
Comment on lines
-2627
to
+2634
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 is the main fix for cloudflare - we inline the URL creation and just use the current request to determine the origin |
||
|
||
// Support relative routing in internal redirects | ||
if (!external) { | ||
let activeMatches = matches.slice(0, matches.indexOf(match) + 1); | ||
let routePathnames = getPathContributingMatches(activeMatches).map( | ||
(match) => match.pathnameBase | ||
); | ||
let requestPath = createURL(request.url).pathname; | ||
let resolvedLocation = resolveTo(location, routePathnames, requestPath); | ||
let resolvedLocation = resolveTo( | ||
location, | ||
routePathnames, | ||
currentUrl.pathname | ||
); | ||
invariant( | ||
createPath(resolvedLocation), | ||
`Unable to resolve redirect location: ${location}` | ||
|
@@ -2713,12 +2723,15 @@ async function callLoaderOrAction( | |
return { type: ResultType.data, data: result }; | ||
} | ||
|
||
function createRequest( | ||
// Utility method for creating the Request instances for loaders/actions during | ||
// client-side navigations and fetches. During SSR we will always have a | ||
// Request instance from the static handler (query/queryRoute) | ||
function createClientSideRequest( | ||
location: string | Location, | ||
signal: AbortSignal, | ||
submission?: Submission | ||
): Request { | ||
let url = createURL(stripHashFromPath(location)).toString(); | ||
let url = createClientSideURL(stripHashFromPath(location)).toString(); | ||
let init: RequestInit = { signal }; | ||
|
||
if (submission) { | ||
|
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.
Moved from
utils.ts
to here to avoid a circular dependency