feat(library): a link API for JavaScript modules - #751
Conversation
📝 Documentation GuidelinesThank you for contributing to our documentation! To ensure your contributions meet our standards, please review these resources:
This comment is posted automatically when changes are detected in the |
🦜 Chachalog
|
commit: |
83d77c7 to
db86b96
Compare
db86b96 to
f598987
Compare
f598987 to
8a24e44
Compare
appendParameters split on `?` alone, so a target carrying a fragment came back as `#main?a=b` — a query string inside the fragment, which never reaches the server. It now inserts the parameters before the fragment, and returns the URL untouched when there is nothing to append. Also exports the RFC 3986 scheme regexp so the link tier reads a scheme the same way buildModuleFileUrl does, rather than carrying a third copy. Refs #749
getLinkProps turns whatever names a link target — a node, an already-built URL, or nothing — into anchor attributes plus the state around them, and resolveContentLink reads that target off a content node first. Not being navigable is a result rather than an error: publishing a page does not publish the pages it links to, so an unresolved reference is the normal state of a link, and buildNodeUrl throwing on it took the whole fragment down. Neither function throws, and neither returns an href it could not build. On the way they register the render cache dependency, put every URL the library did not build itself through a scheme allow-list, validate the anchor target against the four values jmix:link allows, add rel to _blank, derive the label, and answer whether the target is the page being rendered. Refs #749
<JLink node={page}>Title</JLink> renders a bare <a>: the URL, a validated
target with its rel, aria-current on the page being rendered, and a render
cache dependency on the target. It takes one of three targets — a node, a
content node describing a link, or an already-built URL — as a
discriminated union, so naming two of them is a type error.
It never renders an <a> without an href. When the link is not navigable it
renders the children on their own, or nothing when whenUnresolved says so.
Server-side only, because it registers the cache dependency: a client
component takes getLinkProps' anchor and spreads it instead.
Refs #749
Adds vitest to the library, and one spec per tier. The nodes are hand-rolled stubs whose getters fail the way JCR fails — an unresolvable reference throws rather than returning null, getUrl() returns null on a repository error — because those are the failures the props tier has to absorb. Refs #749
Covers the one-liner, why an unresolvable reference is the normal state rather than an edge case, the cache-dependency key forms, the cache.mainResource=true rule that current-page state depends on, target and rel with the page-builder carve-out, why an href is a server-side intermediate that must never be string-compared, links inside Islands, and what rich text puts out of reach. Refs #749
Drops the none/internal/external switch: resolveContentLink reads the link off the node, and JLink renders the plain title when there is none. Refs #749
8a24e44 to
d1656c2
Compare
1 participant
Summary
Gives the library a link concept, so a module author renders a correct, cacheable, accessible link by naming the target instead of assembling one. First implementation pass on #749. Stacked on #746 — base is
feat/image-api, so the diff here is link code only; review #746 first.That one line now produces an
hrefbuilt throughbuildNodeUrl, a render cache dependency on the target,aria-current="page"when the target is the page being rendered, a validatedtargetwith itsrel— and, when the reference does not resolve, the children rendered without an anchor instead of a throw.Why
buildNodeUrlthrows on a falsy node (urlBuilder.ts:72), and an unresolvable reference is the normal state of a link: publishing a page does not publish thejnt:pageit points at. So a live section becomes<!-- Module error : Expected a node in buildNodeUrl -->— HTTP 200, section gone. The rest was hand-written too: thenone | internal | externalswitch appears insamples/hydrogen, Jahia/luxe-jahia-demo, Jahia/jahia.com and Jahia/se-utils under four incompatible property names, and nothing in the library readj:target, emittedrelon an author-supplied URL, or checked that a target exists in the language being linked to.Changes
getLinkProps(target, options, context)— takes a node, a string,nullorundefined, and never throws. Returns{ anchor, state }:anchoris DOM-spreadable by construction (href,target,rel,title),state(navigable,isCurrent,isAncestor,label) never is. Registers the cache dependency, validatestargetagainst the fourjmix:linkvalues, addsrel="noopener noreferrer"to_blank, derives the label, and appliesparameters/hash/language.requireTranslation(default true) makes an untranslated target non-navigable;fr_CHandfr-CHare the same locale, and language is dropped for language-neutral content so annt:filekeeps its/files/…URL instead of a/cms/render/….htmlone that does not serve the file.resolveContentLink(node, options, context)— reads the link off a content node:jnt:nodeLink(j:node),jnt:externalLink(j:url),jmix:link'sj:target,mix:title, plus thej:linkTypeconvention from Jahia/default. The discriminator is only ever read for its "no link" value, and every property name is a parameter (typeProperty,noneValue,referenceProperties,urlProperty) because four spellings exist in the wild. A reference wins over the URL;referenceProperties: []is the escape hatch for content where an earlier edit left a reference behind.<JLink>— a bare<a>, no styling, every other anchor attribute passed through. A discriminated union makesnode/content/hrefmutually exclusive at compile time, and thehrefshape additionally requireschildrenoraria-label(WCAG 2.4.4, the rolealtplays on<JImage>).whenUnresolvedchooses between the unwrapped children and nothing. Server-only; an Island takes the data instead,<a {...anchor}>.href,j:url) is checked againsthttp/https/mailto/tel/ftpafter tab, newline and control stripping, so ajavascript:,data:orvbscript:URL is never rendered. A relative URL is allowed only while it names no host://evil.exampleand/\evil.exampleare rejected, since they leave the site.appendParametersfix — query parameters went after the fragment (#main?a=b, where the server never sees them) and an empty set appended a bare?. Now fragment-aware and shared, sobuildNodeUrl,buildEndpointUrlandbuildModuleFileUrlall get the fix. The RFC 3986 scheme regexp is exported and reused instead of a second inline copy.docs/2-guides/9-links), and thesamples/hydrogencall to action rewritten.One behaviour change in
samples/hydrogen, not a refactor: the CTA'snonebranch rendered<s>{title}</s>and now renders the plain title, because<JLink content>treats "no link" as not-navigable. The NavBar adoption is split out into #760 (issue #759) — it changes what the reference navigation renders and adds a caching rule, which deserves its own review.Validation
tsc --noEmitclean on the library and onsamples/hydrogen, plus a run with the spec files included, whichtsconfig.jsonnormally excludes.eslintclean,prettierclean on everything this branch touches,yarn build+publintclean.yarn workspace @jahia/javascript-modules-library test— 132 passing (105 link, 27 inherited from the base).hrefand the page-builder behaviour (EditModeFilterrewritingtargetand/cms/edit/in the delivered DOM) are unverified in situ, and there is no Cypress coverage — that is breakdown item 7. The natural next step is deployingsamples/hydrogenon a local 8.2.3 and checking the two cases the unit tests cannot reach: the same nav rendered on two pages through anAbsoluteArea, and a flush after the target's title changes.Decided, and what is left
{ uuid }cache dependency does not work, and this PR documents that instead of fixing it.RenderHelper.renderTagpopulates the tag before it sets the page context, soAddCacheDependencyTag.setUuiddereferences a null page context, theNullPointerExceptionis swallowed bycatch (Throwable), and nothing is registered. The library still picks the form — it is the only key an unresolved reference offers — but the JSDoc and the guide say it is dropped today, and point at addCacheDependency with a uuid key registers nothing #750. The fix is a one-line reorder in the engine that changes setter ordering for every tag rendered throughrenderTag, which is not something to land unexercised inside a library PR. Until it lands, the "publish a previously-dangling target and watch the fallback flush" case cannot pass;flushOnPathMatchingRegexpis the working substitute, and is exposed for that reason.{ flushOnPathMatchingRegexp }was added to the union — it is a real fourthAddCacheDependencyTagkey form the spec did not list.contextis a parameter, not a default ofuseServerContext()— React'suse()throws outside a render, which would break the "never throws" guarantee for callers outside one. Omitting it degrades instead: no cache dependency, no current-page state.feat/image-apiremoves the duplicate vitest bootstrap (Bootstrap a unit-test runner in the monorepo #745) and theyarn.locktree from this diff — both now come from the base — and settles the guide numbering, since8-imagesexists in the base and this takes9-links. It also means this cannot merge before feat(library): an image API for JavaScript modules #746.yarn testis still wired to nothing. The library's pom runsyarn,yarn lintandyarn build, and CI runs only the vite-plugin tests. Same hole feat(library): an image API for JavaScript modules #746 has, and the exec-maven-plugin execution that fixes it belongs in a change shared with that PR, not silently here.j:linknodewhenj:linkTypeswitches to external. The escape hatch and the warning are correct either way, but the default precedence renders the stale internal target.Part of #749 (props tier #752, component #753, content vocabulary #754, guide #757, tests #758); each commit references the epic.