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

fix: make url-encoding history-aware #9496

Merged
merged 4 commits into from Oct 24, 2022

Conversation

brophdawg11
Copy link
Contributor

Forgot about this callout in the special character encoding PR: #9477 (comment)

We do not want to automatically encode incoming router.navigate() locations when using memory routers, so we introduce a history.encodeLocation method so encoding can be history aware.

Decoding proves to a be a bit trickier, since we decode in matchRoutes which is not history-aware, but since memory locations should never be encoded in the first place, I think it's safe to decode all the time in there, since decodeURI('/path') === '/path'

@changeset-bot
Copy link

changeset-bot bot commented Oct 24, 2022

🦋 Changeset detected

Latest commit: 68b2478

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 5 packages
Name Type
@remix-run/router Patch
react-router Patch
react-router-dom Patch
react-router-dom-v5-compat Patch
react-router-native Patch

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

@@ -2038,14 +2035,13 @@ export function unstable_createStaticHandler(
): Promise<Omit<StaticHandlerContext, "location"> | Response> {
let result: DataResult;
if (!actionMatch.route.action) {
let href = createServerHref(new URL(request.url));
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 createServerHref utility came over from Remix and was used in a few places that it didn't make sense. It's main purpose is to strip the hash from a URL because that doesn't get sent to the server, so really we want it in 2 places - (1) creating a submission and (2) creating a Request for a loader/action. So I removed the usages of createServerHref and instead use a more targeted stripHasFromPath utility in those 2 places.

search: url.search,
hash: url.hash,
};
location = init.history.encodeLocation(location);
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 is the main part of this PR - encode incoming router.navigate locations using the implementation provided by history

@@ -3059,13 +3048,8 @@ function getTargetMatch(
return pathMatches[pathMatches.length - 1];
}

function createURL(location: Location | string): URL {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

createURL moves over to history.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants