From 55280b8348a8a12cdabdd15eced6fa0aa2223a0c Mon Sep 17 00:00:00 2001 From: Roman Pominov Date: Tue, 29 Nov 2022 14:42:40 +0400 Subject: [PATCH] use RemixLink in Link (#488) Fixes #457 --- apps/designer/app/canvas/canvas.tsx | 2 +- .../app/canvas/custom-components/index.ts | 8 +++ .../app/canvas/custom-components/link.test.ts | 33 ++++++++++ .../app/canvas/custom-components/link.tsx | 66 +++++++++++++++++++ apps/designer/app/routes/$.tsx | 7 +- apps/designer/app/routes/s/css.ts | 6 +- .../app/shared/css-utils/generate-css-text.ts | 7 +- .../shared/router-utils/build-params.test.ts | 20 ++++-- .../app/shared/router-utils/build-params.ts | 63 +++++++----------- .../src/app/custom-components/index.ts | 4 ++ .../src/app/custom-components/link.tsx | 4 ++ .../app/custom-components/rich-text-link.tsx | 4 ++ .../custom-components/shared/remix-link.tsx | 40 +++++++++++ packages/react-sdk/src/components/link.tsx | 8 +-- .../src/components/rich-text-link.tsx | 13 ++-- packages/react-sdk/src/tree/root.ts | 4 +- 16 files changed, 218 insertions(+), 71 deletions(-) create mode 100644 apps/designer/app/canvas/custom-components/index.ts create mode 100644 apps/designer/app/canvas/custom-components/link.test.ts create mode 100644 apps/designer/app/canvas/custom-components/link.tsx create mode 100644 packages/react-sdk/src/app/custom-components/link.tsx create mode 100644 packages/react-sdk/src/app/custom-components/rich-text-link.tsx create mode 100644 packages/react-sdk/src/app/custom-components/shared/remix-link.tsx diff --git a/apps/designer/app/canvas/canvas.tsx b/apps/designer/app/canvas/canvas.tsx index 86f28792c2..b771e77ab0 100644 --- a/apps/designer/app/canvas/canvas.tsx +++ b/apps/designer/app/canvas/canvas.tsx @@ -6,7 +6,6 @@ import { useAllUserProps, registerComponents, registerComponentsMeta, - customComponents, customComponentsMeta, setParams, type OnChangeChildren, @@ -51,6 +50,7 @@ import { useSubscribeDesignerReady } from "./shared/use-designer-ready"; import type { Asset } from "@webstudio-is/asset-uploader"; import { useInstanceCopyPaste } from "~/shared/copy-paste"; import { useSelectedInstance } from "./shared/nano-states"; +import { customComponents } from "./custom-components"; registerContainers(); diff --git a/apps/designer/app/canvas/custom-components/index.ts b/apps/designer/app/canvas/custom-components/index.ts new file mode 100644 index 0000000000..e4cf7a2b3d --- /dev/null +++ b/apps/designer/app/canvas/custom-components/index.ts @@ -0,0 +1,8 @@ +import { customComponents as baseCustomComponents } from "@webstudio-is/react-sdk"; +import { wrapLinkComponent } from "./link"; + +export const customComponents = { + ...baseCustomComponents, + Link: wrapLinkComponent(baseCustomComponents.Link), + RichTextLink: wrapLinkComponent(baseCustomComponents.RichTextLink), +}; diff --git a/apps/designer/app/canvas/custom-components/link.test.ts b/apps/designer/app/canvas/custom-components/link.test.ts new file mode 100644 index 0000000000..6a65e3fae9 --- /dev/null +++ b/apps/designer/app/canvas/custom-components/link.test.ts @@ -0,0 +1,33 @@ +import { preserveBuildParams } from "./link"; + +describe("preserveBuildParams", () => { + test("absolute url is returned as is", () => { + const search = "?mode=preview&projectId=123"; + expect(preserveBuildParams("https://example.com", search)).toBe( + "https://example.com" + ); + expect(preserveBuildParams("mailto:alice@example.com", search)).toBe( + "mailto:alice@example.com" + ); + }); + + test("doesn't crash on a weird value", () => { + expect(preserveBuildParams("http://", "?projectId=123")).toBe("http://"); + }); + + test("projectId is preserved", () => { + expect(preserveBuildParams("/foo", "?projectId=123")).toBe( + "/foo?projectId=123" + ); + }); + + test("mode is preserved", () => { + expect(preserveBuildParams("/foo", "?mode=preview")).toBe( + "/foo?mode=preview" + ); + }); + + test("invalid mode is not preserved", () => { + expect(preserveBuildParams("/foo", "?mode=invalid")).toBe("/foo"); + }); +}); diff --git a/apps/designer/app/canvas/custom-components/link.tsx b/apps/designer/app/canvas/custom-components/link.tsx new file mode 100644 index 0000000000..bb43351339 --- /dev/null +++ b/apps/designer/app/canvas/custom-components/link.tsx @@ -0,0 +1,66 @@ +import { useLocation } from "@remix-run/react"; +import type { + ElementRef, + ComponentProps, + ForwardRefExoticComponent, + RefAttributes, +} from "react"; +import { forwardRef } from "react"; +import { preserveSearchBuildParams } from "~/shared/router-utils"; + +const isAbsoluteUrl = (href: string) => { + try { + new URL(href); + return true; + } catch (e) { + return false; + } +}; + +export const preserveBuildParams = (href: string, sourceSearch: string) => { + if (isAbsoluteUrl(href)) { + return href; + } + + let url; + let sourceSearchParams; + try { + url = new URL( + href, + // doesn't affect anything, just need some base to use URL + "http://localhost" + ); + sourceSearchParams = new URLSearchParams(sourceSearch); + + // eslint-disable-next-line no-empty + } catch (e) {} + + if (url === undefined || sourceSearchParams === undefined) { + return href; + } + + preserveSearchBuildParams(sourceSearchParams, url.searchParams); + + const search = url.searchParams.toString(); + + return `${url.pathname}${search === "" ? "" : `?${search}`}`; +}; + +type Props = Omit, "href"> & { href?: string }; + +type Ref = ElementRef<"a">; + +export const wrapLinkComponent = ( + BaseLink: ForwardRefExoticComponent> +) => { + const Component = forwardRef(({ href = "", ...props }, ref) => { + const { search } = useLocation(); + return ( + + ); + }); + + Component.displayName = BaseLink.displayName; + + return Component; +}; diff --git a/apps/designer/app/routes/$.tsx b/apps/designer/app/routes/$.tsx index 7fd42bbd59..4baf19e6f5 100644 --- a/apps/designer/app/routes/$.tsx +++ b/apps/designer/app/routes/$.tsx @@ -19,6 +19,7 @@ import { import { db } from "@webstudio-is/project/server"; import type { DynamicLinksFunction } from "remix-utils"; import type { CanvasData } from "@webstudio-is/project"; +import { customComponents } from "~/canvas/custom-components"; type Data = CanvasData & { env: Env; mode: BuildMode }; @@ -51,7 +52,7 @@ export const loader: LoaderFunction = async ({ request }): Promise => { throw redirect(dashboardPath()); } - const { mode, pathname } = buildParams; + const { mode } = buildParams; const project = await db.project.loadByParams(buildParams); @@ -62,7 +63,7 @@ export const loader: LoaderFunction = async ({ request }): Promise => { const canvasData = await loadCanvasData( project, mode === "published" ? "prod" : "dev", - pathname + "pageId" in buildParams ? buildParams.pageId : buildParams.pagePath ); if (canvasData === undefined) { @@ -90,7 +91,7 @@ const Content = () => { const Outlet = data.mode === "edit" ? () => - : () => ; + : () => ; // @todo This is non-standard for Remix, is there a better way? // Maybe there is a way to tell remix to use the right outlet somehow and avoid passing it? diff --git a/apps/designer/app/routes/s/css.ts b/apps/designer/app/routes/s/css.ts index fc172ecdd2..7d3a5e7f30 100644 --- a/apps/designer/app/routes/s/css.ts +++ b/apps/designer/app/routes/s/css.ts @@ -1,16 +1,16 @@ import { ActionFunction, json } from "@remix-run/node"; -import { BuildParams, getBuildParams } from "~/shared/router-utils"; +import { getBuildParams } from "~/shared/router-utils"; import env from "~/env.server"; import { sentryException } from "~/shared/sentry"; import { generateCssText } from "~/shared/css-utils"; export const loader: ActionFunction = async ({ request }) => { try { - const buildParams: BuildParams | undefined = getBuildParams(request); + const buildParams = getBuildParams(request); if (buildParams === undefined) { - throw json("Required project info", { status: 404 }); + throw json("Required project info", { status: 400 }); } const cssText = await generateCssText(buildParams); diff --git a/apps/designer/app/shared/css-utils/generate-css-text.ts b/apps/designer/app/shared/css-utils/generate-css-text.ts index 3f8d6dcec0..bdd3d88d2a 100644 --- a/apps/designer/app/shared/css-utils/generate-css-text.ts +++ b/apps/designer/app/shared/css-utils/generate-css-text.ts @@ -6,10 +6,7 @@ import { createCssEngine } from "@webstudio-is/css-engine"; import { idAttribute } from "@webstudio-is/react-sdk"; import type { BuildParams } from "../router-utils"; -export const generateCssText = async (buildParams: BuildParams | undefined) => { - if (buildParams === undefined) { - throw json("Required project info", { status: 404 }); - } +export const generateCssText = async (buildParams: BuildParams) => { const project = await db.project.loadByParams(buildParams); if (project === null) { @@ -19,7 +16,7 @@ export const generateCssText = async (buildParams: BuildParams | undefined) => { const canvasData = await loadCanvasData( project, buildParams.mode === "published" ? "prod" : "dev", - buildParams.pageId + "pageId" in buildParams ? buildParams.pageId : buildParams.pagePath ); if (canvasData === undefined) { diff --git a/apps/designer/app/shared/router-utils/build-params.test.ts b/apps/designer/app/shared/router-utils/build-params.test.ts index a3cbb81974..663a2a9238 100644 --- a/apps/designer/app/shared/router-utils/build-params.test.ts +++ b/apps/designer/app/shared/router-utils/build-params.test.ts @@ -47,7 +47,7 @@ describe("getBuildOrigin", () => { }); }); -describe("getCanvasRequestParams", () => { +describe("getBuildParams", () => { const env = { ...process.env, BUILD_ORIGIN: "https://foo.com", @@ -86,13 +86,25 @@ describe("getCanvasRequestParams", () => { test("detects path", () => { expect( - getBuildParams(makeRequest("https://bar.foo.com"), env)?.pathname + ( + getBuildParams(makeRequest("https://bar.foo.com"), env) as { + pagePath: string; + } + ).pagePath ).toBe("/"); expect( - getBuildParams(makeRequest("https://bar.foo.com/abc"), env)?.pathname + ( + getBuildParams(makeRequest("https://bar.foo.com/abc"), env) as { + pagePath: string; + } + ).pagePath ).toBe("/abc"); expect( - getBuildParams(makeRequest("https://bar.foo.com/abc/123"), env)?.pathname + ( + getBuildParams(makeRequest("https://bar.foo.com/abc/123"), env) as { + pagePath: string; + } + ).pagePath ).toBe("/abc/123"); }); diff --git a/apps/designer/app/shared/router-utils/build-params.ts b/apps/designer/app/shared/router-utils/build-params.ts index 6dc5ee1508..02e3061e46 100644 --- a/apps/designer/app/shared/router-utils/build-params.ts +++ b/apps/designer/app/shared/router-utils/build-params.ts @@ -2,19 +2,8 @@ // "Build" means user generated content — what user builds. export type BuildMode = "edit" | "preview" | "published"; -export type BuildParams = - | { - projectId: string; - mode: BuildMode; - pathname: string; - pageId?: string; - } - | { - projectDomain: string; - mode: BuildMode; - pathname: string; - pageId?: string; - }; +export type BuildParams = ({ pagePath: string } | { pageId: string }) & + ({ projectId: string } | { projectDomain: string }) & { mode: BuildMode }; // A subtype of Request. To make testing easier. type MinimalRequest = { @@ -71,20 +60,7 @@ export const getBuildOrigin = ( export const getBuildParams = ( request: MinimalRequest, env = process.env -): - | { - projectId: string; - mode: BuildMode; - pathname: string; - pageId?: string; - } - | { - projectDomain: string; - mode: BuildMode; - pathname: string; - pageId?: string; - } - | undefined => { +): BuildParams | undefined => { const url = new URL(request.url); const requestHost = getRequestHost(request); @@ -94,23 +70,32 @@ export const getBuildParams = ( if (env.BUILD_REQUIRE_SUBDOMAIN !== "true") { const projectId = url.searchParams.get("projectId"); if (projectId !== null && buildHost === requestHost) { - return { - projectId, - mode: getMode(url), - pathname: url.pathname, - pageId, - }; + return pageId === undefined + ? { projectId, mode: getMode(url), pagePath: url.pathname } + : { projectId, mode: getMode(url), pageId }; } } const [projectDomain, ...rest] = requestHost.split("."); const baseHost = rest.join("."); if (baseHost === buildHost) { - return { - projectDomain, - mode: getMode(url), - pathname: url.pathname, - pageId, - }; + return pageId === undefined + ? { projectDomain, mode: getMode(url), pagePath: url.pathname } + : { projectDomain, mode: getMode(url), pageId }; + } +}; + +export const preserveSearchBuildParams = ( + source: URLSearchParams, + target: URLSearchParams +): void => { + const mode = source.get("mode"); + if (typeof mode === "string" && modes.includes(mode as BuildMode)) { + target.set("mode", mode); + } + + const projectId = source.get("projectId"); + if (projectId) { + target.set("projectId", projectId); } }; diff --git a/packages/react-sdk/src/app/custom-components/index.ts b/packages/react-sdk/src/app/custom-components/index.ts index 1bba09b7f2..34159ba2d1 100644 --- a/packages/react-sdk/src/app/custom-components/index.ts +++ b/packages/react-sdk/src/app/custom-components/index.ts @@ -1,9 +1,13 @@ import { Image } from "./image"; +import { Link } from "./link"; +import { RichTextLink } from "./rich-text-link"; import { imageProps } from "@webstudio-is/image"; import type { MetaProps } from "../../components/component-type"; export const customComponents = { Image, + Link, + RichTextLink, }; export const customComponentsMeta = { diff --git a/packages/react-sdk/src/app/custom-components/link.tsx b/packages/react-sdk/src/app/custom-components/link.tsx new file mode 100644 index 0000000000..ca033384e4 --- /dev/null +++ b/packages/react-sdk/src/app/custom-components/link.tsx @@ -0,0 +1,4 @@ +import { Link as BaseLink } from "../../components/link"; +import { wrapLinkComponent } from "./shared/remix-link"; + +export const Link = wrapLinkComponent(BaseLink); diff --git a/packages/react-sdk/src/app/custom-components/rich-text-link.tsx b/packages/react-sdk/src/app/custom-components/rich-text-link.tsx new file mode 100644 index 0000000000..9902df3d03 --- /dev/null +++ b/packages/react-sdk/src/app/custom-components/rich-text-link.tsx @@ -0,0 +1,4 @@ +import { RichTextLink as BaseLink } from "../../components/rich-text-link"; +import { wrapLinkComponent } from "./shared/remix-link"; + +export const RichTextLink = wrapLinkComponent(BaseLink); diff --git a/packages/react-sdk/src/app/custom-components/shared/remix-link.tsx b/packages/react-sdk/src/app/custom-components/shared/remix-link.tsx new file mode 100644 index 0000000000..93ed9cd1a9 --- /dev/null +++ b/packages/react-sdk/src/app/custom-components/shared/remix-link.tsx @@ -0,0 +1,40 @@ +import { Link } from "@remix-run/react"; +import type { + ElementRef, + ComponentProps, + RefAttributes, + ForwardRefExoticComponent, +} from "react"; +import { forwardRef } from "react"; + +const isAbsoluteUrl = (href: string) => { + try { + new URL(href); + return true; + } catch (e) { + return false; + } +}; + +type Props = Omit, "href"> & { href?: string }; + +type Ref = ElementRef<"a">; + +export const wrapLinkComponent = ( + BaseLink: ForwardRefExoticComponent> +) => { + // We're not actually wrapping BaseLink (no way to wrap with Remix's Link), + // but this is still useful because we're making sure that props/ref types are compatible + const Component = forwardRef(({ href = "", ...props }, ref) => + isAbsoluteUrl(href) ? ( + + ) : ( + + ) + ); + + // This is the only part that we use from BaseLink at runtime + Component.displayName = BaseLink.displayName; + + return Component; +}; diff --git a/packages/react-sdk/src/components/link.tsx b/packages/react-sdk/src/components/link.tsx index 54f79167a9..622a8730c5 100644 --- a/packages/react-sdk/src/components/link.tsx +++ b/packages/react-sdk/src/components/link.tsx @@ -1,12 +1,8 @@ import { forwardRef, type ElementRef, type ComponentProps } from "react"; -const defaultTag = "a"; +type Props = Omit, "href"> & { href?: string }; -type LinkProps = Omit, "href"> & { - href?: string; -}; - -export const Link = forwardRef, LinkProps>( +export const Link = forwardRef, Props>( ({ href = "", ...props }, ref) => ); diff --git a/packages/react-sdk/src/components/rich-text-link.tsx b/packages/react-sdk/src/components/rich-text-link.tsx index 268bb99033..764e164faa 100644 --- a/packages/react-sdk/src/components/rich-text-link.tsx +++ b/packages/react-sdk/src/components/rich-text-link.tsx @@ -1,14 +1,9 @@ import { forwardRef, type ElementRef, type ComponentProps } from "react"; -const defaultTag = "a"; +type Props = Omit, "href"> & { href?: string }; -type RichTextLinkProps = Omit, "href"> & { - href?: string; -}; - -export const RichTextLink = forwardRef< - ElementRef, - RichTextLinkProps ->(({ href = "", ...props }, ref) => ); +export const RichTextLink = forwardRef, Props>( + ({ href = "", ...props }, ref) => +); RichTextLink.displayName = "RichTextLink"; diff --git a/packages/react-sdk/src/tree/root.ts b/packages/react-sdk/src/tree/root.ts index 7d146c57bf..12f6ef5c05 100644 --- a/packages/react-sdk/src/tree/root.ts +++ b/packages/react-sdk/src/tree/root.ts @@ -6,7 +6,7 @@ import type { Asset } from "@webstudio-is/asset-uploader"; import { type ComponentProps } from "react"; import type { Breakpoint } from "@webstudio-is/css-data"; import { registerComponents } from "../components"; -import { customComponents } from "../app/custom-components"; +import { customComponents as defaultCustomComponents } from "../app/custom-components"; import { setParams, type Params } from "../app/params"; export type Data = { @@ -20,11 +20,13 @@ export type Data = { type RootProps = { data: Data; Component?: (props: ComponentProps) => JSX.Element; + customComponents?: Parameters[0]; }; export const InstanceRoot = ({ data, Component, + customComponents = defaultCustomComponents, }: RootProps): JSX.Element | null => { if (data.tree === null) { throw new Error("Tree is null");