Skip to content

Show the directory the app created, from the moment it creates it #213

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

Closed
juanmaguitar wants to merge 1 commit into from
Closed

Show the directory the app created, from the moment it creates it #213

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
119 changes: 74 additions & 45 deletions src/renderer/index.jsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import { planDevServerStart, formatElapsed } from './dev-server-command.cjs';
import { pathBasename } from './path-basename.cjs';
import { trunkAgeInfo, planUpdateSteps, updateStepStatuses, SKIP_INSTALL_MESSAGE, planApplySteps, APPLY_STATE_TO_STEP } from './update-plan.cjs';
import { pickLatest } from '../latest-patch.cjs';
import { beginSetup, adoptSetupPath, discardSetup, rowPathAfterStatus } from './pending-setup.cjs';
import { parsePrRef } from '../patch-sources.cjs';
import { ticketUrl, attachUrl } from './trac-ticket.cjs';
import { highlightDiff } from './diff-highlight.cjs';
Expand Down Expand Up @@ -190,25 +191,41 @@ function useContributorProvenance() {
return { handle, event, rememberHandle, rememberEvent };
}

// The two halves are one piece of state because one change moves both: adopting
// the directory the app really created has to retire the guessed row and carry
// its metadata across, and two setters cannot do that without a render in
// between where the site has a path under one key and a label under another.
// `setSiteMeta` keeps its old signature so every other caller is untouched;
// `applySetup` is for the changes that need the pair, which is every change the
// create flow makes.
function useSites() {
const [sites, setSites] = useState([]);
const [siteMeta, setSiteMeta] = useState({});
const [state, setState] = useState({ sites: [], siteMeta: {} });
const setSiteMeta = useCallback((update) => setState((prev) => ({
...prev,
siteMeta: typeof update === 'function' ? update(prev.siteMeta) : update
})), []);
const applySetup = useCallback((fn) => setState(fn), []);
const refresh = useCallback(async () => {
const { sites: list, siteMeta: meta } = await window.api.getSitesWithMeta();
setSites(list);
setSiteMeta(meta || {});
setState({ sites: list, siteMeta: meta || {} });
}, []);
useEffect(() => { refresh(); }, [refresh]);
return { sites, siteMeta, refresh, setSiteMeta, setSites };
return { sites: state.sites, siteMeta: state.siteMeta, refresh, setSiteMeta, applySetup };
}

function App() {
const { sites, siteMeta, refresh, setSiteMeta, setSites } = useSites();
const { sites, siteMeta, refresh, setSiteMeta, applySetup } = useSites();
// One answer for the window, shared by every site row: which applications this
// machine has is a fact about the machine, not about a site.
const detectedApplications = useDetectedEditors();
const wporg = useContributorProvenance();
const [downloadPhase, setDownloadPhase] = useState('');
// Where the row for the setup in flight currently lives. It starts as the
// window's guess and becomes the directory the main process reports, and it
// is a ref because the status subscription below has to read it without being
// torn down and rebuilt every time it changes. Null when nothing is being
// created.
const setupRowPathRef = useRef(null);
// Directories whose clone is still running. An array rather than a single
// path because the main process may settle on a different (deduplicated)
// directory than the one the renderer optimistically created a row for.
Expand Down Expand Up @@ -330,6 +347,22 @@ function App() {
});
const unsubStat = window.api.subscribeSetupStatus((s) => {
if (!s) return;
// The first moment the window learns where the site is actually being
// made. Until now the row kept the guess it was drawn with, which differs
// whenever the folder name was taken — so it showed the wrong path and,
// once the guards started keying on the real directory, asked about a
// folder the app had never created (#180).
const guess = setupRowPathRef.current;
const adopted = rowPathAfterStatus(guess, s);
if (adopted) {
setupRowPathRef.current = adopted;
moveSetupLog(guess, adopted);
applySetup((state) => adoptSetupPath(state, { from: guess, to: adopted }));
// The selection follows the row. Without this the panel is pointed at a
// path that no longer exists in the list, and the contributor watches
// their new site's checklist disappear mid-clone.
setActiveSite((current) => (current === guess ? adopted : current));
}
if (s.target && s.phase !== 'done') addPendingSite(s.target);
const key = s.sitePath || s.target;
if (key) {
Expand All @@ -341,14 +374,22 @@ function App() {
else if (s.phase === 'done') { setDownloadPhase(''); clearPendingSites(); setTerminalMsgs(''); }
});
return () => { if (unsubProg) unsubProg(); if (unsubStat) unsubStat(); };
}, [addPendingSite, appendSetupLog, clearPendingSites]);

}, [addPendingSite, appendSetupLog, applySetup, clearPendingSites, moveSetupLog]);

