From 02f59e0c6feba1b0fefbd37df61c2b02bd409be9 Mon Sep 17 00:00:00 2001 From: Romain Gauthier Date: Sun, 23 Aug 2026 21:41:10 +0200 Subject: [PATCH 1/2] feat(library)!: JLink never blocks the caller, and the edit-mode URL is correct `attributes` takes a record or a function of the resolved link, so a value derived from the URL and the label the component computed is reachable at last; `asChild` hands the link to a wrapper that is not a bare ``; and the anchor attributes the component accepts are derived from its own props rather than hand-listed. `labelProperties` and `labelFrom` say where the label of a link mixin lives, `state.node` returns what the link resolved to, `readNodeReference` exports the safe reference read behind it as a JCR concern, and `allowedSchemes` / `setLinkDefaults` narrow the scheme allow-list per call or per module. `buildNodeUrl` emits `/cms/editframe/` for edit mode: `/cms/edit/` redirects to the jContent UI on 8.2.3 and only ever reached a page because EditModeFilter substitutes the two for an `a[href]`. --- .chachalog/lnk4Pw2Hs.md | 12 ++ .../src/components/JLink.spec.tsx | 159 ++++++++++++++ .../src/components/JLink.tsx | 204 ++++++++++++++---- javascript-modules-library/src/index.ts | 10 +- .../src/utils/jcr/readNodeReference.spec.ts | 64 ++++++ .../src/utils/jcr/readNodeReference.ts | 61 ++++++ .../src/utils/link/devWarnings.ts | 57 +++++ .../src/utils/link/getLinkProps.ts | 33 ++- .../src/utils/link/link.spec.ts | 196 ++++++++++++++++- .../src/utils/link/linkDefaults.ts | 70 ++++++ .../src/utils/link/resolveContentLink.ts | 73 ++++--- .../src/utils/link/types.ts | 19 ++ .../src/utils/urlBuilder/urlBuilder.spec.ts | 65 ++++++ .../src/utils/urlBuilder/urlBuilder.ts | 5 +- 14 files changed, 944 insertions(+), 84 deletions(-) create mode 100644 .chachalog/lnk4Pw2Hs.md create mode 100644 javascript-modules-library/src/utils/jcr/readNodeReference.spec.ts create mode 100644 javascript-modules-library/src/utils/jcr/readNodeReference.ts create mode 100644 javascript-modules-library/src/utils/link/devWarnings.ts create mode 100644 javascript-modules-library/src/utils/link/linkDefaults.ts create mode 100644 javascript-modules-library/src/utils/urlBuilder/urlBuilder.spec.ts diff --git a/.chachalog/lnk4Pw2Hs.md b/.chachalog/lnk4Pw2Hs.md new file mode 100644 index 00000000..41e4853d --- /dev/null +++ b/.chachalog/lnk4Pw2Hs.md @@ -0,0 +1,12 @@ +--- +# Allowed version bumps: patch, minor, major +javascript-modules: minor +--- + +Made the link API usable on a real site: an open `attributes` map, `asChild`, the label of a link mixin, a narrowable scheme allow-list, and an edit-mode URL that is correct wherever it is put. (#768, #769, #770, #771, #772) + +`` no longer pushes a caller off the component. `attributes` takes a record or a function of the resolved link — `({ anchor, state }) => ({ "data-element-url": anchor.href, "data-element-text": state.label })` — which is the only way to reach `data-*` and the only way to read back the URL and the label the component computed. `asChild` hands the link to the element you render, for a call to action that is not a bare ``. The anchor attributes the component accepts are now derived from its own props rather than hand-listed, so a prop added later cannot silently swallow one. + +A link mixin sits on a node whose `jcr:title` is the heading, not the link label; `labelProperties` and `labelFrom` say where the label really lives. `state.node` returns what the link resolved to, and the safe reference read behind it is exported as `readNodeReference`, next to `getNodeProps` — reading a `weakreference` without letting a dangling one break the render is a JCR concern, not a link one. The scheme allow-list can be narrowed, per call with `allowedSchemes` or once per module with `setLinkDefaults`; it narrows only, and says so on a development instance. + +`buildNodeUrl` now emits `/cms/editframe/…` for edit mode. `/cms/edit/…` does not render a page on 8.2.3 — it redirects to the jContent UI — and only reached one because `EditModeFilter` substitutes the two for an `a[href]` and nothing else, so the same URL in an Island payload or a `data-*` attribute pointed at a second copy of jContent. The [Links guide](https://github.com/Jahia/javascript-modules/blob/main/docs/2-guides/9-links/README.md) now states which contexts core finishes a URL in and which it leaves alone. diff --git a/javascript-modules-library/src/components/JLink.spec.tsx b/javascript-modules-library/src/components/JLink.spec.tsx index 1e20492c..d04e4183 100644 --- a/javascript-modules-library/src/components/JLink.spec.tsx +++ b/javascript-modules-library/src/components/JLink.spec.tsx @@ -152,3 +152,162 @@ describe("a link that is not navigable", () => { expect(markup).toBe("Later"); }); }); + +describe("attributes the component's own props cannot express", () => { + const node = jcrNode({ identifier: "u-other", path: "/sites/test/other" }); + + it("spreads a record onto the anchor", () => { + const markup = render( + + Other + , + ); + expect(markup).toBe( + 'Other', + ); + }); + + it("hands the resolved link to the function form, href and label together", () => { + const markup = render( + ({ + "data-element-url": anchor.href, + "data-element-text": state.label, + "data-element-current": state.isCurrent, + })} + />, + ); + expect(markup).toBe( + 'Home', + ); + }); + + it("drops the keys whose value is undefined, rather than rendering them empty", () => { + const markup = render( + + Other + , + ); + expect(markup).toBe('Other'); + }); + + it("wins over an anchor attribute of the same name, being spread last", () => { + const markup = render( + + Other + , + ); + expect(markup).toBe('Other'); + }); + + it("is not called at all when the link is not navigable", () => { + const attributes = vi.fn(() => ({ "data-x": "1" })); + expect( + render( + + Later + , + ), + ).toBe("Later"); + expect(attributes).not.toHaveBeenCalled(); + }); +}); + +describe("asChild", () => { + const node = jcrNode({ identifier: "u-other", path: "/sites/test/other" }); + /** A design system's call to action: not a bare anchor, and the reason asChild exists. */ + const CTA = ({ variant, ...rest }: { variant: string } & Record) => ( + + ); + + it("hands the anchor attributes to the element the caller rendered", () => { + const markup = render( + + Read more + , + ); + expect(markup).toBe('Read more'); + }); + + it("carries aria-current and the extra attributes through the child too", () => { + const markup = render( + ({ "data-element-url": anchor.href })} + > + Home + , + ); + expect(markup).toContain('aria-current="page"'); + expect(markup).toContain('data-element-url="/sites/test/home.html"'); + }); + + it("still renders the child, without a link, when the target does not resolve", () => { + const markup = render( + + Coming soon + , + ); + expect(markup).toBe('Coming soon'); + expect(markup).not.toContain("href"); + }); + + it("renders nothing when the caller asks for nothing", () => { + const markup = render( + + Coming soon + , + ); + expect(markup).toBe(""); + }); + + it("says what is wrong when it is given anything but one element", () => { + for (const children of [ + undefined, + "just text", + [, ], + ]) { + expect(() => + render( + + {children} + , + ), + ).toThrow("asChild renders the child as the link"); + } + }); +}); + +describe("the anchor attributes JLink accepts", () => { + it("still passes through the ones its own props do not claim", () => { + const markup = render( + + Report + , + ); + expect(markup).toContain('media="print"'); + expect(markup).toContain('referrerPolicy="no-referrer"'); + }); + + it("keeps every prop it consumes off the DOM, derived from the props themselves", () => { + const markup = render( + + Partner + , + ); + expect(markup).toBe('Partner'); + }); +}); diff --git a/javascript-modules-library/src/components/JLink.tsx b/javascript-modules-library/src/components/JLink.tsx index 70814098..9d13d53f 100644 --- a/javascript-modules-library/src/components/JLink.tsx +++ b/javascript-modules-library/src/components/JLink.tsx @@ -1,24 +1,43 @@ -import type { AnchorHTMLAttributes, JSX, ReactNode } from "react"; +import { + Children, + cloneElement, + isValidElement, + type AnchorHTMLAttributes, + type JSX, + type ReactElement, + type ReactNode, +} from "react"; import type { JCRNodeWrapper } from "org.jahia.services.content"; import { useServerContext } from "../hooks/useServerContext.js"; import { getLinkProps } from "../utils/link/getLinkProps.js"; -import { resolveContentLink } from "../utils/link/resolveContentLink.js"; -import type { LinkOptions } from "../utils/link/types.js"; +import { resolveContentLink, type LinkLabelSource } from "../utils/link/resolveContentLink.js"; +import type { LinkOptions, LinkProps } from "../utils/link/types.js"; -/** Everything the three shapes of `` have in common. */ -type JLinkCommon = { - /** The content of the anchor. With none, the derived label is rendered instead. */ - children?: ReactNode; - /** - * What to render when the link is not navigable — an unresolved reference, a rejected URL, a - * missing translation. `"children"` renders the children without an anchor, `"none"` renders - * nothing at all. - * - * @default "children" - */ - whenUnresolved?: "children" | "none"; -} & Pick< +/** + * Attributes spread onto the anchor last, after everything the component computed. + * + * A record covers the static ones — which a plain JSX attribute already expresses. The function + * form is for a value that depends on the link the library resolved: an analytics attribute + * carrying the final URL and the derived label, both of which the component computes and a call + * site otherwise has no way to read back. + * + * It receives exactly what {@link getLinkProps} returns, so the same callback works on both tiers. + */ +export type ExtraAnchorAttributes = + | Record + | ((link: LinkProps) => Record); + +/** + * What `JLink` decides for itself. + * + * The HTML attributes it accepts are everything an `` takes _minus these_ — derived, not + * hand-listed, so adding a prop here can never silently swallow an attribute that used to reach the + * element. `content` is in the list because it is both React's RDFa attribute and this component's + * own prop name. + */ +interface JLinkOwnProps extends Pick< LinkOptions, + | "allowedSchemes" | "parameters" | "hash" | "language" @@ -28,12 +47,35 @@ type JLinkCommon = { | "title" | "isCurrent" | "cacheDependency" -> & - // `content` is React's RDFa attribute, and it is this component's own prop name - Omit, "href" | "target" | "rel" | "title" | "content">; - -/** How {@link resolveContentLink} reads the link off the `content` node. */ -type JLinkDiscriminator = { +> { + /** The node to link to. `null` and `undefined` are ordinary: the link is not navigable. */ + node?: JCRNodeWrapper | null; + /** A content node carrying a link — a `jnt:nodeLink`, a `jnt:externalLink`, a CTA mixin. */ + content?: JCRNodeWrapper; + /** An already-built URL. Goes through the scheme allow-list like any other string. */ + href?: string; + /** The content of the anchor. With none, the derived label is rendered instead. */ + children?: ReactNode; + /** + * What to render when the link is not navigable — an unresolved reference, a rejected URL, a + * missing translation. `"children"` renders the children without an anchor, `"none"` renders + * nothing at all. + * + * @default "children" + */ + whenUnresolved?: "children" | "none"; + /** Spread onto the anchor last. */ + attributes?: ExtraAnchorAttributes; + /** + * Render the single child element as the link instead of wrapping it in an ``: the anchor + * attributes are passed to it, and the element it renders is the one that carries the `href`. + * + * For a design system whose call to action is not a bare anchor. Next.js met the same need and + * called it `passHref`. + * + * @default false + */ + asChild?: boolean; /** * Property saying which kind of link the content node carries. Only its "no link" value is read. * luxe uses `"ctaType"`, se-utils `"seu:linkType"`. @@ -60,7 +102,34 @@ type JLinkDiscriminator = { * @default "j:url" */ urlProperty?: string; -}; + /** + * Properties holding the label of the link itself, tried in order. Name your own when the link is + * a mixin on a node whose `jcr:title` is the heading rather than the link label. + * + * @default ["jcr:title", "j:linkTitle"] + */ + labelProperties?: readonly string[]; + /** + * Where the label comes from. `"target"` skips the content node and uses the displayable name of + * whatever the link points at. + * + * @default "content" + */ + labelFrom?: LinkLabelSource; +} + +/** The keys of the discriminator, which only the `content` shape accepts. */ +type JLinkDiscriminatorKey = + | "typeProperty" + | "noneValue" + | "referenceProperties" + | "urlProperty" + | "labelProperties" + | "labelFrom"; + +/** Everything the three shapes of `` have in common. */ +type JLinkCommon = Omit & + Omit, keyof JLinkOwnProps>; /** * A discriminated union, so that naming two targets, or none, is a type error rather than prose. @@ -75,24 +144,51 @@ export type JLinkProps = node: JCRNodeWrapper | null | undefined; content?: never; href?: never; - } & JLinkCommon & { [K in keyof JLinkDiscriminator]?: never }) + } & JLinkCommon & { [K in JLinkDiscriminatorKey]?: never }) | ({ /** A content node carrying a link — a `jnt:nodeLink`, a `jnt:externalLink`, a CTA mixin. */ content: JCRNodeWrapper; node?: never; href?: never; } & JLinkCommon & - JLinkDiscriminator) + Pick) | ({ /** An already-built URL. Goes through the scheme allow-list like any other string. */ href: string; node?: never; content?: never; - } & JLinkCommon & { [K in keyof JLinkDiscriminator]?: never } & ( + } & JLinkCommon & { [K in JLinkDiscriminatorKey]?: never } & ( | { children: ReactNode } | { "aria-label": string } )); +/** + * The single element `asChild` hands the link to. + * + * The `Children` helpers are the only way to count children without assuming their shape, and + * `asChild` is by definition the feature that clones one: there is no alternative form of "render + * the caller's element as the link". + */ +/* eslint-disable @eslint-react/no-children-count, @eslint-react/no-children-to-array */ +function onlyElement(children: ReactNode): ReactElement> { + // `Children.only` throws React's own message on a text child, which names neither this component + // nor the way out of the mistake + const child = isValidElement(children) + ? children + : Children.count(children) === 1 + ? Children.toArray(children)[0] + : undefined; + if (!isValidElement(child)) { + throw new Error( + "JLink: asChild renders the child as the link, so it needs exactly one element child. " + + "Drop asChild to have JLink render the itself.", + ); + } + + return child as ReactElement>; +} +/* eslint-enable @eslint-react/no-children-count, @eslint-react/no-children-to-array */ + /** * Renders a link as a bare ``: the URL, the validated `target` and its `rel`, `aria-current` on * the page being rendered, and a render cache dependency on the target. @@ -102,7 +198,9 @@ export type JLinkProps = * the children on their own instead, or nothing when `whenUnresolved` says so. * * The element carries no styling of its own: pass a `className`. Every other anchor attribute — - * `onClick`, `hreflang`, `download` — is passed through. + * `onClick`, `hreflang`, `download` — is passed through, and `attributes` covers the ones React's + * typings cannot model, `data-*` among them. Where the wrapper is not an anchor at all, `asChild` + * hands the link to the element you render. * * Server-side only, because it registers the cache dependency. A client component receives link * data instead: build it with {@link getLinkProps} and spread it, ``. @@ -112,9 +210,15 @@ export type JLinkProps = * {label} * * + * ({ + * "data-element-url": anchor.href, + * "data-element-text": state.label, + * })} /> + * Read more * ```; * - * @returns The `` element, or the unwrapped children when the link is not navigable. + * @returns The `` element, the child `asChild` was given, or the unwrapped children when the + * link is not navigable. * @see {@link getLinkProps} for the semantics of every option. */ export function JLink(props: JLinkProps): JSX.Element | null { @@ -124,6 +228,9 @@ export function JLink(props: JLinkProps): JSX.Element | null { href, children, whenUnresolved = "children", + attributes, + asChild = false, + allowedSchemes, parameters, hash, language, @@ -137,6 +244,8 @@ export function JLink(props: JLinkProps): JSX.Element | null { noneValue, referenceProperties, urlProperty, + labelProperties, + labelFrom, // Whatever is left is an anchor attribute: everything this component consumes is named above, // so that a link option added later cannot reach the DOM. ...anchorAttributes @@ -144,6 +253,7 @@ export function JLink(props: JLinkProps): JSX.Element | null { const context = useServerContext(); const options: LinkOptions = { + allowedSchemes, parameters, hash, language, @@ -158,7 +268,15 @@ export function JLink(props: JLinkProps): JSX.Element | null { const link = content ? resolveContentLink( content, - { ...options, typeProperty, noneValue, referenceProperties, urlProperty }, + { + ...options, + typeProperty, + noneValue, + referenceProperties, + urlProperty, + labelProperties, + labelFrom, + }, context, ) : getLinkProps(href ?? node, options, context); @@ -167,17 +285,25 @@ export function JLink(props: JLinkProps): JSX.Element | null { const body = children ?? link?.state.label; if (!link?.state.navigable) { - return whenUnresolved === "none" ? null : <>{body}; + if (whenUnresolved === "none") return null; + // `asChild` still renders its child, just not as a link: a call to action that lost its target + // is a call to action with no href, not a hole in the page + return asChild ? onlyElement(children) : <>{body}; } - return ( - // `state` is never spread: none of its keys is an anchor attribute - - {body} - + // `state` is never spread: none of its keys is an anchor attribute + const linkAttributes = { + ...link.anchor, + "aria-current": link.state.isCurrent ? ("page" as const) : undefined, + ...anchorAttributes, + ...(typeof attributes === "function" ? attributes(link) : attributes), + }; + + return asChild ? ( + // The whole point of asChild: the caller's element becomes the link + // eslint-disable-next-line @eslint-react/no-clone-element + cloneElement(onlyElement(children), linkAttributes) + ) : ( + {body} ); } diff --git a/javascript-modules-library/src/index.ts b/javascript-modules-library/src/index.ts index b1ea5353..ef10f2c0 100644 --- a/javascript-modules-library/src/index.ts +++ b/javascript-modules-library/src/index.ts @@ -16,7 +16,7 @@ export { type JImageProps, type MarkupBox, } from "./components/JImage.js"; -export { JLink, type JLinkProps } from "./components/JLink.js"; +export { JLink, type ExtraAnchorAttributes, type JLinkProps } from "./components/JLink.js"; // Declaration and registration export { jahiaComponent } from "./framework/jahiaComponent.js"; @@ -30,6 +30,7 @@ export { useServerContext, ServerContextProvider } from "./hooks/useServerContex export { getChildNodes } from "./utils/jcr/getChildNodes.js"; export { getNodeProps } from "./utils/jcr/getNodeProps.js"; export { getNodesByJCRQuery } from "./utils/jcr/getNodesByJCRQuery.js"; +export { readNodeReference, type NodeReference } from "./utils/jcr/readNodeReference.js"; // URL builder export { @@ -71,7 +72,12 @@ export { readImageMeta, type ImageMeta } from "./utils/image/imageMeta.js"; // Links export { getLinkProps } from "./utils/link/getLinkProps.js"; -export { resolveContentLink } from "./utils/link/resolveContentLink.js"; +export { + setLinkDefaults, + DEFAULT_ALLOWED_SCHEMES, + type LinkDefaults, +} from "./utils/link/linkDefaults.js"; +export { resolveContentLink, type LinkLabelSource } from "./utils/link/resolveContentLink.js"; export type { AnchorProps, LinkContext, diff --git a/javascript-modules-library/src/utils/jcr/readNodeReference.spec.ts b/javascript-modules-library/src/utils/jcr/readNodeReference.spec.ts new file mode 100644 index 00000000..80625921 --- /dev/null +++ b/javascript-modules-library/src/utils/jcr/readNodeReference.spec.ts @@ -0,0 +1,64 @@ +import { describe, expect, it } from "vitest"; +import type { JCRNodeWrapper } from "org.jahia.services.content"; +import { readNodeReference } from "./readNodeReference.js"; + +const target = { getPath: () => "/sites/test/news" } as unknown as JCRNodeWrapper; + +/** + * A node carrying reference properties, each described by the UUID it stores and — when it resolves + * — the node behind it. + */ +const jcrNode = (references: Record) => + ({ + hasProperty: (property: string) => property in references, + getPropertyAsString: (property: string) => references[property]?.uuid ?? null, + getProperty: (property: string) => ({ + getValue: () => ({ + getNode: () => { + // An unresolvable reference throws, it does not return null + if (!references[property].target) throw new Error("ItemNotFoundException"); + return references[property].target; + }, + }), + }), + }) as unknown as JCRNodeWrapper; + +describe("readNodeReference", () => { + it("returns both ends of a reference that resolves", () => { + const node = jcrNode({ "example:related": { uuid: "u-news", target } }); + expect(readNodeReference(node, "example:related")).toEqual({ node: target, uuid: "u-news" }); + }); + + it("separates an unset property from one whose target is gone", () => { + const unset = jcrNode({}); + expect(readNodeReference(unset, "example:related")).toBeNull(); + + const dangling = jcrNode({ "example:related": { uuid: "u-draft" } }); + expect(readNodeReference(dangling, "example:related")).toEqual({ + node: undefined, + uuid: "u-draft", + }); + }); + + it("treats an empty or whitespace-only value as unset", () => { + for (const uuid of ["", " ", "\t"]) { + expect(readNodeReference(jcrNode({ "example:related": { uuid } }), "example:related")).toBe( + null, + ); + } + }); + + it("absorbs a node that cannot answer at all", () => { + const dead = new Proxy({} as JCRNodeWrapper, { + get: () => () => { + throw new Error("RepositoryException"); + }, + }); + expect(() => readNodeReference(dead, "example:related")).not.toThrow(); + expect(readNodeReference(dead, "example:related")).toBeNull(); + }); + + it("answers for a missing node rather than throwing on it", () => { + expect(readNodeReference(undefined as unknown as JCRNodeWrapper, "example:related")).toBeNull(); + }); +}); diff --git a/javascript-modules-library/src/utils/jcr/readNodeReference.ts b/javascript-modules-library/src/utils/jcr/readNodeReference.ts new file mode 100644 index 00000000..f485ab4c --- /dev/null +++ b/javascript-modules-library/src/utils/jcr/readNodeReference.ts @@ -0,0 +1,61 @@ +import type { JCRNodeWrapper } from "org.jahia.services.content"; + +/** + * Both ends of a reference property: the node it resolves to, and the raw value it stores. + * + * The two are independent. A property that is set but whose target the visitor cannot reach yields + * a `uuid` and no `node` — the ordinary state of a link to an unpublished page. The reverse never + * happens. + */ +export interface NodeReference { + /** The referenced node, or `undefined` when the reference does not resolve. */ + node?: JCRNodeWrapper; + /** + * The identifier the property stores, present whenever the property is set. + * + * Only ever a cache-dependency key. A node the visitor may not read still yields it, so putting + * it in an `href` or a label would both leak a target's existence and point at nothing. + */ + uuid?: string; +} + +/** + * Reads a `reference` or `weakreference` property without letting it break the render. + * + * A reference that no longer resolves throws from `getNode()` rather than returning null, and a + * `getNodeProps` read surfaces that as a plain falsy value — which is why every view that touches a + * reference ends up writing the same try/catch. This is that try/catch, once. + * + * It also separates the two cases the falsy value merges: an unset property returns `null`, a set + * one whose target is gone returns a `uuid` with no `node`. What it cannot separate is _why_ the + * target is gone — unpublished, deleted, and not readable by this visitor all arrive here + * identically, and no JCR read can tell them apart. + * + * @example + * ```ts + * const related = readNodeReference(currentNode, "example:related")?.node; + * ```; + * + * @param node - The node carrying the property. + * @param property - The reference property to read. + * @returns The reference, or `null` when the property is unset, empty or unreadable. Never throws. + * @see {@link getNodeProps} for reading the ordinary property types. + */ +export function readNodeReference(node: JCRNodeWrapper, property: string): NodeReference | null { + if (!node) return null; + + let uuid: string | undefined; + let referenced: JCRNodeWrapper | undefined; + + try { + if (!node.hasProperty(property)) return null; + const raw = node.getPropertyAsString(property); + uuid = raw && raw.trim() ? raw : undefined; + referenced = node.getProperty(property).getValue().getNode() ?? undefined; + } catch { + // The set-but-unresolvable case, and the case this function exists for + referenced = undefined; + } + + return referenced || uuid ? { node: referenced, uuid } : null; +} diff --git a/javascript-modules-library/src/utils/link/devWarnings.ts b/javascript-modules-library/src/utils/link/devWarnings.ts new file mode 100644 index 00000000..c186e056 --- /dev/null +++ b/javascript-modules-library/src/utils/link/devWarnings.ts @@ -0,0 +1,57 @@ +/** + * True on a development instance. + * + * Every part of this call can be missing — the whole `server` bridge outside the engine, the method + * on an engine older than it — and a diagnostic that cannot tell must stay quiet rather than fail. + */ +const isDevelopmentMode = (): boolean => { + try { + return server.config.isDevelopmentMode(); + } catch { + return false; + } +}; + +/** + * What has already been said. A diagnostic describes a mistake in the code, not an event, so the + * second occurrence of the same key carries no information the first did not. + */ +const reported = new Set(); + +/** + * Prints one line per key, on a development instance only — a production instance pays nothing. + * + * @param key - What makes two occurrences the same mistake. + * @param message - Built only when it will actually be printed. + */ +const warnOnce = (key: string, message: () => string): void => { + if (!isDevelopmentMode() || reported.has(key)) return; + + reported.add(key); + console.warn(message()); +}; + +/** Where the messages send the reader for the long version. */ +const GUIDE = "docs/2-guides/9-links/README.md"; + +/** + * Warns, once per scheme, that `allowedSchemes` was asked to allow a scheme the library refuses. + * + * The option narrows the built-in list and cannot widen it, so the request is dropped rather than + * honoured. Saying nothing would leave a project believing its `s3:` links are rendered when they + * are silently not navigable — a wrong result that looks exactly like missing content. + * + * @param schemes - The requested schemes that are not in the built-in list. + */ +export function warnUnknownAllowedSchemes(schemes: readonly string[]): void { + for (const scheme of schemes) { + warnOnce( + `unknown-allowed-scheme:${scheme}`, + () => + `getLinkProps: allowedSchemes asks for "${scheme}", which the library does not allow. ` + + `The option narrows the built-in list — http, https, mailto, tel, ftp — it does not ` + + `extend it, so "${scheme}" links stay not navigable. ` + + `See ${GUIDE}, "URLs you did not build".`, + ); + } +} diff --git a/javascript-modules-library/src/utils/link/getLinkProps.ts b/javascript-modules-library/src/utils/link/getLinkProps.ts index 5273ca4c..29bbb3dd 100644 --- a/javascript-modules-library/src/utils/link/getLinkProps.ts +++ b/javascript-modules-library/src/utils/link/getLinkProps.ts @@ -2,19 +2,33 @@ import type { Locale } from "java.util"; import type { JCRNodeWrapper } from "org.jahia.services.content"; import type { RenderContext } from "org.jahia.services.render"; import { appendParameters, buildNodeUrl, schemeRegExp } from "../urlBuilder/urlBuilder.js"; +import { warnUnknownAllowedSchemes } from "./devWarnings.js"; +import { DEFAULT_ALLOWED_SCHEMES, getLinkDefaults } from "./linkDefaults.js"; import type { AnchorProps, LinkContext, LinkOptions, LinkProps, LinkTarget } from "./types.js"; /** The values `jmix:link`'s `j:target` allows. Anything else omits the attribute. */ const TARGET_ATTRIBUTES: readonly string[] = ["_blank", "_parent", "_self", "_top"]; /** - * Schemes a link may use, for every URL the library did not build itself — an author-supplied - * `j:url` included. + * The schemes this call allows: the call's own list, then the module's, then the built-in one. * - * React neutralises `javascript:` alone, by substituting a throwing URL rather than removing the - * attribute; `data:`, `blob:` and `vbscript:` are covered by this list and by nothing else. + * Whichever is chosen is intersected with the built-in list, so the option can only ever narrow. + * Allowing a scheme the library rejects is not a call-site decision — `javascript:` and `data:` are + * the reason the list exists — and a project that wants one has to be told so rather than to + * discover its links quietly missing. */ -const ALLOWED_SCHEMES: readonly string[] = ["http", "https", "mailto", "tel", "ftp"]; +function resolveAllowedSchemes( + requested: readonly string[] | undefined, + context: LinkContext | undefined, +): readonly string[] { + const asked = requested ?? getLinkDefaults(context?.bundleKey).allowedSchemes; + if (!asked) return DEFAULT_ALLOWED_SCHEMES; + + const normalized = asked.map((scheme) => scheme.trim().toLowerCase()); + const allowed = normalized.filter((scheme) => DEFAULT_ALLOWED_SCHEMES.includes(scheme)); + warnUnknownAllowedSchemes(normalized.filter((scheme) => !allowed.includes(scheme))); + return allowed; +} /** * Reproduces what a URL parser removes before it reads the scheme: ASCII tab and newline anywhere @@ -34,7 +48,7 @@ function normalizeUrl(raw: string): string { } /** The URL to navigate to, or `undefined` when its scheme is not allow-listed. */ -function allowedHref(raw: string): string | undefined { +function allowedHref(raw: string, allowedSchemes: readonly string[]): string | undefined { const url = normalizeUrl(raw); if (!url) return undefined; @@ -47,7 +61,7 @@ function allowedHref(raw: string): string | undefined { if (url.startsWith("/")) return url[1] === "/" || url[1] === "\\" ? undefined : url; const scheme = schemeRegExp.exec(url)?.[1].toLowerCase(); - return scheme && ALLOWED_SCHEMES.includes(scheme) ? url : undefined; + return scheme && allowedSchemes.includes(scheme) ? url : undefined; } /** @@ -204,7 +218,7 @@ function registerCacheDependency( * reads no React context of its own, so omitting it inside a render does not fall back to one. It * degrades instead — without `renderContext` no cache dependency is registered, and without * `mainNode` `isCurrent` and `isAncestor` are always false. Omit it only outside a render, where - * there is nothing to read. + * there is nothing to read. Its `bundleKey` selects the module whose `setLinkDefaults` apply. * @returns The anchor attributes and the state of the link. Never throws. * @see {@link resolveContentLink} to read the target off a content node first. */ @@ -219,7 +233,7 @@ export function getLinkProps( if (!target) { href = undefined; } else if (typeof target === "string") { - href = allowedHref(target); + href = allowedHref(target, resolveAllowedSchemes(options.allowedSchemes, context)); } else if ( !options.language || options.requireTranslation === false || @@ -239,6 +253,7 @@ export function getLinkProps( const mainPath = mainNode && read(mainNode, (main) => main.getPath()); const state = { + node, navigable: href !== undefined, isCurrent: options.isCurrent ?? isSameNode(node, mainNode), isAncestor: diff --git a/javascript-modules-library/src/utils/link/link.spec.ts b/javascript-modules-library/src/utils/link/link.spec.ts index 443f7429..3d78d40b 100644 --- a/javascript-modules-library/src/utils/link/link.spec.ts +++ b/javascript-modules-library/src/utils/link/link.spec.ts @@ -33,6 +33,7 @@ vi.mock("../urlBuilder/urlBuilder.js", async (importOriginal) => ({ const { getLinkProps } = await import("./getLinkProps.js"); const { resolveContentLink } = await import("./resolveContentLink.js"); +const { setLinkDefaults, clearLinkDefaults } = await import("./linkDefaults.js"); /** A reference property: the UUID it stores, and the node it resolves to — when it does. */ interface Reference { @@ -150,7 +151,19 @@ describe("a target that cannot be linked to", () => { expect(() => getLinkProps(dead, {}, { mainNode: dead })).not.toThrow(); const { anchor, state } = getLinkProps(dead, {}, { mainNode: dead }); expect(anchor).toEqual({}); - expect(state).toEqual({ navigable: false, isCurrent: false, isAncestor: false, label: "" }); + // The node is reported as it was handed in, unread: what it cannot answer is what is missing + expect(state.node).toBe(dead); + expect(Object.keys(state).sort()).toEqual([ + "isAncestor", + "isCurrent", + "label", + "navigable", + "node", + ]); + expect(state.navigable).toBe(false); + expect(state.isCurrent).toBe(false); + expect(state.isAncestor).toBe(false); + expect(state.label).toBe(""); }); it("never emits an anchor attribute with nothing to hang on", () => { @@ -721,3 +734,184 @@ describe("resolveContentLink", () => { expect(link?.state.label).toBe("Find"); }); }); + +describe("narrowing the scheme allow-list", () => { + afterEach(() => { + clearLinkDefaults(); + Reflect.deleteProperty(globalThis, "bundleKey"); + }); + + /** The engine sets this global while it evaluates a module's bundle. */ + const inModule = (name: string) => Reflect.set(globalThis, "bundleKey", name); + + it("refuses a scheme the call site left out", () => { + const options = { allowedSchemes: ["https"] }; + expect(getLinkProps("https://example.com", options).state.navigable).toBe(true); + expect(getLinkProps("http://example.com", options).state.navigable).toBe(false); + expect(getLinkProps("mailto:someone@example.com", options).state.navigable).toBe(false); + }); + + it("narrows every link of the module that asked for it", () => { + inModule("acme-module"); + setLinkDefaults({ allowedSchemes: ["https"] }); + + const context = { bundleKey: "acme-module" }; + expect(getLinkProps("https://example.com", {}, context).state.navigable).toBe(true); + expect(getLinkProps("http://example.com", {}, context).state.navigable).toBe(false); + }); + + it("leaves another module, and no module at all, on the built-in list", () => { + inModule("acme-module"); + setLinkDefaults({ allowedSchemes: ["https"] }); + + expect( + getLinkProps("http://example.com", {}, { bundleKey: "other-module" }).state.navigable, + ).toBe(true); + expect(getLinkProps("http://example.com").state.navigable).toBe(true); + }); + + it("lets one call widen back to the module's own floor, and no further", () => { + inModule("acme-module"); + setLinkDefaults({ allowedSchemes: ["https"] }); + + const context = { bundleKey: "acme-module" }; + expect( + getLinkProps("mailto:someone@example.com", { allowedSchemes: ["https", "mailto"] }, context) + .state.navigable, + ).toBe(true); + }); + + it("cannot be used to allow a scheme the library rejects", () => { + for (const url of ["javascript:alert(1)", "data:text/html,x", "s3://bucket/key"]) { + const scheme = url.slice(0, url.indexOf(":")); + expect(getLinkProps(url, { allowedSchemes: [scheme, "https"] }).state.navigable).toBe(false); + } + }); + + it("says so in development, rather than letting the links quietly disappear", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + Reflect.set(globalThis, "server", { + render: { addCacheDependency }, + config: { isDevelopmentMode: () => true }, + }); + + getLinkProps("s3://bucket/key", { allowedSchemes: ["https", "s3"] }); + expect(warn).toHaveBeenCalledOnce(); + expect(warn.mock.calls[0][0]).toContain('"s3"'); + + // A property of the code, not of the URL: the second call adds nothing + getLinkProps("s3://other/key", { allowedSchemes: ["https", "s3"] }); + expect(warn).toHaveBeenCalledOnce(); + + warn.mockRestore(); + }); + + it("stays silent in production", () => { + const warn = vi.spyOn(console, "warn").mockImplementation(() => {}); + Reflect.set(globalThis, "server", { + render: { addCacheDependency }, + config: { isDevelopmentMode: () => false }, + }); + + getLinkProps("gopher://example.com", { allowedSchemes: ["https", "gopher"] }); + expect(warn).not.toHaveBeenCalled(); + warn.mockRestore(); + }); + + it("ignores the case and the whitespace a configuration value carries", () => { + expect( + getLinkProps("https://example.com", { allowedSchemes: [" HTTPS "] }).state.navigable, + ).toBe(true); + }); + + it("refuses everything when the list is empty, which is a policy and not a mistake", () => { + expect(getLinkProps("https://example.com", { allowedSchemes: [] }).state.navigable).toBe(false); + // A site-relative path names no scheme, so no allow-list can judge it + expect(getLinkProps("/search", { allowedSchemes: [] }).state.navigable).toBe(true); + }); + + it("refuses to attach defaults outside a module's bundle evaluation", () => { + expect(() => setLinkDefaults({ allowedSchemes: ["https"] })).toThrow( + "no module to attach these defaults to", + ); + }); +}); + +describe("the node a link resolved to", () => { + it("hands back the node target, so a caller can read it without resolving it twice", () => { + const node = jcrNode({ path: "/sites/test/home/news" }); + expect(getLinkProps(node).state.node).toBe(node); + }); + + it("reports no node for a URL target and for no target at all", () => { + expect(getLinkProps("https://example.com").state.node).toBeUndefined(); + expect(getLinkProps(null).state.node).toBeUndefined(); + }); + + it("hands back the reference a content node carried", () => { + const target = jcrNode({ identifier: "u-target", path: "/sites/test/home/news" }); + const cta = jcrNode({ references: { "j:node": { uuid: "u-target", target } } }); + expect(resolveContentLink(cta, {}, { renderContext })?.state.node).toBe(target); + }); + + it("reports no node when the reference did not resolve", () => { + const cta = jcrNode({ references: { "j:node": { uuid: "u-draft" } } }); + const link = resolveContentLink(cta, {}, { renderContext }); + expect(link?.state.navigable).toBe(false); + expect(link?.state.node).toBeUndefined(); + }); +}); + +describe("where a mixin-shaped link takes its label from", () => { + const target = jcrNode({ + identifier: "u-target", + path: "/sites/test/home/news", + displayableName: "News", + }); + const context = { renderContext }; + + /** A CTA mixin on a card: the card's jcr:title is the heading, not the link label. */ + const card = jcrNode({ + strings: { "jcr:title": "Our latest work", "acme:ctaLabel": "See the projects" }, + references: { "j:node": { uuid: "u-target", target } }, + }); + + it("takes the heading by default, which is the bug this option exists for", () => { + expect(resolveContentLink(card, {}, context)?.state.label).toBe("Our latest work"); + }); + + it("takes the property the mixin actually stores its label in", () => { + expect( + resolveContentLink(card, { labelProperties: ["acme:ctaLabel"] }, context)?.state.label, + ).toBe("See the projects"); + }); + + it("falls through to the target's own name when the named properties are empty", () => { + expect( + resolveContentLink(card, { labelProperties: ["acme:missing"] }, context)?.state.label, + ).toBe("News"); + expect(resolveContentLink(card, { labelProperties: [] }, context)?.state.label).toBe("News"); + }); + + it("skips the content node entirely on labelFrom: target", () => { + expect(resolveContentLink(card, { labelFrom: "target" }, context)?.state.label).toBe("News"); + }); + + it("lets labelFrom win over labelProperties, as its documentation says", () => { + expect( + resolveContentLink(card, { labelFrom: "target", labelProperties: ["acme:ctaLabel"] }, context) + ?.state.label, + ).toBe("News"); + }); + + it("still lets an explicit label win over both", () => { + expect( + resolveContentLink(card, { label: "Read on", labelFrom: "target" }, context)?.state.label, + ).toBe("Read on"); + }); + + it("leaves the label of an external link empty when nothing on the node supplies one", () => { + const external = jcrNode({ strings: { "j:url": "https://example.com" } }); + expect(resolveContentLink(external, { labelFrom: "target" }, context)?.state.label).toBe(""); + }); +}); diff --git a/javascript-modules-library/src/utils/link/linkDefaults.ts b/javascript-modules-library/src/utils/link/linkDefaults.ts new file mode 100644 index 00000000..dc2551d2 --- /dev/null +++ b/javascript-modules-library/src/utils/link/linkDefaults.ts @@ -0,0 +1,70 @@ +/** + * The engine sets `bundleKey` as a context global while it evaluates a module's server bundle, and + * `useServerContext()` reports the same value while a view of that module renders. + * + * @see {@link setLinkDefaults} for why the link defaults are keyed by it. + */ +declare const bundleKey: string | undefined; + +/** + * Schemes a link may use, for every URL the library did not build itself — an author-supplied + * `j:url` included. + * + * React neutralises `javascript:` alone, by substituting a throwing URL rather than removing the + * attribute; `data:`, `blob:` and `vbscript:` are covered by this list and by nothing else. + */ +export const DEFAULT_ALLOWED_SCHEMES: readonly string[] = ["http", "https", "mailto", "tel", "ftp"]; + +/** The parts of a link a module can decide once instead of at every call site. */ +export interface LinkDefaults { + /** + * Schemes this module's links may use. A subset of {@link DEFAULT_ALLOWED_SCHEMES}: a scheme the + * library does not allow is dropped from the list rather than added to it. + */ + allowedSchemes?: readonly string[]; +} + +/** + * Every JavaScript module in an instance shares one GraalJS context, so a plain module-level + * variable in this library would be engine-wide: one module's policy would govern another module's + * links. Keying by bundle keeps a default inside the module that declared it. + */ +const defaultsByBundle = new Map(); + +/** + * Sets the link defaults of the calling module: every `JLink`, `getLinkProps` and + * `resolveContentLink` in it uses them unless the call overrides them. + * + * Call it at the top level of a server file — the engine only knows which module is speaking while + * it evaluates that module's bundle. + * + * @example + * ```ts + * // src/server/links.ts, imported once from a view + * setLinkDefaults({ allowedSchemes: ["http", "https"] }); + * ```; + * + * @param defaults - Merged into whatever the module already set. + * @throws When called outside a module's bundle evaluation, where there is no module to attach the + * defaults to. + */ +export function setLinkDefaults(defaults: LinkDefaults): void { + if (typeof bundleKey !== "string" || !bundleKey) { + throw new Error( + "setLinkDefaults: no module to attach these defaults to. Call it at the top level of a " + + "server file of your module, not inside a render or a callback.", + ); + } + + defaultsByBundle.set(bundleKey, { ...defaultsByBundle.get(bundleKey), ...defaults }); +} + +/** The defaults a module registered, or an empty set for a module that registered none. */ +export function getLinkDefaults(bundle: string | undefined): LinkDefaults { + return (bundle ? defaultsByBundle.get(bundle) : undefined) ?? {}; +} + +/** Drops every module's defaults. Exported for tests, which share one module registry. */ +export function clearLinkDefaults(): void { + defaultsByBundle.clear(); +} diff --git a/javascript-modules-library/src/utils/link/resolveContentLink.ts b/javascript-modules-library/src/utils/link/resolveContentLink.ts index 97a71b20..b88cb495 100644 --- a/javascript-modules-library/src/utils/link/resolveContentLink.ts +++ b/javascript-modules-library/src/utils/link/resolveContentLink.ts @@ -1,4 +1,5 @@ import type { JCRNodeWrapper } from "org.jahia.services.content"; +import { readNodeReference } from "../jcr/readNodeReference.js"; import { getLinkProps } from "./getLinkProps.js"; import type { LinkContext, LinkOptions, LinkProps, LinkTargetAttribute } from "./types.js"; @@ -11,6 +12,9 @@ const REFERENCE_PROPERTIES: readonly string[] = ["j:node", "j:linknode"]; /** The label of the link itself: `mix:title`, then `jmix:externalLink`. */ const TITLE_PROPERTIES: readonly string[] = ["jcr:title", "j:linkTitle"]; +/** Where {@link resolveContentLink} takes the label from. */ +export type LinkLabelSource = "content" | "target"; + /** A property read as a string, or `undefined` when it is absent, empty, or unreadable. */ function readString(node: JCRNodeWrapper, property: string): string | undefined { try { @@ -29,33 +33,6 @@ const readFirst = (node: JCRNodeWrapper, properties: readonly string[]): string undefined, ); -/** - * A reference, as both ends of it: the node it resolves to, and the raw value it stores. - * - * The raw value is the target's UUID, and it is only ever a cache-dependency key. A node the - * visitor may not read still yields it, so putting it in an `href` or a label would both leak a - * target's existence and render a URL that leads nowhere. - */ -function readReference( - node: JCRNodeWrapper, - property: string, -): { target?: JCRNodeWrapper; uuid?: string } | undefined { - let uuid: string | undefined; - let target: JCRNodeWrapper | undefined; - - try { - if (!node.hasProperty(property)) return undefined; - uuid = readString(node, property); - target = node.getProperty(property).getValue().getNode() ?? undefined; - } catch { - // An unresolvable reference throws rather than returning null, and is the case this API exists - // for: the property is set, the target is not there yet. - target = undefined; - } - - return target || uuid ? { target, uuid } : undefined; -} - /** * Reads a link off a content node, and turns it into the props of an anchor. * @@ -79,6 +56,11 @@ function readReference( * internal reference that does not resolve returns a link that is not navigable, carrying a cache * dependency on the raw reference, rather than nothing at all. * + * The label follows the same rule as the link itself: it is read off the content node, which is + * right when the link _is_ the content and wrong when the link is a mixin on something else. A + * mixin sits on a node whose `jcr:title` is the heading, not the link label — `labelProperties` and + * `labelFrom` say where the label really lives. + * * @example * ```tsx * const link = resolveContentLink(currentNode, {}, useServerContext()); @@ -86,7 +68,8 @@ function readReference( * ```; * * @param node - The content node carrying the link. - * @param options - Everything {@link getLinkProps} takes, plus the properties to read. + * @param options - Everything {@link getLinkProps} takes, plus the properties to read and where the + * label comes from. * @param context - What the link is resolved against. Pass `useServerContext()`; see * {@link getLinkProps} for what omitting it costs. * @returns The link props, or `null` when the node carries no link at all. Never throws. @@ -113,6 +96,27 @@ export function resolveContentLink( * @default "j:url" */ urlProperty?: string; + /** + * Properties holding the label of the link itself, tried in order. + * + * The default is right when the link _is_ the content, and wrong when the link is a mixin on + * something else: a CTA mixin sits on a card or a hero whose `jcr:title` is the heading, not + * the link label. Name the label property of your own mixin, or pass `[]` to fall through to + * the target's own name. + * + * @default ["jcr:title", "j:linkTitle"] + */ + labelProperties?: readonly string[]; + /** + * Where the label comes from. `"target"` skips the content node entirely and uses the + * displayable name of whatever the link points at — the readable spelling of `labelProperties: + * []`, and what a mixin-shaped link usually wants. + * + * It takes precedence: `labelFrom: "target"` ignores `labelProperties`. + * + * @default "content" + */ + labelFrom?: LinkLabelSource; } = {}, context?: LinkContext, ): LinkProps | null { @@ -123,30 +127,35 @@ export function resolveContentLink( noneValue = "none", referenceProperties = REFERENCE_PROPERTIES, urlProperty = "j:url", + labelProperties = TITLE_PROPERTIES, + labelFrom = "content", ...linkOptions } = options; if (readString(node, typeProperty)?.trim() === noneValue) return null; + // No label here means the props tier derives one from the target, which is what "target" asks for + const contentLabel = labelFrom === "target" ? undefined : readFirst(node, labelProperties); + const shared: LinkOptions = { ...linkOptions, - label: linkOptions.label ?? readFirst(node, TITLE_PROPERTIES), + label: linkOptions.label ?? contentLabel, // A content value, so it may be anything; getLinkProps drops what is not a jmix:link target target: linkOptions.target ?? (readString(node, "j:target") as LinkTargetAttribute | undefined), }; for (const property of referenceProperties) { - const reference = readReference(node, property); + const reference = readNodeReference(node, property); if (!reference) continue; // `true` and the default both mean "pick the key form", and the UUID of an unresolved // reference is only in hand here: the props tier receives nothing it could derive it from const automatic = shared.cacheDependency === undefined || shared.cacheDependency === true; const cacheDependency = - automatic && !reference.target && reference.uuid + automatic && !reference.node && reference.uuid ? { uuid: reference.uuid } : shared.cacheDependency; - return getLinkProps(reference.target, { ...shared, cacheDependency }, context); + return getLinkProps(reference.node, { ...shared, cacheDependency }, context); } const url = readString(node, urlProperty); diff --git a/javascript-modules-library/src/utils/link/types.ts b/javascript-modules-library/src/utils/link/types.ts index 5a9e7a8a..3df2b553 100644 --- a/javascript-modules-library/src/utils/link/types.ts +++ b/javascript-modules-library/src/utils/link/types.ts @@ -55,6 +55,15 @@ export interface LinkState { isAncestor: boolean; /** The label to render when the caller provides no children. Empty when nothing supplies one. */ label: string; + /** + * The node the link resolved to, when there was one: a node target, or the reference + * {@link resolveContentLink} read off a content node. Absent for a URL target and for a reference + * that did not resolve. + * + * It is called `node` and not `target` because `target` is already the anchor attribute, and the + * two are never the same thing. + */ + node?: JCRNodeWrapper; } /** Everything a link needs to be rendered: the anchor attributes, and the rest. */ @@ -64,6 +73,14 @@ export interface LinkProps { } export interface LinkOptions { + /** + * Schemes a URL target may use, narrowing the built-in list — `http`, `https`, `mailto`, `tel`, + * `ftp`. A scheme outside that list is dropped rather than added, because a component cannot be + * the place a project loosens its own URL policy. + * + * Set it module-wide with `setLinkDefaults` and override it here for the one field that differs. + */ + allowedSchemes?: readonly string[]; /** Query string parameters. Inserted before any fragment, on node and string targets alike. */ parameters?: Record; /** Fragment, without the leading `#`. Appended last, replacing a fragment the target carries. */ @@ -132,4 +149,6 @@ export interface LinkContext { currentResource?: Resource; /** The node of the main resource — the page being rendered, not the node being rendered. */ mainNode?: JCRNodeWrapper; + /** Selects the module whose `setLinkDefaults` apply. */ + bundleKey?: string; } diff --git a/javascript-modules-library/src/utils/urlBuilder/urlBuilder.spec.ts b/javascript-modules-library/src/utils/urlBuilder/urlBuilder.spec.ts new file mode 100644 index 00000000..1fa8309c --- /dev/null +++ b/javascript-modules-library/src/utils/urlBuilder/urlBuilder.spec.ts @@ -0,0 +1,65 @@ +import { describe, expect, it } from "vitest"; +import type { JCRNodeWrapper } from "org.jahia.services.content"; +import type { RenderContext, Resource } from "org.jahia.services.render"; +import { appendParameters, buildNodeUrl } from "./urlBuilder.js"; + +/** A node, seen only through the path and URL the builder reads. */ +const jcrNode = (path = "/sites/test/home", url = "/cms/render/live/en/sites/test/home.html") => + ({ + getPath: () => path, + getUrl: () => url, + getResolveSite: () => ({ getServerName: () => undefined }), + }) as unknown as JCRNodeWrapper; + +/** The two context objects `buildNodeUrl` reads, with no context path and no URL encoding. */ +const context = (mode?: string, locale = "en") => ({ + renderContext: { + getMode: () => mode, + getRequest: () => ({ getContextPath: () => "" }), + getResponse: () => ({ encodeURL: (url: string) => url }), + } as unknown as RenderContext, + currentResource: { + getLocale: () => ({ toString: () => locale }), + getTemplateType: () => "html", + } as unknown as Resource, +}); + +describe("appendParameters", () => { + it("keeps the query string ahead of the fragment", () => { + expect(appendParameters("/page.html#main", { a: "b" })).toBe("/page.html?a=b#main"); + expect(appendParameters("/page.html?x=1#main", { a: "b" })).toBe("/page.html?x=1&a=b#main"); + }); + + it("returns the URL untouched when there is nothing to append", () => { + expect(appendParameters("/page.html#main", {})).toBe("/page.html#main"); + }); +}); + +describe("the servlet path of the manual branch", () => { + it("points edit mode at the servlet that actually renders the page", () => { + // /cms/edit/ redirects to the jContent UI; only an survives it, because EditModeFilter + // substitutes the two on its way out. Everything else — an island payload, a data-* attribute — + // carries the URL as built. + expect(buildNodeUrl(jcrNode(), { mode: "edit" }, context("edit"))).toBe( + "/cms/editframe/default/en/sites/test/home.html", + ); + expect(buildNodeUrl(jcrNode(), { language: "de" }, context("edit"))).toBe( + "/cms/editframe/default/de/sites/test/home.html", + ); + }); + + it("leaves the other two modes where they were", () => { + expect(buildNodeUrl(jcrNode(), { mode: "preview" }, context("preview"))).toBe( + "/cms/render/default/en/sites/test/home.html", + ); + expect(buildNodeUrl(jcrNode(), { mode: "live" }, context("live"))).toBe( + "/cms/render/live/en/sites/test/home.html", + ); + }); + + it("still asks the node itself when no mode, language or extension is named", () => { + expect(buildNodeUrl(jcrNode(), {}, context("edit"))).toBe( + "/cms/render/live/en/sites/test/home.html", + ); + }); +}); diff --git a/javascript-modules-library/src/utils/urlBuilder/urlBuilder.ts b/javascript-modules-library/src/utils/urlBuilder/urlBuilder.ts index 8dda53b7..e5c4430c 100644 --- a/javascript-modules-library/src/utils/urlBuilder/urlBuilder.ts +++ b/javascript-modules-library/src/utils/urlBuilder/urlBuilder.ts @@ -138,7 +138,10 @@ export function buildNodeUrl( return toAbsoluteUrl( buildEndpointUrl( (mode === "edit" - ? "/cms/edit/default/" + ? // The page builder renders from /cms/editframe/; /cms/edit/ redirects to the jContent + // UI instead, and only reaches the page because EditModeFilter substitutes the two — + // for an and nothing else + "/cms/editframe/default/" : mode === "preview" ? "/cms/render/default/" : "/cms/render/live/") + From effccbb558f0793989fcdb8f282fb79e83442bbb Mon Sep 17 00:00:00 2001 From: Romain Gauthier Date: Sun, 23 Aug 2026 21:41:10 +0200 Subject: [PATCH 2/2] docs(links): what the component now lets you do, and which URLs core finishes --- docs/2-guides/9-links/README.md | 132 ++++++++++++++++++++++++++++++-- 1 file changed, 127 insertions(+), 5 deletions(-) diff --git a/docs/2-guides/9-links/README.md b/docs/2-guides/9-links/README.md index 49598cd5..92f560fb 100644 --- a/docs/2-guides/9-links/README.md +++ b/docs/2-guides/9-links/README.md @@ -36,6 +36,42 @@ That single line builds the URL through `buildNodeUrl`, registers a render cache Everything else you pass is a plain anchor attribute: `className`, `hreflang`, `download`, `onClick`. There is no styling of its own. +## Attributes the component does not know about + +Two things a real site needs, and neither is expressible as a prop. + +The first is `data-*`. React's typings do not model it on a component's props, so `attributes` takes an open map: + +```tsx + +``` + +A static map is the easy half. The interesting form is a function, because the values an analytics layer wants are the ones the component just computed and would otherwise keep to itself — the resolved URL and the derived label: + +```tsx + ({ + "data-element-url": anchor.href, + "data-element-text": state.label, + "data-element-current": state.isCurrent, + })} +/> +``` + +It receives exactly what `getLinkProps` returns, so the same callback works on both tiers. It is spread last, so it wins over anything else on the element, and it is not called at all when the link is not navigable. `` is the same shape, for the same reason. + +The second is a wrapper that is not a bare ``. A design system's call to action is usually its own component, and wrapping it in an anchor gives you two nested interactive elements. `asChild` hands the link to the element you render instead — Next.js calls the same thing `passHref`: + +```tsx + + Read more + +// → Read more +``` + +The child receives `href`, `target`, `rel`, `aria-current` and whatever `attributes` produced, and must forward them to the element it renders. It needs exactly one element child; anything else is an error naming the way out. When the link is not navigable the child is still rendered, simply without the link — `whenUnresolved="none"` is how you drop it entirely. + ## A target that does not resolve is normal This is the part that surprises people. Publishing a page does **not** publish the pages it links to: `jnt:page` is in `referencedNodeTypesToSkip`. So a perfectly ordinary editorial workflow — build a card, point it at a page that is still a draft, publish the card — leaves you with a reference that resolves to nothing in live. @@ -72,7 +108,27 @@ const { anchor, state } = getLinkProps(node, {}, useServerContext()); return state.navigable ? {state.label} : {state.label}; ``` -`anchor` is spreadable onto an `` — every key is a valid anchor attribute, by construction. `state` is not: `navigable`, `isCurrent`, `isAncestor` and `label` are yours to read, never to spread. +`anchor` is spreadable onto an `` — every key is a valid anchor attribute, by construction. `state` is not: `navigable`, `isCurrent`, `isAncestor`, `label` and `node` are yours to read, never to spread. + +`state.node` is what the link resolved to — the node target, or the reference read off a content node. It saves the second resolution a fallback usually needs: + +```tsx +const { anchor, state } = resolveContentLink(cta, {}, useServerContext()) ?? {}; +const label = state?.label || state?.node?.getProperty("acme:shortName")?.getString(); +``` + +Reading a reference yourself is the other half of that problem, and it is a JCR concern rather than a link one: an unresolvable reference reaches JavaScript as a plain falsy value, so every view that touches a `weakreference` ends up writing the same try/catch. `readNodeReference` is that try/catch, once, next to `getNodeProps`: + +```tsx +import { readNodeReference } from "@jahia/javascript-modules-library"; + +const related = readNodeReference(currentNode, "acme:related"); +// null → the property is unset +// { uuid } → it is set, and the target is not reachable (unpublished, deleted, forbidden) +// { uuid, node } → it resolved +``` + +It never throws. What it cannot tell you is _why_ an unreachable target is unreachable: unpublished, deleted and "you may not see it" arrive identically, and no JCR read separates them. :::info `getLinkProps` reads no React context of its own. Inside a view, pass `useServerContext()`. Without it you still get an `href`, but no cache dependency is registered and `isCurrent` is always false — a silent downgrade, not an error. @@ -90,6 +146,22 @@ Pass the content node and let the library read it: With no children, the label comes from the content: `jcr:title`, then `j:linkTitle`, then the displayable name of the target. +### When the link is a mixin, the label is somewhere else + +That default is right when the link **is** the content — a `jnt:nodeLink` exists to be a link, and its `jcr:title` is the link label. It is wrong as soon as the link is a **mixin on something else**. A CTA mixin sits on a card, a panel or a hero that already has a `jcr:title`, and that title is the heading. Take it as the label and every call to action on the page is named after the section it lives in. + +Say where the label really lives: + +```tsx +// The mixin stores its own label + + +// There is no label property: use the name of the page it points at + +``` + +`labelProperties` replaces the list that is tried on the content node, in order. `labelFrom="target"` skips the content node altogether — the readable spelling of `labelProperties={[]}` — and takes precedence over `labelProperties` when both are given. An explicit `label`, or children, still wins over either. + Because the `j:linkType` convention is a module convention and at least four spellings of it exist in the wild, the discriminator is a parameter: ```tsx @@ -115,6 +187,25 @@ Any string that the library did not build itself goes through a scheme allow-lis This applies to an `href` you pass and to an author-supplied `j:url` alike, and it is applied at render time, so it also covers content stored before anyone thought to validate it. React alone is not enough here: it neutralises `javascript:` by substituting a throwing URL, and it matches no other scheme. +A project is often stricter than that. A "partner website" field that must be `https://` and nothing else does not want `tel:` links quietly working. Narrow the list — per call, or once for the whole module: + +```ts +// src/server/links.ts, imported once from a view +import { setLinkDefaults } from "@jahia/javascript-modules-library"; + +setLinkDefaults({ allowedSchemes: ["http", "https"] }); +``` + +```tsx + +``` + +`setLinkDefaults` is keyed by the module that calls it, the same way `setImageDefaults` is: every JavaScript module in an instance shares one JavaScript context, so a module-level variable would be a policy for the whole server. Call it at the top level of a server file, not inside a render. + +:::warning +The option **narrows only**. A scheme that is not on the built-in list is dropped rather than added, because a call site is not the place a project loosens its own URL policy — `javascript:` and `data:` are the reason the list exists. On a development instance the library says so once per scheme; in production the links are simply not navigable. +::: + Query parameters and a fragment are options rather than string surgery, and they land in the right order: ```tsx @@ -187,18 +278,47 @@ A language switcher is the case where the computation is wrong and you know bett `target` is validated against the four values `jmix:link` allows (`_blank`, `_parent`, `_self`, `_top`). Anything else omits the attribute rather than emitting `target=""`, which matters because the value often comes straight from content. `rel="noopener noreferrer"` is added whenever `target` resolves to `_blank`; pass `rel` yourself to replace it. :::info -These are live and preview guarantees. In the page builder, `EditModeFilter` rewrites the anchors it delivers: it turns `/cms/edit/` into `/cms/editframe/` and either deletes `target` or staples `target="_blank"` on with no `rel`. Assert on the delivered DOM, not on what your component returned. +These are live and preview guarantees. In the page builder, `EditModeFilter` rewrites the anchors it delivers: it either deletes `target` or staples `target="_blank"` on with no `rel`. Assert on the delivered DOM, not on what your component returned. ::: ## `href` is a server-side intermediate -The `href` you get back is not the URL the visitor receives. Core finishes it after the render — vanity URLs, SEO rewriting, and the `?jsite=` parameter that live adds to a cross-site link — and it does so by walking the emitted HTML. `URLTraverser` only visits a fixed set of tag/attribute pairs (`a[href]`, `img[src]`, `form[action]`, `link[href]`, and a few more) in an `html` template type. +The `href` you get back is not the URL the visitor receives. Core finishes it after the render, and it does so by walking the emitted HTML rather than by touching the value you built. Two filters do the work, and each visits a fixed set of tag/attribute pairs in an `html` template type: + +| Filter | What it adds | Where it looks | +| ---------------- | ------------------------------------------------------------------------------------- | ---------------------------------------------------------------------------------------------------------------------------------- | +| `URLFilter` | Vanity URLs, the SEO server name, and the `?jsite=` parameter of a cross-site link | `a[href]`, `img[src\|srcset\|data-src\|data-srcset]`, `form[action]`, `link[href]`, `source[srcset]`, `embed[src]`, `param[value]` | +| `EditModeFilter` | In the page builder: `/cms/edit/` → `/cms/editframe/`, and `target` deleted or forced | `a[href]` only | -So: +So the rule is simple, and it is about **where you put the URL**, not about how you built it: -- Put the URL anywhere else — a `data-*` attribute, an Island payload, the JSON body of an action — and it stays exactly as you built it. No vanity URL, no `?jsite=`. +- Emit it as one of those attributes and it is finished for you. +- Put it anywhere else — a `data-*` attribute of your own, an Island payload, the JSON body of an action, a `` tag, a JSON-LD block, a CSS `url()` — and it stays exactly as you built it. No vanity URL, no `?jsite=`. - Never string-compare an `href`, and never parse it to decide something. Compare nodes, or use `state.isCurrent` and `state.isAncestor`. +There is no call that runs the finishing pass for you: it needs the assembled HTML, which does not exist yet while your view runs. What you can do is make the URL correct without it — see below — and reach for `buildNodeUrl(node, { absolute: true })` when the URL leaves the page altogether (`og:url`, an email, JSON-LD). + +### The edit-mode URL, which used to need a workaround + +One rewrite used to bite hard enough that projects patched it by hand: + +```tsx +// Don't. This is what the library now gets right. +buildNodeUrl(target).replace("/cms/edit/", "/cms/editframe/"); +``` + +The reason it existed: `/cms/edit/…` does not render a page. On Jahia 8.2.3 it answers `302` to the jContent UI, and it only ever reached the page because `EditModeFilter` substituted the two — for an `a[href]` and nothing else. A URL in an Island payload kept the redirecting form, so the nav rendered by that Island navigated the iframe to a whole second copy of jContent. + +`buildNodeUrl` now emits `/cms/editframe/…` for edit mode directly, which is what `node.getUrl()` already returned when no `mode`, `language` or `extension` was named. The workaround is no longer needed, and neither is the branch around it: + +```tsx +// The URL is correct wherever it goes, including into an Island +const { anchor, state } = getLinkProps(page, { language }, useServerContext()); +; +``` + +Note that this is the URL of the page **inside** the builder's frame. Deep-linking a visitor into the jContent editor is a different URL (`/jahia/jcontent/…`) and not something this API builds. + ## Links inside Islands The library cannot be imported from a client bundle: the Vite plugin fails the build if you try. An Island therefore receives link _data_, not a link component, and renders the anchor itself: @@ -229,4 +349,6 @@ If you need a policy on those anchors, it belongs in a render filter — `regist - [`JLink`](https://github.com/Jahia/javascript-modules/blob/main/javascript-modules-library/README.md#jlink) — the component - [`getLinkProps`](https://github.com/Jahia/javascript-modules/blob/main/javascript-modules-library/README.md#getlinkprops) — the props tier, for Islands and custom markup - [`resolveContentLink`](https://github.com/Jahia/javascript-modules/blob/main/javascript-modules-library/README.md#resolvecontentlink) — reading a link off a content node +- [`setLinkDefaults`](https://github.com/Jahia/javascript-modules/blob/main/javascript-modules-library/README.md#setlinkdefaults) — the module-wide scheme allow-list +- [`readNodeReference`](https://github.com/Jahia/javascript-modules/blob/main/javascript-modules-library/README.md#readnodereference) — reading a reference property safely - [`buildNodeUrl`](https://github.com/Jahia/javascript-modules/blob/main/javascript-modules-library/README.md#buildnodeurl) — the URL tier underneath