diff --git a/packages/app/src/cli/services/dev/ui.tsx b/packages/app/src/cli/services/dev/ui.tsx index 8477287ff4..ec359e9922 100644 --- a/packages/app/src/cli/services/dev/ui.tsx +++ b/packages/app/src/cli/services/dev/ui.tsx @@ -6,6 +6,7 @@ import {render} from '@shopify/cli-kit/node/ui' import {terminalSupportsPrompting} from '@shopify/cli-kit/node/system' import {isTruthy} from '@shopify/cli-kit/node/context/utilities' import {isUnitTest} from '@shopify/cli-kit/node/context/local' +import {outputDebug} from '@shopify/cli-kit/node/output' export async function renderDev({ processes, @@ -28,9 +29,19 @@ export async function renderDev({ organizationName?: string configPath?: string }) { - if (!terminalSupportsPrompting()) { + const supportsPrompting = terminalSupportsPrompting() + const supportsDevSessions = app.developerPlatformClient.supportsDevSessions + + outputDebug(`[renderDev] Terminal supports prompting: ${supportsPrompting}`) + outputDebug(`[renderDev] stdin.isTTY: ${process.stdin.isTTY}, stdout.isTTY: ${process.stdout.isTTY}`) + outputDebug(`[renderDev] Developer platform supports dev sessions: ${supportsDevSessions}`) + outputDebug(`[renderDev] Number of processes: ${processes.length}`) + + if (!supportsPrompting) { + outputDebug(`[renderDev] Using NON-INTERACTIVE mode (piping to process.stdout/stderr directly)`) await renderDevNonInteractive({processes, app, abortController, developerPreview, shopFqdn}) - } else if (app.developerPlatformClient.supportsDevSessions) { + } else if (supportsDevSessions) { + outputDebug(`[renderDev] Using DevSessionUI (interactive with dev sessions)`) return render( { expect(renderInstance.waitUntilExit().isFulfilled()).toBe(false) }) + + test('handles delayed/buffered writes correctly (simulates Ubuntu 24.04 issue #6726)', async () => { + // This test simulates the scenario where child process output may be + // delayed or buffered differently on certain Linux distributions. + // The issue manifests as hot reload working but terminal output being silent. + + const processSync = new Synchronizer() + const receivedOutput: string[] = [] + + const delayedProcess = { + prefix: 'web', + action: async (stdout: Writable, _stderr: Writable, _signal: AbortSignal) => { + // Simulate delayed writes like a real dev server would produce + stdout.write('Starting server...\n') + + // Small delay to simulate async server startup + await new Promise((resolve) => setTimeout(resolve, 10)) + stdout.write('Server listening on port 3000\n') + + // Another delay to simulate file change detection + await new Promise((resolve) => setTimeout(resolve, 10)) + stdout.write('File changed: index.tsx\n') + stdout.write('Rebuilding...\n') + + await new Promise((resolve) => setTimeout(resolve, 10)) + stdout.write('Build complete\n') + + processSync.resolve() + }, + } + + // When + const renderInstance = render( + , + ) + + await processSync.promise + // Give time for all writes to be processed + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Then - verify all messages were captured + const output = unstyled(renderInstance.lastFrame()!) + expect(output).toContain('Starting server...') + expect(output).toContain('Server listening on port 3000') + expect(output).toContain('File changed: index.tsx') + expect(output).toContain('Rebuilding...') + expect(output).toContain('Build complete') + }) + + test('handles rapid consecutive writes without dropping output', async () => { + // Tests for potential race conditions in output handling + const processSync = new Synchronizer() + const messageCount = 100 + + const rapidWriteProcess = { + prefix: 'rapid', + action: async (stdout: Writable, _stderr: Writable, _signal: AbortSignal) => { + // Rapidly write many messages without any delay + for (let i = 0; i < messageCount; i++) { + stdout.write(`message ${i}\n`) + } + processSync.resolve() + }, + } + + // When + const renderInstance = render( + , + ) + + await processSync.promise + await new Promise((resolve) => setTimeout(resolve, 100)) + + // Then - verify all messages were captured + const output = unstyled(renderInstance.lastFrame()!) + const lines = output.split('\n').filter((line) => line.includes('message')) + expect(lines.length).toBe(messageCount) + }) + + test('handles stderr output alongside stdout', async () => { + const processSync = new Synchronizer() + + const mixedOutputProcess = { + prefix: 'mixed', + action: async (stdout: Writable, stderr: Writable, _signal: AbortSignal) => { + stdout.write('stdout: normal output\n') + stderr.write('stderr: error output\n') + stdout.write('stdout: more output\n') + stderr.write('stderr: warning\n') + processSync.resolve() + }, + } + + // When + const renderInstance = render( + , + ) + + await processSync.promise + await new Promise((resolve) => setTimeout(resolve, 50)) + + // Then - both stdout and stderr should be captured + const output = unstyled(renderInstance.lastFrame()!) + expect(output).toContain('stdout: normal output') + expect(output).toContain('stderr: error output') + expect(output).toContain('stdout: more output') + expect(output).toContain('stderr: warning') + }) }) diff --git a/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx b/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx index da83738b86..c9f75782d2 100644 --- a/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx +++ b/packages/cli-kit/src/private/node/ui/components/ConcurrentOutput.tsx @@ -1,4 +1,4 @@ -import {OutputProcess} from '../../../../public/node/output.js' +import {OutputProcess, outputDebug} from '../../../../public/node/output.js' import {AbortSignal} from '../../../../public/node/abort.js' import React, {FunctionComponent, useCallback, useEffect, useMemo, useState} from 'react' import {Box, Static, Text, TextProps, useApp} from 'ink' @@ -132,24 +132,40 @@ const ConcurrentOutput: FunctionComponent = ({ const writableStream = useCallback( (process: OutputProcess, prefixes: string[]) => { return new Writable({ - write(chunk, _encoding, next) { - const context = outputContextStore.getStore() - const prefix = context?.outputPrefix ?? process.prefix - const shouldStripAnsi = context?.stripAnsi ?? true - const log = chunk.toString('utf8').replace(/(\n)$/, '') - - const index = addPrefix(prefix, prefixes) - - const lines = shouldStripAnsi ? stripAnsi(log).split(/\n/) : log.split(/\n/) - setProcessOutput((previousProcessOutput) => [ - ...previousProcessOutput, - { - color: lineColor(index), - prefix, - lines, - }, - ]) - next() + // Explicitly set options for cross-platform compatibility + // This addresses potential buffering issues on Ubuntu 24.04 (#6726) + decodeStrings: false, + defaultEncoding: 'utf8', + // Use a smaller high water mark to ensure data flows through quickly + highWaterMark: 16, + write(chunk, encoding, next) { + try { + const context = outputContextStore.getStore() + const prefix = context?.outputPrefix ?? process.prefix + const shouldStripAnsi = context?.stripAnsi ?? true + // Handle both Buffer and string chunks + const log = (typeof chunk === 'string' ? chunk : chunk.toString('utf8')).replace(/(\n)$/, '') + + outputDebug(`[ConcurrentOutput] Received chunk for prefix "${prefix}": ${log.substring(0, 100)}${log.length > 100 ? '...' : ''}`) + + const index = addPrefix(prefix, prefixes) + + const lines = shouldStripAnsi ? stripAnsi(log).split(/\n/) : log.split(/\n/) + outputDebug(`[ConcurrentOutput] Processing ${lines.length} line(s) for prefix "${prefix}"`) + + setProcessOutput((previousProcessOutput) => [ + ...previousProcessOutput, + { + color: lineColor(index), + prefix, + lines, + }, + ]) + next() + } catch (error) { + outputDebug(`[ConcurrentOutput] Error processing chunk: ${error}`) + next(error as Error) + } }, }) }, diff --git a/packages/cli-kit/src/public/node/system.test.ts b/packages/cli-kit/src/public/node/system.test.ts index 6019d294ae..cd1b93d975 100644 --- a/packages/cli-kit/src/public/node/system.test.ts +++ b/packages/cli-kit/src/public/node/system.test.ts @@ -1,6 +1,6 @@ import * as system from './system.js' import {execa} from 'execa' -import {describe, expect, test, vi} from 'vitest' +import {afterEach, describe, expect, test, vi} from 'vitest' import which from 'which' import {Readable} from 'stream' import * as fs from 'fs' @@ -113,3 +113,122 @@ describe('readStdinString', () => { expect(got).toBe('hello world') }) }) + +describe('getWslDistroName', () => { + test('returns the WSL_DISTRO_NAME environment variable when set', () => { + // Given + const originalEnv = process.env.WSL_DISTRO_NAME + process.env.WSL_DISTRO_NAME = 'Ubuntu' + + // When + const got = system.getWslDistroName() + + // Then + expect(got).toBe('Ubuntu') + + // Cleanup + process.env.WSL_DISTRO_NAME = originalEnv + }) + + test('returns undefined when WSL_DISTRO_NAME is not set', () => { + // Given + const originalEnv = process.env.WSL_DISTRO_NAME + delete process.env.WSL_DISTRO_NAME + + // When + const got = system.getWslDistroName() + + // Then + expect(got).toBeUndefined() + + // Cleanup + process.env.WSL_DISTRO_NAME = originalEnv + }) +}) + +describe('convertToWslFileUrl', () => { + afterEach(() => { + vi.resetModules() + }) + + test('converts path to WSL file URL when in WSL environment', async () => { + // Given + const originalEnv = process.env.WSL_DISTRO_NAME + process.env.WSL_DISTRO_NAME = 'Ubuntu' + + // Reset modules and mock is-wsl module + vi.resetModules() + vi.doMock('is-wsl', () => ({default: true})) + + // Re-import to get fresh module with mocked dependency + const {convertToWslFileUrl} = await import('./system.js') + + // When + const got = await convertToWslFileUrl('/tmp/test-file.html') + + // Then + expect(got).toBe('file://wsl.localhost/Ubuntu/tmp/test-file.html') + + // Cleanup + process.env.WSL_DISTRO_NAME = originalEnv + }) + + test('converts home directory path to WSL file URL', async () => { + // Given + const originalEnv = process.env.WSL_DISTRO_NAME + process.env.WSL_DISTRO_NAME = 'Ubuntu-22.04' + + // Reset modules and mock is-wsl module + vi.resetModules() + vi.doMock('is-wsl', () => ({default: true})) + + // Re-import to get fresh module with mocked dependency + const {convertToWslFileUrl} = await import('./system.js') + + // When + const got = await convertToWslFileUrl('/home/user/.nvm/versions/node/v20/lib/file.js') + + // Then + expect(got).toBe('file://wsl.localhost/Ubuntu-22.04/home/user/.nvm/versions/node/v20/lib/file.js') + + // Cleanup + process.env.WSL_DISTRO_NAME = originalEnv + }) + + test('returns standard file URL when not in WSL', async () => { + // Given + vi.resetModules() + vi.doMock('is-wsl', () => ({default: false})) + + // Re-import to get fresh module with mocked dependency + const {convertToWslFileUrl} = await import('./system.js') + + // When + const got = await convertToWslFileUrl('/tmp/test-file.html') + + // Then + expect(got).toBe('file:///tmp/test-file.html') + }) + + test('returns standard file URL when WSL_DISTRO_NAME is not set', async () => { + // Given + const originalEnv = process.env.WSL_DISTRO_NAME + delete process.env.WSL_DISTRO_NAME + + // Reset modules and mock is-wsl to return true, but distro name is not set + vi.resetModules() + vi.doMock('is-wsl', () => ({default: true})) + + // Re-import to get fresh module with mocked dependency + const {convertToWslFileUrl} = await import('./system.js') + + // When + const got = await convertToWslFileUrl('/tmp/test-file.html') + + // Then + expect(got).toBe('file:///tmp/test-file.html') + + // Cleanup + process.env.WSL_DISTRO_NAME = originalEnv + }) +}) diff --git a/packages/cli-kit/src/public/node/system.ts b/packages/cli-kit/src/public/node/system.ts index b906417431..2c9c479535 100644 --- a/packages/cli-kit/src/public/node/system.ts +++ b/packages/cli-kit/src/public/node/system.ts @@ -78,10 +78,20 @@ export async function exec(command: string, args: string[], options?: ExecOption } if (options?.stderr && options.stderr !== 'inherit') { + outputDebug(`[exec] Piping stderr for command: ${command}`) commandProcess.stderr?.pipe(options.stderr, {end: false}) + // Add debug listener to track data flow + commandProcess.stderr?.on('data', (chunk) => { + outputDebug(`[exec] stderr data received (${chunk.length} bytes) for: ${command}`) + }) } if (options?.stdout && options.stdout !== 'inherit') { + outputDebug(`[exec] Piping stdout for command: ${command}`) commandProcess.stdout?.pipe(options.stdout, {end: false}) + // Add debug listener to track data flow + commandProcess.stdout?.on('data', (chunk) => { + outputDebug(`[exec] stdout data received (${chunk.length} bytes) for: ${command}`) + }) } let aborted = false options?.signal?.addEventListener('abort', () => { @@ -137,6 +147,9 @@ function buildExec(command: string, args: string[], options?: ExecOptions): Exec windowsHide: false, detached: options?.background, cleanup: !options?.background, + // Disable buffering for stdout/stderr to ensure real-time streaming + // This helps address output swallowing issues on Ubuntu 24.04 (#6726) + buffer: false, }) outputDebug(`Running system process${options?.background ? ' in background' : ''}: ยท Command: ${command} ${args.join(' ')} @@ -241,3 +254,38 @@ export async function readStdinString(): Promise { } return data.trim() } + +/** + * Get the WSL distribution name from the environment. + * In WSL, the WSL_DISTRO_NAME environment variable contains the distribution name. + * + * @returns The WSL distribution name, or undefined if not in WSL or not available. + */ +export function getWslDistroName(): string | undefined { + return process.env.WSL_DISTRO_NAME +} + +/** + * Convert a Unix path to a WSL-compatible file URL that can be opened from Windows. + * In WSL, file paths need to be converted to the format: file://wsl.localhost//path + * + * @param unixPath - The Unix-style path to convert (e.g., /tmp/file.html or /home/user/file.js) + * @returns A file URL that works when opened from Windows in WSL environments. + * + * @example + * // In WSL with Ubuntu distro: + * // Input: /tmp/speedscope-123.html + * // Output: file://wsl.localhost/Ubuntu/tmp/speedscope-123.html + */ +export async function convertToWslFileUrl(unixPath: string): Promise { + const inWsl = await isWsl() + const distroName = getWslDistroName() + + if (inWsl && distroName) { + // Convert to WSL localhost format for Windows to access + return `file://wsl.localhost/${distroName}${unixPath}` + } + + // Not in WSL, return standard file URL + return `file://${unixPath}` +} diff --git a/packages/theme/src/cli/services/profile.test.ts b/packages/theme/src/cli/services/profile.test.ts index fdf86ce1af..2eaf11c583 100644 --- a/packages/theme/src/cli/services/profile.test.ts +++ b/packages/theme/src/cli/services/profile.test.ts @@ -1,14 +1,21 @@ import {profile} from './profile.js' import {render} from '../utilities/theme-environment/storefront-renderer.js' import {ensureAuthenticatedStorefront} from '@shopify/cli-kit/node/session' -import {openURL} from '@shopify/cli-kit/node/system' +import {openURL, convertToWslFileUrl} from '@shopify/cli-kit/node/system' import {vi, describe, expect, beforeEach, test} from 'vitest' import {outputResult} from '@shopify/cli-kit/node/output' import {AbortError} from '@shopify/cli-kit/node/error' import {readFile} from 'fs/promises' vi.mock('@shopify/cli-kit/node/session') -vi.mock('@shopify/cli-kit/node/system') +vi.mock('@shopify/cli-kit/node/system', () => { + const openURLMock = vi.fn() + const convertToWslFileUrlMock = vi.fn((path: string) => Promise.resolve(`file://${path}`)) + return { + openURL: openURLMock, + convertToWslFileUrl: convertToWslFileUrlMock, + } +}) vi.mock('@shopify/cli-kit/node/output') vi.mock('../utilities/theme-environment/storefront-password-prompt.js') vi.mock('../utilities/theme-environment/storefront-session.js') diff --git a/packages/theme/src/cli/services/profile.ts b/packages/theme/src/cli/services/profile.ts index 5aa13179c8..1bed844eb4 100644 --- a/packages/theme/src/cli/services/profile.ts +++ b/packages/theme/src/cli/services/profile.ts @@ -3,7 +3,7 @@ import {ensureValidPassword} from '../utilities/theme-environment/storefront-pas import {fetchDevServerSession} from '../utilities/theme-environment/dev-server-session.js' import {render} from '../utilities/theme-environment/storefront-renderer.js' import {resolveAssetPath} from '../utilities/asset-path.js' -import {openURL} from '@shopify/cli-kit/node/system' +import {openURL, convertToWslFileUrl} from '@shopify/cli-kit/node/system' import {joinPath} from '@shopify/cli-kit/node/path' import {AdminSession} from '@shopify/cli-kit/node/session' import {writeFile, tempDirectory} from '@shopify/cli-kit/node/fs' @@ -57,8 +57,8 @@ export async function profile( async function openProfile(profileJson: string) { // Adapted from https://github.com/jlfwong/speedscope/blob/146477a8508a6d2da697cb0ea0a426ba81b3e8dc/bin/cli.js#L63 - let urlToOpen = await resolveAssetPath('speedscope', 'index.html') - outputDebug(`[Theme Profile] Resolved URL to open: ${urlToOpen}`) + const speedscopePath = await resolveAssetPath('speedscope', 'index.html') + outputDebug(`[Theme Profile] Resolved speedscope path: ${speedscopePath}`) const filename = 'liquid-profile' const sourceBase64 = Buffer.from(profileJson).toString('base64') @@ -69,7 +69,18 @@ async function openProfile(profileJson: string) { outputDebug(`[Theme Profile] writing JS file to: ${jsPath}`) await writeFile(jsPath, jsSource) outputDebug(`[Theme Profile] JS file created successfully: ${jsPath}`) - urlToOpen += `#localProfilePath=${jsPath}` + + // Convert paths to WSL-compatible file URLs if running in WSL + // This is needed because Windows browsers can't access WSL paths directly + // They need the wsl.localhost// prefix (see issue #5414) + const speedscopeUrl = await convertToWslFileUrl(speedscopePath) + const jsFileUrl = await convertToWslFileUrl(jsPath) + outputDebug(`[Theme Profile] Converted speedscope URL: ${speedscopeUrl}`) + outputDebug(`[Theme Profile] Converted JS file URL: ${jsFileUrl}`) + + // Build the URL with the localProfilePath parameter + // In WSL, both the main URL and the localProfilePath need to use WSL-compatible paths + let urlToOpen = `${speedscopeUrl}#localProfilePath=${jsFileUrl}` // For some silly reason, the OS X open command ignores any query parameters or hash parameters // passed as part of the URL. To get around this weird issue, we'll create a local HTML file @@ -79,7 +90,8 @@ async function openProfile(profileJson: string) { await writeFile(htmlPath, ``) outputDebug(`[Theme Profile] HTML file created successfully: ${htmlPath}`) - urlToOpen = `file://${htmlPath}` + // Convert the HTML file path to a WSL-compatible URL as well + urlToOpen = await convertToWslFileUrl(htmlPath) outputDebug(`[Theme Profile] Opening URL: ${urlToOpen}`) const opened = await openURL(urlToOpen) outputDebug(`[Theme Profile] URL opened successfully: ${opened}`)