Rework query parsing

src/server/handlers/catch-all-handler.tsx

@@ -3,6 +3,7 @@ import { getHttpBaseInternal } from "@utils/env";
 import { ErrorPageData } from "@utils/types";
 import type { Request, Response } from "express";
 import { StaticRouter, matchPath } from "inferno-router";
+import { Match } from "inferno-router/dist/Route";
 import { renderToString } from "inferno-server";
 import { GetSiteResponse, LemmyHttp } from "lemmy-js-client";
 import { App } from "../../shared/components/app/app";
@@ -25,6 +26,8 @@ import {
   LanguageService,
   UserService,
 } from "../../shared/services/";
+import { parsePath } from "history";
+import { getQueryString } from "@utils/helpers";
 
 export default async (req: Request, res: Response) => {
   try {
@@ -40,7 +43,10 @@ export default async (req: Request, res: Response) => {
         .sort((a, b) => b.q - a.q)
         .map(x => (x.lang === "*" ? "en" : x.lang)) ?? [];
 
-    const activeRoute = routes.find(route => matchPath(req.path, route));
+    let match: Match<any> | null | undefined;
+    const activeRoute = routes.find(
+      route => (match = matchPath(req.path, route)),
+    );
 
     const headers = setForwardedHeaders(req.headers);
     const auth = getJwtCookie(req.headers);
@@ -49,7 +55,7 @@ export default async (req: Request, res: Response) => {
       new LemmyHttp(getHttpBaseInternal(), { headers }),
     );
 
-    const { path, url, query } = req;
+    const { path, url } = req;
 
     // Get site data first
     // This bypasses errors, so that the client can hit the error on its own,
@@ -71,7 +77,7 @@ export default async (req: Request, res: Response) => {
     }
 
     if (!auth && isAuthPath(path)) {
-      return res.redirect(`/login?prev=${encodeURIComponent(url)}`);
+      return res.redirect(`/login${getQueryString({ prev: url })}`);
     }
 
     if (try_site.state === "success") {
@@ -83,10 +89,12 @@ export default async (req: Request, res: Response) => {
         return res.redirect("/setup");
       }
 
-      if (site && activeRoute?.fetchInitialData) {
-        const initialFetchReq: InitialFetchRequest = {
+      if (site && activeRoute?.fetchInitialData && match) {
+        const { search } = parsePath(url);
+        const initialFetchReq: InitialFetchRequest<Record<string, any>> = {
           path,
-          query,
+          query: activeRoute.getQueryParams?.(search, site) ?? {},
+          match,
           site,
           headers,
         };

src/shared/components/app/app.tsx

@@ -49,7 +49,12 @@ export class App extends Component<any, any> {
             <div className="mt-4 p-0 fl-1">
               <Switch>
                 {routes.map(
-                  ({ path, component: RouteComponent, fetchInitialData }) => (
+                  ({
+                    path,
+                    component: RouteComponent,
+                    fetchInitialData,
+                    getQueryParams,
+                  }) => (
                     <Route
                       key={path}
                       path={path}
@@ -59,20 +64,34 @@ export class App extends Component<any, any> {
                           FirstLoadService.falsify();
                         }
 
+                        let queryProps = routeProps;
+                        if (getQueryParams && this.isoData.site_res) {
+                          // ErrorGuard will not render its children when
+                          // site_res is missing, this guarantees that props
+                          // will always contain the query params.
+                          queryProps = {
+                            ...routeProps,
+                            ...getQueryParams(
+                              routeProps.location.search,
+                              this.isoData.site_res,
+                            ),
+                          };
+                        }
+
                         return (
                           <ErrorGuard>
                             <div tabIndex={-1}>
                               {RouteComponent &&
                                 (isAuthPath(path ?? "") ? (
                                   <AuthGuard {...routeProps}>
-                                    <RouteComponent {...routeProps} />
+                                    <RouteComponent {...queryProps} />
                                   </AuthGuard>
                                 ) : isAnonymousPath(path ?? "") ? (
                                   <AnonymousGuard>
-                                    <RouteComponent {...routeProps} />
+                                    <RouteComponent {...queryProps} />
                                   </AnonymousGuard>
                                 ) : (
-                                  <RouteComponent {...routeProps} />
+                                  <RouteComponent {...queryProps} />
                                 ))}
                             </div>
                           </ErrorGuard>

src/shared/components/common/auth-guard.tsx

@@ -2,6 +2,7 @@ import { Component } from "inferno";
 import { RouteComponentProps } from "inferno-router/dist/Route";
 import { UserService } from "../../services";
 import { Spinner } from "./icon";
+import { getQueryString } from "@utils/helpers";
 
 interface AuthGuardState {
   hasRedirected: boolean;
@@ -26,7 +27,7 @@ class AuthGuard extends Component<
     if (!UserService.Instance.myUserInfo) {
       const { pathname, search } = this.props.location;
       this.context.router.history.replace(
-        `/login?prev=${encodeURIComponent(pathname + search)}`,
+        `/login${getQueryString({ prev: pathname + search })}`,
       );
     } else {
       this.setState({ hasRedirected: true });

src/shared/components/common/pictrs-image.tsx

@@ -68,28 +68,31 @@ export class PictrsImage extends Component<PictrsImageProps, any> {
     // sample url:
     // http://localhost:8535/pictrs/image/file.png?thumbnail=256&format=jpg
 
-    const split = this.props.src.split("/pictrs/image/");
+    let url: URL | undefined;
+    try {
+      url = new URL(this.props.src);
+    } catch {
+      return this.props.src;
+    }
 
-    // If theres not multiple, then its not a pictrs image
-    if (split.length === 1) {
+    // If theres no match, then its not a pictrs image
+    if (!url.pathname.includes("/pictrs/image/")) {
       return this.props.src;
     }
 
-    const host = split[0];
-    const path = split[1];
+    // Keeps original search params. Could probably do `url.search = ""` here.
 
-    const params = { format };
+    url.searchParams.set("format", format);
 
     if (this.props.thumbnail) {
-      params["thumbnail"] = thumbnailSize;
+      url.searchParams.set("thumbnail", thumbnailSize.toString());
     } else if (this.props.icon) {
-      params["thumbnail"] = iconThumbnailSize;
+      url.searchParams.set("thumbnail", iconThumbnailSize.toString());
+    } else {
+      url.searchParams.delete("thumbnail");
     }
 
-    const paramsStr = new URLSearchParams(params).toString();
-    const out = `${host}/pictrs/image/${path}?${paramsStr}`;
-
-    return out;
+    return url.href;
   }
 
   alt(): string {

src/shared/components/common/subscribe-button.tsx

@@ -1,4 +1,4 @@
-import { validInstanceTLD } from "@utils/helpers";
+import { getQueryString, validInstanceTLD } from "@utils/helpers";
 import classNames from "classnames";
 import { NoOptionI18nKeys } from "i18next";
 import { Component, MouseEventHandler, linkEvent } from "inferno";
@@ -134,8 +134,8 @@ function submitRemoteFollow(
     instanceText = `http${VERSION !== "dev" ? "s" : ""}://${instanceText}`;
   }
 
-  window.location.href = `${instanceText}/activitypub/externalInteraction?uri=${encodeURIComponent(
-    communityActorId,
+  window.location.href = `${instanceText}/activitypub/externalInteraction${getQueryString(
+    { uri: communityActorId },
   )}`;
 }
 

src/shared/components/community/communities.tsx

@@ -36,6 +36,8 @@ import { CommunityLink } from "./community-link";
 import { communityLimit } from "../../config";
 import { SubscribeButton } from "../common/subscribe-button";
 import { getHttpBaseInternal } from "../../utils/env";
+import { RouteComponentProps } from "inferno-router/dist/Route";
+import { IRoutePropsWithFetch } from "../../routes";
 
 type CommunitiesData = RouteDataResponse<{
   listCommunitiesResponse: ListCommunitiesResponse;
@@ -62,15 +64,30 @@ function getSortTypeFromQuery(type?: string): SortType {
   return type ? (type as SortType) : "TopMonth";
 }
 
-function getCommunitiesQueryParams() {
-  return getQueryParams<CommunitiesProps>({
-    listingType: getListingTypeFromQuery,
-    sort: getSortTypeFromQuery,
-    page: getPageFromString,
-  });
+export function getCommunitiesQueryParams(source?: string): CommunitiesProps {
+  return getQueryParams<CommunitiesProps>(
+    {
+      listingType: getListingTypeFromQuery,
+      sort: getSortTypeFromQuery,
+      page: getPageFromString,
+    },
+    source,
+  );
 }
 
-export class Communities extends Component<any, CommunitiesState> {
+type CommunitiesPathProps = Record<string, never>;
+type CommunitiesRouteProps = RouteComponentProps<CommunitiesPathProps> &
+  CommunitiesProps;
+export type CommunitiesFetchConfig = IRoutePropsWithFetch<
+  CommunitiesData,
+  CommunitiesPathProps,
+  CommunitiesProps
+>;
+
+export class Communities extends Component<
+  CommunitiesRouteProps,
+  CommunitiesState
+> {
   private isoData = setIsoData<CommunitiesData>(this.context);
   state: CommunitiesState = {
     listCommunitiesResponse: EMPTY_REQUEST,
@@ -79,7 +96,7 @@ export class Communities extends Component<any, CommunitiesState> {
     isIsomorphic: false,
   };
 
-  constructor(props: any, context: any) {
+  constructor(props: CommunitiesRouteProps, context: any) {
     super(props, context);
     this.handlePageChange = this.handlePageChange.bind(this);
     this.handleSortChange = this.handleSortChange.bind(this);
@@ -118,7 +135,7 @@ export class Communities extends Component<any, CommunitiesState> {
           </h5>
         );
       case "success": {
-        const { listingType, sort, page } = getCommunitiesQueryParams();
+        const { listingType, sort, page } = this.props;
         return (
           <div>
             <h1 className="h4 mb-4">
@@ -268,7 +285,7 @@ export class Communities extends Component<any, CommunitiesState> {
       listingType: urlListingType,
       sort: urlSort,
       page: urlPage,
-    } = getCommunitiesQueryParams();
+    } = this.props;
 
     const queryParams: QueryParams<CommunitiesProps> = {
       listingType: listingType ?? urlListingType,
@@ -302,27 +319,28 @@ export class Communities extends Component<any, CommunitiesState> {
 
   handleSearchSubmit(i: Communities, event: any) {
     event.preventDefault();
-    const searchParamEncoded = encodeURIComponent(i.state.searchText);
-    const { listingType } = getCommunitiesQueryParams();
+    const searchParamEncoded = i.state.searchText;
+    const { listingType } = i.props;
     i.context.router.history.push(
-      `/search?q=${searchParamEncoded}&type=Communities&listingType=${listingType}`,
+      `/search${getQueryString({ q: searchParamEncoded, type: "Communities", listingType })}`,
     );
   }
 
   static async fetchInitialData({
     headers,
     query: { listingType, sort, page },
   }: InitialFetchRequest<
-    QueryParams<CommunitiesProps>
+    CommunitiesPathProps,
+    CommunitiesProps
   >): Promise<CommunitiesData> {
     const client = wrapClient(
       new LemmyHttp(getHttpBaseInternal(), { headers }),
     );
     const listCommunitiesForm: ListCommunities = {
-      type_: getListingTypeFromQuery(listingType),
-      sort: getSortTypeFromQuery(sort),
+      type_: listingType,
+      sort,
       limit: communityLimit,
-      page: getPageFromString(page),
+      page,
     };
 
     return {
@@ -346,7 +364,7 @@ export class Communities extends Component<any, CommunitiesState> {
   async refetch() {
     this.setState({ listCommunitiesResponse: LOADING_REQUEST });
 
-    const { listingType, sort, page } = getCommunitiesQueryParams();
+    const { listingType, sort, page } = this.props;
 
     this.setState({
       listCommunitiesResponse: await HttpService.client.listCommunities({