diff --git a/packages/cli-kit/src/private/node/ui/components/Alert.test.tsx b/packages/cli-kit/src/private/node/ui/components/Alert.test.tsx index 4c44ae594ee..d956487745a 100644 --- a/packages/cli-kit/src/private/node/ui/components/Alert.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Alert.test.tsx @@ -142,4 +142,38 @@ describe('Alert', async () => { " `) }) + + test("footnotes a long URL written as `[label](url)` in the body so it doesn't wrap inside the banner border", async () => { + // Regression: a 100-char URL embedded as plain text wraps across the + // banner border at ~78 cols, splitting the URL with │ characters and + // making it neither clickable nor copy-pasteable. Marking the URL up + // with `[label](url)` should place the label inline (with a `[N]` + // anchor) and emit the URL in the post-banner footnote block. + const longUrl = + 'https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties' + const options = { + body: `See specification requirements: [docs](${longUrl})`, + } + + const {lastFrame} = render() + const frame = unstyled(lastFrame()!) + + // The URL must not appear inside the bordered box. + const bodyLines = frame.split('\n').filter((line) => line.startsWith('│')) + bodyLines.forEach((line) => { + expect(line).not.toContain(longUrl) + }) + + // The footnote block (rendered after the closing `╰`) must list the + // URL. Ink wraps the long URL onto its own line when it exceeds terminal + // width, so we assert the `[1]` anchor and the URL show up *outside* the + // bordered box rather than as a single contiguous `[1] URL` substring. + const closingBorderIndex = frame.indexOf('╰') + expect(closingBorderIndex).toBeGreaterThanOrEqual(0) + const afterBox = frame.slice(closingBorderIndex) + expect(afterBox).toContain('[1]') + expect(afterBox).toContain(longUrl) + // And the body must reference the footnote. + expect(frame).toContain('docs [1]') + }) }) diff --git a/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx b/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx index 51721c0a06d..2e234e928dc 100644 --- a/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx @@ -245,4 +245,37 @@ describe('FatalError', async () => { " `) }) + + test('routes a plain-string error.message through TokenizedText so opt-in `[label](url)` markdown renders as a footnote-backed link', async () => { + // Regression: previously a `FatalError` constructed with a plain string + // message bypassed `TokenizedText` and rendered as a bare ``, + // meaning any URL embedded in the message wrapped against the banner + // border. Wiring `error.message` through `TokenizedText` lets servers + // (or the CLI itself) opt in to the same footnote treatment via + // CommonMark `[label](url)` markdown. + const longUrl = + 'https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties' + const error = new AbortError(`See specification requirements: [docs](${longUrl})`) + + const {lastFrame} = render() + const frame = unstyled(lastFrame()!) + + // The URL must not appear inside the bordered box. + const bodyLines = frame.split('\n').filter((line) => line.startsWith('│')) + bodyLines.forEach((line) => { + expect(line).not.toContain(longUrl) + }) + + // The footnote block must list the URL outside the box. Ink wraps the + // long URL onto its own line when it exceeds terminal width, so we + // assert the `[1]` anchor and the URL show up *after* the closing + // border rather than as a single contiguous `[1] URL` substring. + const closingBorderIndex = frame.indexOf('╰') + expect(closingBorderIndex).toBeGreaterThanOrEqual(0) + const afterBox = frame.slice(closingBorderIndex) + expect(afterBox).toContain('[1]') + expect(afterBox).toContain(longUrl) + // And the body must reference the footnote with the markdown label. + expect(frame).toContain('docs [1]') + }) }) diff --git a/packages/cli-kit/src/private/node/ui/components/FatalError.tsx b/packages/cli-kit/src/private/node/ui/components/FatalError.tsx index f8876fab6ea..033942b16ed 100644 --- a/packages/cli-kit/src/private/node/ui/components/FatalError.tsx +++ b/packages/cli-kit/src/private/node/ui/components/FatalError.tsx @@ -48,7 +48,15 @@ const FatalError: FunctionComponent = ({error}) => { ) : null} - {error.formattedMessage ? : {error.message}} + {error.formattedMessage ? ( + + ) : ( + // Route plain-string error messages through TokenizedText so that any + // opt-in `[label](url)` / `` markdown the server (or the caller) + // emits is rendered as a footnote-backed Link inside the FatalError + // banner. + + )} {error.tryMessage ? : null} diff --git a/packages/cli-kit/src/private/node/ui/components/Link.test.tsx b/packages/cli-kit/src/private/node/ui/components/Link.test.tsx index 00fb8a8b7c0..29fbc4ee135 100644 --- a/packages/cli-kit/src/private/node/ui/components/Link.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Link.test.tsx @@ -1,4 +1,5 @@ import {Link} from './Link.js' +import {LinksContext} from '../contexts/LinksContext.js' import {render} from '../../testing/ui.js' import {describe, expect, test, vi} from 'vitest' import React from 'react' @@ -100,6 +101,68 @@ describe('Link', async () => { expect(lastFrame()).toMatchInlineSnapshot('"https://example.com"') }) + test('renders a label-bearing link inside a LinksContext as `label [N]` and registers the URL in the footnote table', async () => { + // Inside a Banner's LinksContext, the visible label stays compact + // (`label [1]`) and the URL is captured for rendering outside the + // bordered box, where it can wrap freely without `│` interleaving. + supportHyperLinks(false) + + const links: Record = {} + const link = { + url: 'https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties', + label: 'docs', + } + + const {lastFrame} = render( + { + const id = (Object.keys(links).length + 1).toString() + links[id] = {label, url} + return id + }, + }} + > + + , + ) + + expect(lastFrame()).toBe('docs [1]') + expect(links['1']).toEqual({label: 'docs', url: link.url}) + }) + + test('renders a label-less link inside a LinksContext as a bare `[N]` anchor (no inline URL)', async () => { + // Regression: previously this path rendered `${url} [N]`, putting the + // long URL inside the bordered box and defeating the footnote + // mechanism. The footnote alone is now the source of truth for the URL. + supportHyperLinks(false) + + const links: Record = {} + const link = { + url: 'https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties', + } + + const {lastFrame} = render( + { + const id = (Object.keys(links).length + 1).toString() + links[id] = {label, url} + return id + }, + }} + > + + , + ) + + expect(lastFrame()).toBe('[1]') + expect(lastFrame()).not.toContain(link.url) + expect(links['1']).toEqual({label: undefined, url: link.url}) + }) + function supportHyperLinks(isSupported: boolean) { vi.mocked(supportsHyperlinks).stdout = isSupported } diff --git a/packages/cli-kit/src/private/node/ui/components/Link.tsx b/packages/cli-kit/src/private/node/ui/components/Link.tsx index 0690dcbd133..c5e3733fbdb 100644 --- a/packages/cli-kit/src/private/node/ui/components/Link.tsx +++ b/packages/cli-kit/src/private/node/ui/components/Link.tsx @@ -12,16 +12,19 @@ interface LinkProps { function link(label: string | undefined, url: string, linksContext: LinksContextValue | null) { if (!supportsHyperlinks.stdout) { - if (url === (label ?? url)) { - return url - } - if (linksContext === null) { + if (url === (label ?? url)) { + return url + } return label ? `${label} ${chalk.dim(`( ${url} )`)}` : url } + // Inside a LinksContext, register every link in the footnote table — even + // ones whose label equals the URL — so the visible label stays compact and + // the URL is rendered outside the bordered box where it can wrap without + // being interleaved with `│` characters. const linkId = linksContext.addLink(label, url) - return `${label ?? url} [${linkId}]` + return label ? `${label} [${linkId}]` : `[${linkId}]` } return ansiEscapes.link(label ?? url, url) diff --git a/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx b/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx index 16fd76c7002..f81b38161da 100644 --- a/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx +++ b/packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx @@ -1,9 +1,40 @@ import {tokenItemToString, TokenizedText} from './TokenizedText.js' +import {LinksContext, Link} from '../contexts/LinksContext.js' import {unstyled} from '../../../../public/node/output.js' import {render} from '../../testing/ui.js' -import {describe, expect, test} from 'vitest' +import {describe, expect, test, vi} from 'vitest' +import supportsHyperlinks from 'supports-hyperlinks' -import React from 'react' +import React, {FunctionComponent, useRef} from 'react' + +vi.mock('supports-hyperlinks') + +// Matches the on-the-wire OSC 8 sequence emitted by `ansiEscapes.link`, +// which is what `` ultimately renders when the terminal supports +// hyperlinks. Format: `ESC ] 8 ; ; URL BEL TEXT ESC ] 8 ; ; BEL`. +function asOsc8Link(url: string, label?: string) { + return `\u001b]8;;${url}\u0007${label ?? url}\u001b]8;;\u0007` +} + +// Mirrors the LinksContext that sets up at runtime, without pulling +// the whole Banner border into these tests. +const WithLinksContext: FunctionComponent<{children: React.ReactNode}> = ({children}) => { + const links = useRef>({}) + return ( + { + const newId = (Object.keys(links.current).length + 1).toString() + links.current = {...links.current, [newId]: {label, url}} + return newId + }, + }} + > + {children} + + ) +} describe('TokenizedText', async () => { test('renders arrays of items separated by spaces', async () => { @@ -57,6 +88,148 @@ describe('TokenizedText', async () => { `) }) + describe('markdown-link parsing in plain strings', async () => { + test('renders strings without a markdown link unchanged', async () => { + vi.mocked(supportsHyperlinks).stdout = false + + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toBe('no link here, just text') + }) + + test('does not linkify a bare URL — callers must opt in via `[label](url)` or ``', async () => { + vi.mocked(supportsHyperlinks).stdout = true + const url = 'https://example.com/docs' + + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toBe(`visit ${url} now`) + expect(lastFrame()).not.toContain(']8;;') + }) + + test('replaces an opt-in `[label](url)` with the label and a `[N]` footnote anchor when the terminal does not support hyperlinks', async () => { + vi.mocked(supportsHyperlinks).stdout = false + const url = 'https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties' + + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toBe('Reference: See specification requirements [1]') + expect(lastFrame()).not.toContain(url) + }) + + test('wraps the label of a `[label](url)` in OSC 8 escapes when the terminal supports hyperlinks', async () => { + vi.mocked(supportsHyperlinks).stdout = true + const url = 'https://example.com/docs' + + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toContain(asOsc8Link(url, 'docs page')) + }) + + test('renders a label-less `` autolink as a `[N]` anchor and registers the URL in the footnote table', async () => { + vi.mocked(supportsHyperlinks).stdout = false + const url = 'https://shopify.dev/docs' + + const {lastFrame} = render( + + `} /> + , + ) + + expect(lastFrame()).toBe('See specification requirements: [1]') + expect(lastFrame()).not.toContain(url) + }) + + test('parses multiple opt-in links in the same string', async () => { + vi.mocked(supportsHyperlinks).stdout = false + const first = 'https://example.com/a' + const second = 'https://example.com/b' + + const {lastFrame} = render( + + `} /> + , + ) + + expect(lastFrame()).toBe('see a [1] and [2]') + }) + + test('parses back-to-back opt-in links separated only by whitespace', async () => { + vi.mocked(supportsHyperlinks).stdout = false + const first = 'https://example.com/a' + const second = 'https://example.com/b' + + const {lastFrame} = render( + + <${second}>`} /> + , + ) + + expect(lastFrame()).toBe('[1] [2]') + }) + + test('does not parse markdown links that omit the http(s) scheme', async () => { + vi.mocked(supportsHyperlinks).stdout = true + + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toBe('see [the section](#anchor) for more') + expect(lastFrame()).not.toContain(']8;;') + }) + + test('does not parse opt-in markdown when no LinksContext is present (e.g. outside a Banner)', async () => { + vi.mocked(supportsHyperlinks).stdout = true + const url = 'https://example.com/docs' + + const {lastFrame} = render() + + expect(lastFrame()).toBe(`see [docs](${url}) now`) + expect(lastFrame()).not.toContain(']8;;') + }) + + test('echoing back a user-supplied URL inside an error message is left as plain text', async () => { + // Regression: an earlier auto-detection approach would turn the + // user's bad `--tunnel-url` value into a clickable OSC-8 link, + // which is misleading. With opt-in markdown the bare URL stays + // as-is and only the doc reference — which the server marks up — + // becomes clickable. + vi.mocked(supportsHyperlinks).stdout = true + const tunnelUrl = 'https://wrong' + const docUrl = 'https://shopify.dev/docs/tunnels' + + const {lastFrame} = render( + + + , + ) + + expect(lastFrame()).toContain(`Invalid tunnel URL: ${tunnelUrl}.`) + expect(lastFrame()).toContain(asOsc8Link(docUrl, 'tunnel docs')) + // The user-supplied URL must not be wrapped in an OSC 8 escape. + expect(lastFrame()).not.toContain(`\u001b]8;;${tunnelUrl}`) + }) + }) + describe('tokenItemToString', async () => { test("doesn't add a space before char", async () => { expect(tokenItemToString(['Run', {char: '!'}])).toBe('Run!') diff --git a/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx b/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx index 2c9905e01fc..6b3a05562a0 100644 --- a/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx +++ b/packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx @@ -4,7 +4,8 @@ import {List} from './List.js' import {UserInput} from './UserInput.js' import {FilePath} from './FilePath.js' import {Subdued} from './Subdued.js' -import React, {FunctionComponent} from 'react' +import {LinksContext} from '../contexts/LinksContext.js' +import React, {FunctionComponent, useContext} from 'react' import {Box, Text} from 'ink' export interface LinkToken { @@ -148,13 +149,74 @@ interface TokenizedTextProps { item: TokenItem } +// Matches CommonMark inline links of the form `[label](url)` and autolinks of +// the form ``. We deliberately require an explicit `http://` or +// `https://` scheme on the URL so callers can't accidentally trigger +// linkification by typing square brackets or angle brackets in plain prose. +// +// The URL portion forbids whitespace, the closing delimiter (`)` or `>`), and +// `<` to keep parsing predictable. URLs that legitimately contain those +// characters (e.g. balanced parens) must be percent-encoded by the caller — +// per CommonMark §6.3 / §6.4. +const MARKDOWN_LINK_REGEX = /\[([^[\]]+)\]\((https?:\/\/[^()<>\s]+)\)|<(https?:\/\/[^<>\s]+)>/g + +/** + * Parses an opt-in CommonMark link or autolink from a plain string token and + * routes the result through the existing `` component. + * + * Why opt-in rather than auto-detection: server-returned error strings can + * legitimately contain URL-shaped substrings that are not meant to be + * clickable (e.g. a tunnel-URL validation error echoing back the user's bad + * input). Requiring the explicit `[label](url)` / `` shape lets the + * source of the message (server or client) declare intent, and keeps prose + * with bare URLs untouched. + * + * Only invoked when a `LinksContext` is present (i.e. inside a Banner / + * Alert / FatalError); outside of that, the underlying `` would render + * the URL inline and we'd defeat the wrap-resistance the footnote mechanism + * provides. + */ +function renderStringWithMarkdownLinks(str: string): JSX.Element { + const matches = Array.from(str.matchAll(MARKDOWN_LINK_REGEX)) + if (matches.length === 0) { + return {str} + } + + const parts: JSX.Element[] = [] + let cursor = 0 + matches.forEach((match, index) => { + const start = match.index + if (start === undefined) { + return + } + const end = start + match[0].length + if (start > cursor) { + parts.push({str.slice(cursor, start)}) + } + // `[label](url)` captures label in group 1 and url in group 2; + // `` autolinks capture only the url in group 3. + const label = match[1] + const url = match[2] ?? match[3] + if (url === undefined) { + return + } + parts.push() + cursor = end + }) + if (cursor < str.length) { + parts.push({str.slice(cursor)}) + } + return {parts} +} + /** * `TokenizedText` renders a text string with tokens that can be either strings, * links, and commands. */ const TokenizedText: FunctionComponent = ({item}) => { + const linksContext = useContext(LinksContext) if (typeof item === 'string') { - return {item} + return linksContext === null ? {item} : renderStringWithMarkdownLinks(item) } else if ('command' in item) { return } else if ('link' in item) {