diff --git a/electron/handlers/projectHandlers.ts b/electron/handlers/projectHandlers.ts index 057ea640..21211bd7 100644 --- a/electron/handlers/projectHandlers.ts +++ b/electron/handlers/projectHandlers.ts @@ -8,7 +8,7 @@ import * as path from 'path'; import * as fs from 'fs'; import { spawn } from 'child_process'; import { scaffoldProject } from '../project/scaffold'; -import { createStarter } from '../project/starter'; +import { createStarter, newSiteDirectory } from '../project/starter'; import { isWin, errorMessage } from '../lib/mainHelpers'; import { ensureToolPath, run } from '../lib/nodeTools'; import { isAstroProject, detectPackageManager } from '../project/projectFiles'; @@ -46,26 +46,6 @@ function registerProjectOpenDialogHandlers({ } return { canceled: false as const, projectPath: directory }; }); - - ipcMain.handle('project:newDialog', async () => { - const result = await showOpenDialog({ - title: 'Choose an empty folder for the new project', - properties: ['openDirectory', 'createDirectory'], - }); - if (result.canceled || !result.filePaths.length) { - return { canceled: true as const }; - } - const directory = result.filePaths[0]; - assert(directory !== undefined, 'Accepted directory dialog must have a path'); - const entries = fs.readdirSync(directory).filter((name) => !name.startsWith('.')); - if (entries.length > 0) { - return { - canceled: false as const, - error: 'That folder is not empty. Choose or create an empty folder for the new project.', - }; - } - return { canceled: false as const, projectPath: directory }; - }); } function registerProjectCreateAstroHandlers({ @@ -76,20 +56,21 @@ function registerProjectCreateAstroHandlers({ // Runs the real `npm create astro@latest`, answering the questions the CLI // would ask interactively with the choices collected in the app's wizard. // Output is streamed to the renderer so the user sees the same progress the - // terminal would show. The chosen folder is the cwd and "." the target, so - // create-astro never has to guess a name from a parent directory. + // terminal would show. The wizard names the site, so its folder is made here, + // empty, and create-astro runs in it with "." as the target rather than + // guessing a name. ipcMain.handle('project:createAstro', async (_event, options) => { const { - dir: directory, + parentPath, + name, template = 'basics', install = true, git = true, ai = false, } = options || {}; - if (!directory || !fs.existsSync(directory)) { - throw new Error('Choose a folder for the new project first.'); - } + const directory = newSiteDirectory(parentPath, name); ensureToolPath(); // npm is a Node shim — same PATH problem as astro + fs.mkdirSync(directory); const args = [ 'create', @@ -106,8 +87,8 @@ function registerProjectCreateAstroHandlers({ ]; send('create:log', `> npm ${args.join(' ')}\n\n`); - await runCreateAstro({ send }, directory, args); - return { ok: true as const, installed: install }; + await runCreateAstroInNewFolder({ send }, directory, args); + return { ok: true as const, installed: install, projectPath: directory }; }); // A folder to put a new project IN, rather than the project's own folder: the @@ -189,6 +170,24 @@ export async function installDependencies({ send }: Pick, dire // a structured clone, which keeps the absent value. export const PROGRESS_CLEARED = { message: undefined } as const; +// A run that wrote nothing leaves no folder behind, so Try again can reuse the +// name. Anything it did write stays for the user to inspect; nothing is deleted +// recursively. +async function runCreateAstroInNewFolder( + { send }: Pick, + directory: string, + args: readonly string[], +): Promise { + try { + await runCreateAstro({ send }, directory, args); + } catch (error: unknown) { + if (fs.readdirSync(directory).length === 0) { + fs.rmdirSync(directory); + } + throw error; + } +} + // The create-astro run itself, its output streamed to the wizard's log. It // settles once the CLI exits and a package.json has landed in `directory`. function runCreateAstro( diff --git a/electron/preload/preloadBridge.ts b/electron/preload/preloadBridge.ts index 2811a03b..2c2a2a65 100644 --- a/electron/preload/preloadBridge.ts +++ b/electron/preload/preloadBridge.ts @@ -48,7 +48,6 @@ function projectBridge() { // Project: openProjectDialog: invoke('project:openDialog'), - newProjectDialog: invoke('project:newDialog'), scaffoldProject: invoke('project:scaffold'), createAstroProject: invoke('project:createAstro'), parentDialog: invoke('project:parentDialog'), diff --git a/electron/project/starter.ts b/electron/project/starter.ts index 91d630da..c2d3deae 100644 --- a/electron/project/starter.ts +++ b/electron/project/starter.ts @@ -121,27 +121,14 @@ async function createStarter({ if (!template) { throw new Error(`${starter} is not a starter this app knows.`); } - if (!parentPath || !fs.existsSync(parentPath)) { - throw new Error('Choose where the site should go first.'); - } - - const folder = String(name || '').trim(); - if (!folder) { - throw new Error('Give the site a name.'); - } - if (!NAME_RE.test(folder)) { - throw new Error('Use letters, numbers, dashes, dots or underscores for the folder name.'); - } - const directory = path.join(parentPath, folder); - if (fs.existsSync(directory)) { - throw new Error(`${folder} already exists in that folder.`); - } + const directory = newSiteDirectory(parentPath, name); + const folder = path.basename(directory); // `--no-install`: the app installs afterwards, where a failure is an error // the wizard can show rather than a line in a log that scrolled past. const args = ['create', template.create, folder, '--yes', '--', '--no-install']; onLog?.(`> npm create ${template.create} ${folder}\n\n`); - await run(npm || (isWin ? 'npm.cmd' : 'npm'), args, parentPath, onLog); + await run(npm || (isWin ? 'npm.cmd' : 'npm'), args, path.dirname(directory), onLog); if (!fs.existsSync(path.join(directory, 'package.json'))) { throw new Error('The starter finished but there is no package.json in it.'); @@ -195,4 +182,27 @@ async function createStarter({ return { ok: true, projectPath: directory }; } -export { createStarter, STARTERS }; +/** + * The folder a new site gets, `parentPath/name`, which must not exist yet. The + * starter and the Astro wizard both use it, so the name rules and their + * messages are the same in either flow. + */ +function newSiteDirectory(parentPath: string | undefined, name: string | undefined): string { + if (!parentPath || !fs.existsSync(parentPath)) { + throw new Error('Choose where the site should go first.'); + } + const folder = String(name || '').trim(); + if (!folder) { + throw new Error('Give the site a name.'); + } + if (!NAME_RE.test(folder)) { + throw new Error('Use letters, numbers, dashes, dots or underscores for the folder name.'); + } + const directory = path.join(parentPath, folder); + if (fs.existsSync(directory)) { + throw new Error(`${folder} already exists in that folder.`); + } + return directory; +} + +export { createStarter, newSiteDirectory, STARTERS }; diff --git a/shared/ipc/ipcPayloads.ts b/shared/ipc/ipcPayloads.ts index e6957958..c009581a 100644 --- a/shared/ipc/ipcPayloads.ts +++ b/shared/ipc/ipcPayloads.ts @@ -263,7 +263,8 @@ export const IPC_PAYLOADS = { 'project:classes': pathText, 'project:close': maybePath, 'project:createAstro': object({ - dir: pathText, + parentPath: pathText, + name: text, template: optional(text), install: maybeFlag, git: maybeFlag, @@ -273,7 +274,6 @@ export const IPC_PAYLOADS = { 'project:hasNodeModules': pathText, 'project:injectedRoutes': object({ ...project }), 'project:install': pathText, - 'project:newDialog': nothing, 'project:openDialog': nothing, 'project:parentDialog': nothing, 'project:pending': nothing, diff --git a/shared/ipc/ipcResults.ts b/shared/ipc/ipcResults.ts index 1a617a93..5c40e24b 100644 --- a/shared/ipc/ipcResults.ts +++ b/shared/ipc/ipcResults.ts @@ -559,6 +559,7 @@ export interface IpcResults { readonly 'project:createAstro': { readonly ok: true; readonly installed: boolean; + readonly projectPath: string; }; readonly 'project:createStarter': { readonly ok: boolean; @@ -571,22 +572,6 @@ export interface IpcResults { readonly 'project:install': { readonly ok: true; }; - readonly 'project:newDialog': - | { - readonly canceled: true; - readonly error?: never; - readonly projectPath?: never; - } - | { - readonly canceled: false; - readonly error: string; - readonly projectPath?: never; - } - | { - readonly canceled: false; - readonly projectPath: string; - readonly error?: never; - }; readonly 'project:openDialog': | { readonly canceled: true; diff --git a/shared/ipc/preloadApi.ts b/shared/ipc/preloadApi.ts index fc72e2d7..e0d36269 100644 --- a/shared/ipc/preloadApi.ts +++ b/shared/ipc/preloadApi.ts @@ -10,7 +10,6 @@ interface InvokeChannels { readonly studioStatus: 'studio:status'; readonly installStudio: 'studio:install'; readonly openProjectDialog: 'project:openDialog'; - readonly newProjectDialog: 'project:newDialog'; readonly scaffoldProject: 'project:scaffold'; readonly createAstroProject: 'project:createAstro'; readonly parentDialog: 'project:parentDialog'; diff --git a/src/app/styles.css b/src/app/styles.css index f6aa08fc..cdab3814 100644 --- a/src/app/styles.css +++ b/src/app/styles.css @@ -7982,10 +7982,13 @@ textarea::selection { } /* Start from a template: a name, where it goes, and what that means. */ -.starter-field { +/* Beats `.modal-body label { display: block }`, which would drop the gap and leave the + field's name jammed against its input. The gap matches the template question's 6px. */ +.modal-body label.starter-field { display: flex; flex-direction: column; gap: 6px; + margin-bottom: 0; } .starter-field > span { font-size: 11.5px; diff --git a/src/features/welcome/WelcomeScreen.tsx b/src/features/welcome/WelcomeScreen.tsx index 866c0526..7bc3813f 100644 --- a/src/features/welcome/WelcomeScreen.tsx +++ b/src/features/welcome/WelcomeScreen.tsx @@ -4,7 +4,6 @@ import WelcomeSplitButton from './WelcomeSplitButton'; import React, { useEffect, useRef, useState } from 'react'; import { chooseExistingProject, - chooseNewProjectDirectory, chooseStarterParent, listRecentProjects, refreshRecentThumbnail, @@ -24,10 +23,10 @@ interface WelcomeScreenProps { export default function WelcomeScreen({ onOpen, showToast }: WelcomeScreenProps) { const [cloning, setCloning] = useState(false); const [error, setError] = useState(undefined); - const [newProjectDirectory, setNewProjectDirectory] = useState(undefined); + const [newProjectParent, setNewProjectParent] = useState(undefined); const [starterDirectory, setStarterDirectory] = useState(undefined); const recent = useRecentProjects(showToast); - const actions = useWelcomeActions(onOpen, setError, setNewProjectDirectory, setStarterDirectory); + const actions = useWelcomeActions(onOpen, setError, setNewProjectParent, setStarterDirectory); return (
@@ -66,12 +65,12 @@ export default function WelcomeScreen({ onOpen, showToast }: WelcomeScreenProps) }} /> )} - {newProjectDirectory && ( + {newProjectParent && ( setNewProjectDirectory(undefined)} + parentPath={newProjectParent} + onClose={() => setNewProjectParent(undefined)} onDone={(directory) => { - setNewProjectDirectory(undefined); + setNewProjectParent(undefined); showToast('Project created', 'success'); onOpen(directory); }} @@ -84,7 +83,7 @@ export default function WelcomeScreen({ onOpen, showToast }: WelcomeScreenProps) function useWelcomeActions( onOpen: WelcomeScreenProps['onOpen'], setError: React.Dispatch>, - setNewProjectDirectory: React.Dispatch>, + setNewProjectParent: React.Dispatch>, setStarterDirectory: React.Dispatch>, ) { const openExisting = async (): Promise => { @@ -98,15 +97,14 @@ function useWelcomeActions( onOpen(result.value.projectPath); } }; + // Both wizards ask where the site should go and name its folder themselves. const createNew = async (): Promise => { setError(undefined); - const result = await chooseNewProjectDirectory(); + const result = await chooseStarterParent(); if (!result.ok) { setError(result.error); - } else if (!result.value.canceled && 'error' in result.value) { - setError(result.value.error); } else if (!result.value.canceled) { - setNewProjectDirectory(result.value.projectPath); + setNewProjectParent(result.value.parentPath); } }; const startFromLumos = async (): Promise => { diff --git a/src/features/welcome/WelcomeWizards.tsx b/src/features/welcome/WelcomeWizards.tsx index 5f749fff..acc96c84 100644 --- a/src/features/welcome/WelcomeWizards.tsx +++ b/src/features/welcome/WelcomeWizards.tsx @@ -9,6 +9,8 @@ import { } from './welcomeBridge'; const CREATE_LOG_CHARS_MAX = 20_000; +const SITE_NAME_DEFAULT = 'my-site'; +const SITE_NAME_PATTERN = /^[A-Za-z0-9._-]+$/; const TEMPLATES = [ { value: 'basics', label: 'Basics', hint: 'A basic, helpful starter project' }, { value: 'blog', label: 'Blog', hint: 'Content collections and post routing' }, @@ -35,11 +37,11 @@ export function StarterWizard({ onClose, onDone, }: WizardProps & { readonly parentPath: string }) { - const [name, setName] = useState('my-site'); + const [name, setName] = useState(SITE_NAME_DEFAULT); const [state, setState] = useState({ kind: 'idle' }); const log = useCreateLog(); const cleanName = name.trim(); - const invalid = !cleanName || !/^[A-Za-z0-9._-]+$/.test(cleanName); + const invalid = !siteNameValid(name); const running = state.kind === 'running'; const run = async (): Promise => { setState({ kind: 'running' }); @@ -69,25 +71,13 @@ export function StarterWizard({ )} > - -
- {`${parentPath}/${cleanName || 'my-site'}`} -
+ void run()} + />
Runs npm create lumos@latest, then starts a git history of its own — so publishing it later publishes your site, not a fork of the starter. @@ -99,10 +89,11 @@ export function StarterWizard({ } export function NewProjectWizard({ - directory, + parentPath, onClose, onDone, -}: WizardProps & { readonly directory: string }) { +}: WizardProps & { readonly parentPath: string }) { + const [name, setName] = useState(SITE_NAME_DEFAULT); const [template, setTemplate] = useState('basics'); const [install, setInstall] = useState(true); const [git, setGit] = useState(true); @@ -110,12 +101,14 @@ export function NewProjectWizard({ const [state, setState] = useState({ kind: 'idle' }); const log = useCreateLog(); const running = state.kind === 'running'; + const invalid = !siteNameValid(name); const run = async (): Promise => { setState({ kind: 'running' }); log.clear(); - const result = await createAstroProject({ directory, template, install, git, ai }); + const options = { parentPath, name: name.trim(), template, install, git, ai }; + const result = await createAstroProject(options); if (result.ok) { - onDone(directory); + onDone(result.value.projectPath); } else { setState({ kind: 'error', message: result.error }); } @@ -130,15 +123,19 @@ export function NewProjectWizard({ - )} > -
- {directory} -
+ void run()} + /> {state.kind === 'idle' && ( void; + readonly submit: () => void; +}) { + const folder = `${props.parentPath}/${props.name.trim() || SITE_NAME_DEFAULT}`; + return ( + <> + +
+ {folder} +
+ + ); +} + function useCreateLog() { const [text, setText] = useState(''); const elementRef = useRef(null); diff --git a/src/features/welcome/welcomeBridge.ts b/src/features/welcome/welcomeBridge.ts index 92919833..fde5ba70 100644 --- a/src/features/welcome/welcomeBridge.ts +++ b/src/features/welcome/welcomeBridge.ts @@ -20,7 +20,8 @@ export type ParentDialog = export type ProjectTemplate = 'basics' | 'blog' | 'starlight' | 'minimal'; export interface AstroProjectOptions { - readonly directory: string; + readonly parentPath: string; + readonly name: string; readonly template: ProjectTemplate; readonly install: boolean; readonly git: boolean; @@ -87,11 +88,6 @@ export function chooseExistingProject() { return welcomeRequest(parseProjectDialog, () => window.avb.openProjectDialog(payload)); } -export function chooseNewProjectDirectory() { - const payload = parseIpcPayload('project:newDialog', undefined); - return welcomeRequest(parseProjectDialog, () => window.avb.newProjectDialog(payload)); -} - export function chooseStarterParent() { const payload = parseIpcPayload('project:parentDialog', undefined); return welcomeRequest(parseParentDialog, () => window.avb.parentDialog(payload)); @@ -108,7 +104,8 @@ export function createStarterProject(parentPath: string, name: string) { export function createAstroProject(options: AstroProjectOptions) { const payload = parseIpcPayload('project:createAstro', { - dir: options.directory, + parentPath: options.parentPath, + name: options.name, template: options.template, install: options.install, git: options.git, @@ -150,7 +147,11 @@ function parseAstroResult(input: unknown): IpcResults['project:createAstro'] { if (!boolean(value['ok'])) { throw new Error('Create Astro project: expected success'); } - return { ok: true, installed: boolean(value['installed']) }; + return { + ok: true, + installed: boolean(value['installed']), + projectPath: pathText(value['projectPath']), + }; } async function welcomeRequest( diff --git a/test/electron/app/mainBoundaries.test.ts b/test/electron/app/mainBoundaries.test.ts index 966bda5a..8d0a2315 100644 --- a/test/electron/app/mainBoundaries.test.ts +++ b/test/electron/app/mainBoundaries.test.ts @@ -62,7 +62,7 @@ test('the complete channel inventory matches real main and terminal registration // Account sign-in adds one project-scoped channel for start, status, and cancellation. // Published-site discovery has its own channel so remote lookups cannot block local status. // Git pull and its background incoming check each have a project-scoped channel. - assert.equal(harness.handlers.size, 137); + assert.equal(harness.handlers.size, 136); assert.equal(terminalChannels.length, 4); assert.deepEqual( [...Object.keys(IPC_PAYLOADS)].sort(), diff --git a/test/renderer/welcome/welcome.test.js b/test/renderer/welcome/welcome.test.js index 0da30263..64360bca 100644 --- a/test/renderer/welcome/welcome.test.js +++ b/test/renderer/welcome/welcome.test.js @@ -195,13 +195,13 @@ const PIXEL = all('[role=menuitem]')[0]?.textContent === 'Empty Astro project…', ); let emptyRequests = 0; - dom.window.avb.newProjectDialog = async () => { + dom.window.avb.parentDialog = async () => { emptyRequests++; return { canceled: true }; }; await act(async () => all('[role=menuitem]')[0].click()); check( - 'the empty project option opens its folder picker and closes the menu', + 'the empty project option asks where the site should go and closes the menu', emptyRequests === 1 && all('[role=menuitem]').length === 0, ); await act(async () => arrows[0].click()); @@ -288,6 +288,65 @@ const PIXEL = ); } + // An empty Astro project is named the same way: where it goes, then its name. + { + dom.window.avb.parentDialog = async () => ({ canceled: false, parentPath: '/tmp/sites' }); + const created = []; + dom.window.avb.createAstroProject = async (payload) => { + created.push(payload); + return { ok: true, installed: true, projectPath: '/tmp/sites/blog-site' }; + }; + await act(async () => all('.actions button')[3].click()); + await act(async () => { + all('[role=menuitem]')[0].click(); + await settle(30); + }); + const wizard = dom.window.document.querySelector('.new-project-modal'); + check( + 'the Astro wizard opens after the folder', + /New Astro Project/.test(wizard?.textContent || ''), + wizard?.textContent?.slice(0, 80), + ); + check( + 'with the site name and the folder it becomes', + !!wizard?.querySelector('input') && /\/tmp\/sites\/my-site/.test(wizard?.textContent || ''), + wizard?.textContent?.slice(0, 200), + ); + const input = wizard.querySelector('input'); + const create = () => [...wizard.querySelectorAll('.modal-footer button')].at(-1); + await act(async () => { + Object.getOwnPropertyDescriptor(dom.window.HTMLInputElement.prototype, 'value').set.call( + input, + 'bad name/', + ); + input.dispatchEvent(new dom.window.Event('input', { bubbles: true })); + }); + check('a name that is not a folder name cannot be created', create().disabled === true); + await act(async () => { + Object.getOwnPropertyDescriptor(dom.window.HTMLInputElement.prototype, 'value').set.call( + input, + 'blog-site', + ); + input.dispatchEvent(new dom.window.Event('input', { bubbles: true })); + }); + await act(async () => { + create().click(); + await settle(30); + }); + check( + 'creating sends the parent folder and the name, not a folder of its own', + created.length === 1 && + created[0].parentPath === '/tmp/sites' && + created[0].name === 'blog-site' && + !('dir' in created[0]), + JSON.stringify(created), + ); + check( + 'and the wizard closes on the project it made', + !dom.window.document.querySelector('.new-project-modal'), + ); + } + // A first run has nothing to return to, so starting a site is the answer // rather than opening one. { diff --git a/test/shared/ipc/ipcPayloads.test.ts b/test/shared/ipc/ipcPayloads.test.ts index 9d927ece..be44ee01 100644 --- a/test/shared/ipc/ipcPayloads.test.ts +++ b/test/shared/ipc/ipcPayloads.test.ts @@ -26,7 +26,6 @@ const noPayload = new Set([ 'recents:list', 'git:repositories', 'project:openDialog', - 'project:newDialog', 'project:parentDialog', 'dev:stop', ]);