Skip to content

Auto-detect and linkify URLs in plain-string TokenizedText items #7384

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
ryancbahan wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Auto-detect and linkify URLs in plain-string TokenizedText items #7384

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
27 changes: 27 additions & 0 deletions packages/cli-kit/src/private/node/ui/components/Alert.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,33 @@ describe('Alert', async () => {
`)
})

test('auto-detects URLs in a plain-string body and renders them as footnotes', async () => {
const options = {
body: 'See specification requirements: https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties',
}

const {lastFrame} = render(<Alert type="error" {...options} />)
const frame = unstyled(lastFrame()!)

expect(frame).toMatchInlineSnapshot(`
"╭─ error ──────────────────────────────────────────────────────────────────────╮
│ │
│ See specification requirements: [1] │
│ │
╰──────────────────────────────────────────────────────────────────────────────╯
[1]
https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties
"
`)

const footnoteLines = frame.split('\n').filter((line) => line.includes('[1]'))
footnoteLines.forEach((line) => {
if (line.startsWith('[1]')) {
expect(line).not.toContain('│')
}
})
})

test('has the headline in bold', async () => {
const options = {
headline: 'Title.',
Expand Down
20 changes: 20 additions & 0 deletions packages/cli-kit/src/private/node/ui/components/FatalError.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,26 @@ describe('FatalError', async () => {
`)
})

test('auto-detects URLs in a plain-string error message and renders them as footnotes', async () => {
const error = new AbortError(
'See specification requirements: https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties',
)

const {lastFrame} = render(<FatalError error={error} />)
const frame = unstyled(lastFrame()!)

expect(frame).toMatchInlineSnapshot(`
"╭─ error ──────────────────────────────────────────────────────────────────────╮
│ │
│ See specification requirements: [1] │
│ │
╰──────────────────────────────────────────────────────────────────────────────╯
[1]
https://shopify.dev/docs/apps/build/sales-channels/channel-config-extension#specification-properties
"
`)
})

