Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 34 additions & 0 deletions packages/cli-kit/src/private/node/ui/components/Alert.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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(<Alert type="error" {...options} />)
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]')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -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 `<Text>`,
// 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(<FatalError error={error} />)
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]')
})
})
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,15 @@ const FatalError: FunctionComponent<FatalErrorProps> = ({error}) => {
</Text>
) : null}

{error.formattedMessage ? <TokenizedText item={error.formattedMessage} /> : <Text>{error.message}</Text>}
{error.formattedMessage ? (
<TokenizedText item={error.formattedMessage} />
) : (
// Route plain-string error messages through TokenizedText so that any
// opt-in `[label](url)` / `<url>` markdown the server (or the caller)
// emits is rendered as a footnote-backed Link inside the FatalError
// banner.
<TokenizedText item={error.message} />
)}

{error.tryMessage ? <TokenizedText item={error.tryMessage} /> : null}

Expand Down
63 changes: 63 additions & 0 deletions packages/cli-kit/src/private/node/ui/components/Link.test.tsx
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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<string, {label: string | undefined; url: string}> = {}
const link = {
url: 'https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties',
label: 'docs',
}

const {lastFrame} = render(
<LinksContext.Provider
value={{
links: {current: links},
addLink: (label, url) => {
const id = (Object.keys(links).length + 1).toString()
links[id] = {label, url}
return id
},
}}
>
<Link {...link} />
</LinksContext.Provider>,
)

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<string, {label: string | undefined; url: string}> = {}
const link = {
url: 'https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties',
}

const {lastFrame} = render(
<LinksContext.Provider
value={{
links: {current: links},
addLink: (label, url) => {
const id = (Object.keys(links).length + 1).toString()
links[id] = {label, url}
return id
},
}}
>
<Link {...link} />
</LinksContext.Provider>,
)

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
}
Expand Down
13 changes: 8 additions & 5 deletions packages/cli-kit/src/private/node/ui/components/Link.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
@@ -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 `<Link>` 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 <Banner> sets up at runtime, without pulling
// the whole Banner border into these tests.
const WithLinksContext: FunctionComponent<{children: React.ReactNode}> = ({children}) => {
const links = useRef<Record<string, Link>>({})
return (
<LinksContext.Provider
value={{
links,
addLink: (label, url) => {
const newId = (Object.keys(links.current).length + 1).toString()
links.current = {...links.current, [newId]: {label, url}}
return newId
},
}}
>
{children}
</LinksContext.Provider>
)
}

describe('TokenizedText', async () => {
test('renders arrays of items separated by spaces', async () => {
Expand Down Expand Up @@ -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(
<WithLinksContext>
<TokenizedText item="no link here, just text" />
</WithLinksContext>,
)

expect(lastFrame()).toBe('no link here, just text')
})

test('does not linkify a bare URL — callers must opt in via `[label](url)` or `<url>`', async () => {
vi.mocked(supportsHyperlinks).stdout = true
const url = 'https://example.com/docs'

const {lastFrame} = render(
<WithLinksContext>
<TokenizedText item={`visit ${url} now`} />
</WithLinksContext>,
)

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(
<WithLinksContext>
<TokenizedText item={`Reference: [See specification requirements](${url})`} />
</WithLinksContext>,
)

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(
<WithLinksContext>
<TokenizedText item={`Reference: [docs page](${url})`} />
</WithLinksContext>,
)

expect(lastFrame()).toContain(asOsc8Link(url, 'docs page'))
})

test('renders a label-less `<url>` 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(
<WithLinksContext>
<TokenizedText item={`See specification requirements: <${url}>`} />
</WithLinksContext>,
)

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(
<WithLinksContext>
<TokenizedText item={`see [a](${first}) and <${second}>`} />
</WithLinksContext>,
)

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(
<WithLinksContext>
<TokenizedText item={`<${first}> <${second}>`} />
</WithLinksContext>,
)

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(
<WithLinksContext>
<TokenizedText item="see [the section](#anchor) for more" />
</WithLinksContext>,
)

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(<TokenizedText item={`see [docs](${url}) now`} />)

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(
<WithLinksContext>
<TokenizedText item={`Invalid tunnel URL: ${tunnelUrl}. See [tunnel docs](${docUrl}).`} />
</WithLinksContext>,
)

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!')
Expand Down
Loading
Loading