// Refused while one is already running. Everything about this flow is
// single-file and always has been — one pending card, one terminal, one
// `clearPendingSites()` that clears them all — and `setupRowPathRef` is one
// slot for the row being created. The button was the only door left open on a
// second setup, and a second setup does not half-work: it adopts the other
// one's row. Until the flow is genuinely per-site, saying no is the honest
// shape.
const chooseAndSetup = useCallback(() => {
if (createSubmitting) return;
setCreateSiteName('');
setCreateSiteDir('');
setCreateSiteError('');
setCreateModalOpen(true);
}, []);
}, [createSubmitting]);

const sanitizeSiteFolder = useCallback((value) => (
value
Expand Down Expand Up @@ -431,15 +472,11 @@ function App() {
let finalSitePath = targetDir;
const placeholderCreatedAt = new Date().toISOString();

setSites((prev) => (prev.includes(targetDir) ? prev : [...prev, targetDir]));
setSiteMeta((prev = {}) => ({
...prev,
[targetDir]: {
...(prev[targetDir] || {}),
label: nameTrimmed,
createdAt: prev[targetDir]?.createdAt || placeholderCreatedAt,
initialized: false
}
setupRowPathRef.current = targetDir;
applySetup((state) => beginSetup(state, {
path: targetDir,
label: nameTrimmed,
createdAt: placeholderCreatedAt
}));
setActiveSite(targetDir);
setCreateModalOpen(false);
Expand All @@ -455,48 +492,38 @@ function App() {
const createdPath = await window.api.setupWordPress(createSiteDir, { siteName: cleanFolder, siteLabel: nameTrimmed });
if (createdPath) {
finalSitePath = createdPath;
if (createdPath !== targetDir) {
// Ordinarily already done, by the `cloning` status this handler's own
// clone sent minutes ago. Kept because the status event is not a
// guarantee — a missed one would otherwise leave the row on the guess
// for good — and adopting a path the row already has is a no-op.
const current = setupRowPathRef.current;
if (current && current !== createdPath) {
addPendingSite(createdPath);
moveSetupLog(targetDir, createdPath);
setSites((prev) => {
const filtered = prev.filter((path) => path !== targetDir);
return filtered.includes(createdPath) ? filtered : [...filtered, createdPath];
});
setSiteMeta((prev = {}) => {
const next = { ...prev };
const placeholder = next[targetDir] || { createdAt: placeholderCreatedAt, initialized: false };
delete next[targetDir];
next[createdPath] = {
...placeholder,
label: nameTrimmed,
createdAt: placeholder.createdAt || placeholderCreatedAt,
initialized: false
};
return next;
});
moveSetupLog(current, createdPath);
applySetup((state) => adoptSetupPath(state, { from: current, to: createdPath }));
}
setupRowPathRef.current = createdPath;
}
await refresh();
setActiveSite(finalSitePath);
appendSetupLog(finalSitePath, 'Site setup request completed.\n');
} catch (e) {
// Whatever the row is *now*, which is not necessarily what it started as:
// once the clone reports its directory the guess no longer exists, and
// discarding the guess here would strand a row for a setup that failed.
const rowPath = setupRowPathRef.current || targetDir;
setCreateSiteError(String(e));
appendSetupLog(targetDir, `Setup failed: ${String(e)}\n`);
setSites((prev) => prev.filter((path) => path !== targetDir));
setSiteMeta((prev = {}) => {
if (!prev[targetDir]) return prev;
const next = { ...prev };
delete next[targetDir];
return next;
});
appendSetupLog(rowPath, `Setup failed: ${String(e)}\n`);
applySetup((state) => discardSetup(state, rowPath));
} finally {
setupRowPathRef.current = null;
// `setupWordPress` resolving (or throwing) *is* the clone finishing, so
// clearing here guarantees the checklist can never stay locked even if
// the `done` status event is missed.
clearPendingSites();
setCreateSubmitting(false);
}
}, [addPendingSite, appendSetupLog, clearPendingSites, createSiteDir, createSiteName, moveSetupLog, refresh, resolveTargetDir, sanitizeSiteFolder, setSiteMeta, setSites]);
}, [addPendingSite, appendSetupLog, applySetup, clearPendingSites, createSiteDir, createSiteName, moveSetupLog, refresh, resolveTargetDir, sanitizeSiteFolder]);

const closeCreateModal = useCallback(() => {
if (createSubmitting) return;
Expand Down Expand Up @@ -736,8 +763,10 @@ function App() {
icon={plus}
variant="primary"
onClick={chooseAndSetup}
disabled={createSubmitting}
style={{ width: '100%', justifyContent: 'center' }}
aria-label="Create WordPress Core site"
label={createSubmitting ? 'Finish creating the current site first' : undefined}
>
{!sidebarCollapsed ? 'Create WordPress Core site' : null}
</Button>
Expand Down
123 changes: 123 additions & 0 deletions src/renderer/pending-setup.cjs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,123 @@
// The row for a site that is being created, while it is being created.
//
// The window has to draw one before it can know where the site will be. The
// contributor picked a parent directory and typed a name; the main process is
// the one that turns those into a directory, and it may not use the name it was
// given — `findAvailableDirName` appends `-2` when the folder already exists.
// So the row starts on a guess.
//
// A guess was harmless while it was only a label. It stopped being harmless
// when the guards started keying on the directory the app actually created
// (#180): the row hands its path to `dir:show` and `editor:open`, so on a
// collision it was asking about a folder the app had never made, and being
// refused for it. Worse, when the guessed name belongs to a *different*
// registered site, it is asking about that one.
//
// Main reports the real path on its first status event, minutes before the
// clone ends. Adopting it there makes the row honest for the whole clone — the
// path under the title included, which until now simply read wrong.
//
// The three moves are here, and pure, because there were three divergent copies
// of them in the component and because adopting earlier is what makes the third
// one dangerous: the discard branch used to filter the guessed path, which was
// only ever safe because the swap could not have happened yet.
//
// Every function returns new state and leaves its argument alone — these feed
// React setState updaters, which may run more than once.
'use strict';

/**
* The optimistic row, before the main process has answered.
*
* @param {{sites: string[], siteMeta: Object}} state
* @param {{path: string, label: string, createdAt: string}} site
* @return {{sites: string[], siteMeta: Object}}
*/
function beginSetup({ sites, siteMeta }, { path, label, createdAt }) {
return {
sites: sites.includes(path) ? sites : [...sites, path],
siteMeta: {
...siteMeta,
[path]: {
...(siteMeta[path] || {}),
label,
createdAt: siteMeta[path]?.createdAt || createdAt,
initialized: false
}
}
};
}

/**
* Moves the row from the guessed path to the one the app created.
*
* Everything the contributor supplied moves with it, `createdAt` included:
* losing that would drop the row to the bottom of a sidebar sorted by it, in
* the middle of watching the site being made.
*
* @param {{sites: string[], siteMeta: Object}} state
* @param {{from: string, to: string}} move
* @return {{sites: string[], siteMeta: Object}} The same state when there is
* nothing to move — main reports the path more than once per setup.
*/
function adoptSetupPath(state, { from, to }) {
if (from === to) return state;
if (!state.sites.includes(from)) return state;

const kept = state.siteMeta[from];
const siteMeta = { ...state.siteMeta };
delete siteMeta[from];

const withoutGuess = state.sites.filter((p) => p !== from);
return {
sites: withoutGuess.includes(to) ? withoutGuess : [...withoutGuess, to],
siteMeta: { ...siteMeta, [to]: { ...(siteMeta[to] || {}), ...kept } }
};
}

/**
* Drops the row for a setup that failed.
*
* The caller passes the path the row currently has, not the one it started
* with. That distinction is the whole reason this is not a one-liner at the
* call site: after an adoption the guess no longer exists, and filtering it
* would leave the real row behind for a directory whose setup just failed.
*
* @param {{sites: string[], siteMeta: Object}} state
* @param {string} path
* @return {{sites: string[], siteMeta: Object}}
*/
function discardSetup(state, path) {
const siteMeta = { ...state.siteMeta };
delete siteMeta[path];
return { sites: state.sites.filter((p) => p !== path), siteMeta };
}

/**
* The path the in-flight row should move to for a status event, or null when it
* should stay where it is.
*
* Extracted from the subscription because the decision, not the reducer, is
* where this went wrong. The reducer is new code that no old test could have
* failed on; what #180's collision case actually needed was *which* event to
* believe, and a first version that believed any event whose target differed
* let a finishing setup's `done` drag a second setup's row onto its own path.
*
* `cloning` is the one event that announces the directory and arrives exactly
* once per setup, so it is the only one that moves a row. Everything else — no
* setup in flight, a later phase, a target that is already the row's — is "stay
* put", returned as null rather than as a path equal to the current one, so the
* caller cannot accidentally treat it as a move.
*
* @param {?string} currentPath Where the in-flight row is now, or null.
* @param {?Object} status A download:status payload from the main process.
* @return {?string}
*/
function rowPathAfterStatus(currentPath, status) {
if (typeof currentPath !== 'string' || currentPath === '') return null;
if (!status || status.phase !== 'cloning') return null;
if (typeof status.target !== 'string' || status.target === '') return null;
return status.target === currentPath ? null : status.target;
}

module.exports = { beginSetup, adoptSetupPath, discardSetup, rowPathAfterStatus };
Loading
Loading