test('renders correctly with a formatted message', async () => {
const error = new AbortError([
'There has been an error creating your deployment:',
Expand Down
6 changes: 5 additions & 1 deletion packages/cli-kit/src/private/node/ui/components/FatalError.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,11 @@ const FatalError: FunctionComponent<FatalErrorProps> = ({error}) => {
</Text>
) : null}

{error.formattedMessage ? <TokenizedText item={error.formattedMessage} /> : <Text>{error.message}</Text>}
{error.formattedMessage ? (
<TokenizedText item={error.formattedMessage} />
) : (
<TokenizedText item={error.message} />
)}

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

Expand Down
9 changes: 4 additions & 5 deletions packages/cli-kit/src/private/node/ui/components/Link.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,16 +12,15 @@ 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
}

const linkId = linksContext.addLink(label, url)
return `${label ?? url} [${linkId}]`
return label ? `${label} [${linkId}]` : `[${linkId}]`
}

return ansiEscapes.link(label ?? url, url)
Expand Down
54 changes: 53 additions & 1 deletion packages/cli-kit/src/private/node/ui/components/TokenizedText.test.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import {tokenItemToString, TokenizedText} from './TokenizedText.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'

vi.mock('supports-hyperlinks')

describe('TokenizedText', async () => {
test('renders arrays of items separated by spaces', async () => {
const item = [
Expand Down Expand Up @@ -57,6 +60,55 @@ describe('TokenizedText', async () => {
`)
})

describe('URL auto-detection in plain strings', async () => {
test('renders strings without URLs unchanged', async () => {
vi.mocked(supportsHyperlinks).stdout = false

const {lastFrame} = render(<TokenizedText item="no link here, just text" />)

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

test('preserves a URL intact 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(<TokenizedText item={`See specification requirements: ${url}`} />)

expect(lastFrame()).toContain(url)
})

test('wraps detected URLs in OSC 8 escapes when the terminal supports hyperlinks', async () => {
vi.mocked(supportsHyperlinks).stdout = true
const url = 'https://example.com/docs'

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

expect(lastFrame()).toContain(`]8;;${url}${url}]8;;`)
})

test('detects multiple URLs in the same string', async () => {
Copy link
Copy Markdown

@johnnie-chen johnnie-chen Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be worth adding edge test cases where there's back to back URLs

vi.mocked(supportsHyperlinks).stdout = true
const first = 'https://example.com/a'
const second = 'https://example.com/b'

const {lastFrame} = render(<TokenizedText item={`see ${first} and ${second}`} />)

expect(lastFrame()).toContain(`]8;;${first}${first}]8;;`)
expect(lastFrame()).toContain(`]8;;${second}${second}]8;;`)
})

test('strips trailing sentence punctuation from detected URLs', async () => {
vi.mocked(supportsHyperlinks).stdout = true
const url = 'https://example.com/docs'

const {lastFrame} = render(<TokenizedText item={`see ${url}. Thanks`} />)

expect(lastFrame()).toContain(`]8;;${url}${url}]8;;`)
expect(lastFrame()).toContain('. Thanks')
})
})

describe('tokenItemToString', async () => {
test("doesn't add a space before char", async () => {
expect(tokenItemToString(['Run', {char: '!'}])).toBe('Run!')
Expand Down
33 changes: 32 additions & 1 deletion packages/cli-kit/src/private/node/ui/components/TokenizedText.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,40 @@ interface TokenizedTextProps {
* `TokenizedText` renders a text string with tokens that can be either strings,
* links, and commands.
*/
const URL_REGEX = /https?:\/\/\S+/g
const URL_TRAILING_PUNCTUATION = /[.,;:!?)\]}>'"]+$/
Copy link
Copy Markdown
Contributor

@isaacroldan isaacroldan Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐛 Bug: URL_TRAILING_PUNCTUATION = /[.,;:!?)\]}>'"]+$/ unconditionally strips ), ], }, and > from the end of a matched URL, but these characters are legal URL suffixes. Real-world examples that would break: Wikipedia disambiguation links like https://en.wikipedia.org/wiki/Ruby_(programming_language), MDN Function() pages, Microsoft Learn URLs using parens in paths, and query strings ending in JSON-array-style values. Because this helper is now the chokepoint for every plain string passed through TokenizedText, any such URL in a server-returned error message will have its closing bracket stripped, and the <Link> component will emit an OSC 8 escape pointing at a broken target (with the trailing ) rendered outside the link). None of the new tests exercise URLs containing (), so this silent corruption is uncovered.

Suggestion: Narrow the character class to unambiguous sentence punctuation, or balance-count parens before stripping. Minimal fix:

Suggested change
const URL_TRAILING_PUNCTUATION = /[.,;:!?)\]}>'"]+$/
const URL_TRAILING_PUNCTUATION = /[.,;:!?'"]+$/

And add a test for https://en.wikipedia.org/wiki/Foo_(bar) to lock the behavior in.

Copy link
Copy Markdown

@johnnie-chen johnnie-chen Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1 on this


function renderStringWithLinks(str: string): JSX.Element {
const matches = Array.from(str.matchAll(URL_REGEX))
if (matches.length === 0) {
return <Text>{str}</Text>
}

const parts: JSX.Element[] = []
let cursor = 0
matches.forEach((match, index) => {
let url = match[0]
const trailing = url.match(URL_TRAILING_PUNCTUATION)
if (trailing) {
url = url.slice(0, url.length - trailing[0].length)
}
const start = match.index
const end = start + url.length
if (start > cursor) {
parts.push(<Text key={`t${index}`}>{str.slice(cursor, start)}</Text>)
}
parts.push(<Link key={`l${index}`} url={url} />)
cursor = end
})
if (cursor < str.length) {
parts.push(<Text key="tail">{str.slice(cursor)}</Text>)
}
return <Text>{parts}</Text>
}

const TokenizedText: FunctionComponent<TokenizedTextProps> = ({item}) => {
if (typeof item === 'string') {
return <Text>{item}</Text>
return renderStringWithLinks(item)
Copy link
Copy Markdown
Contributor

@isaacroldan isaacroldan Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

📐 Design: The PR is framed as a Banner/alert fix, but renderStringWithLinks runs on every plain-string token passing through TokenizedText. That includes Alert, FatalError, List, TabularData, DangerousConfirmationPrompt, Prompts/PromptLayout, Prompts/InfoMessage, and TextPrompt. In non-Banner contexts there's no LinksContext, so the Link component falls back to raw-URL output for non-OSC-8 terminals (preserved) but emits OSC 8 escapes on OSC-8-capable terminals (behavior change). A concrete example: the placeholder URL in packages/app/src/cli/services/dev/urls.ts:86 (Valid format: "https://my-tunnel-url:port") will now be rendered as a clickable escape to a non-existent host. Reviewers of downstream packages won't catch this from the PR description alone.

Suggestion: Either (a) scope the auto-detection to contexts that own a LinksContext by threading an explicit flag through (e.g. an autoLinkify prop or opt-in from Banner/Alert/FatalError), or (b) keep the current broad behavior but update the PR description/changeset and add snapshot coverage for non-Banner callers (TabularData, List, prompts) so the behavior change is visible at review time.

Copy link
Copy Markdown
Contributor

@isaacroldan isaacroldan Apr 24, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this and does indeed break other alerts:

Captura de pantalla 2026-04-24 a las 10.50.29.png

Copy link
Copy Markdown
Contributor

@isaacroldan isaacroldan Apr 24, 2026 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can trigger this one by running:

pnpm shopify app dev --tunnel-url https://wrong

} else if ('command' in item) {
return <Command command={item.command} />
} else if ('link' in item) {
Expand Down
Loading