fix(router): fix URL creation in Cloudflare Pages #9682
Conversation
🦋 Changeset detectedLatest commit: d518c72 The changes in this PR will be included in the next version bump. This PR includes changesets to release 5 packages
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
brophdawg11
force-pushed
the
brophdawg11/fix-cloudflare-url
branch
from
37bda28
to
03a362a
Compare
| if (value === false || value === null || typeof value === "undefined") { | ||
| throw new Error(message); | ||
| } | ||
| } |
There was a problem hiding this comment.
Moved from utils.ts to here to avoid a circular dependency
MichaelDeBoey
changed the title
| @@ -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.
Make this more apparent that it's not intended to be used in staticHandler
| @@ -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.
When we already have a valid request, we can just inline the request creation. createClientSideRequest is now for when we don't have a request and we just have a path
| 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; |
There was a problem hiding this comment.
This is the main fix for cloudflare - we inline the URL creation and just use the current request to determine the origin
| : "unknown://unknown"; | ||
| : window.location.href; | ||
| let href = typeof location === "string" ? location : createPath(location); | ||
| invariant( | ||
| base, | ||
| `No window.location.(origin|href) available to create URL for href: ${href}` | ||
| ); | ||
| return new URL(href, base); |
There was a problem hiding this comment.
The unknown://unknown was a bit of a snap decision very early on, but I think was really a red herring in some unit testing setup issues since new URL('/', 'unknown://unknown') doesn't actually work quite right anyway! Now that we've better defined these APIs as intended only for client-side use - we can just use invariant if we can't detect a proper base.
For the specific Firefox file:// case, unknown://unknown wasn't actually working right and gave incorrect paths - but it didn't throw an error. Instead, firefox does have a "valid" window.location.href we can use as a fallback base when origin = "null" (in file:// mode):
We ran into an issue in Remix 1.8.0 where the static handler was failing to create a
new URL()in Cloudflare pages. This is because we ended up trying to share some code to createURLinstances that should have been client-only in the static handler. This PR updates our code to prefer creatingURLs fromRequests (which will always have origins) where possible vianew URL(request.url)and makes it more explicit where we need to create client-sideURL/Requestinstances.Closes remix-run/remix#4740