Skip to content
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.

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

Propagate updates to tsserver and client when a cell is updated #130

Merged
merged 2 commits into from
Jul 12, 2024
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
31 changes: 18 additions & 13 deletions packages/api/server/ws.mts
Original file line number Diff line number Diff line change
Expand Up @@ -266,20 +266,20 @@ async function cellUpdate(payload: CellUpdatePayloadType) {
throw new Error(`No session exists for session '${payload.sessionId}'`);
}

const cell = findCell(session, payload.cellId);
const cellBeforeUpdate = findCell(session, payload.cellId);

if (!cell) {
if (!cellBeforeUpdate) {
throw new Error(
`No cell exists for session '${payload.sessionId}' and cell '${payload.cellId}'`,
);
}

const result = await updateCell(session, cell, payload.updates);
const result = await updateCell(session, cellBeforeUpdate, payload.updates);

if (!result.success) {
wss.broadcast(`session:${session.id}`, 'cell:error', {
sessionId: session.id,
cellId: cell.id,
cellId: payload.cellId,
errors: result.errors,
});

Expand All @@ -293,26 +293,31 @@ async function cellUpdate(payload: CellUpdatePayloadType) {

if (
session.metadata.language === 'typescript' &&
result.cell.type === 'code' &&
cellBeforeUpdate.type === 'code' &&
tsservers.has(session.id)
) {
const cell = result.cell;
const cellAfterUpdate = result.cell as CodeCellType;
const tsserver = tsservers.get(session.id);

const file = pathToCodeFile(session.dir, cell.filename);
// These are usually the same. However, if the user renamed the cell, we need to
// ensure that we close the old file in tsserver and open the new one.
const oldFilePath = pathToCodeFile(session.dir, cellBeforeUpdate.filename);
const newFilePath = pathToCodeFile(session.dir, cellAfterUpdate.filename);

// To update a file in tsserver, close and reopen it. I assume performance of
// this implementation is worse than calculating diffs and using `change` command
// (although maybe not since this is not actually reading or writing to disk).
// However, that requires calculating diffs which is more complex and may also
// have performance implications, so sticking with the simple approach for now.
//
// TODO: In either case, we should be aggressively debouncing these updates.
//
tsserver.close({ file });
tsserver.open({ file, fileContent: cell.source });
tsserver.close({ file: oldFilePath });
tsserver.open({ file: newFilePath, fileContent: cellAfterUpdate.source });

sendTypeScriptDiagnostics(tsserver, session, cell);
// Send all diagnostics so that other cells that import this updated cell
// are informed about any updates (changes to exports, file renames).
//
// TODO: Can we be smarter and only do this for cells that import
// this cell, rather than all cells?
sendAllTypeScriptDiagnostics(tsserver, session);
}
}

Expand Down
75 changes: 34 additions & 41 deletions packages/web/src/components/cells/code.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -52,13 +52,8 @@ export default function CodeCell(props: {
return () => channel.off('cell:error', callback);
}, [cell.id, channel]);

async function updateFilename(filename: string) {
setError(null);
channel.push('cell:update', {
cellId: cell.id,
sessionId: session.id,
updates: { filename },
});
function updateFilename(filename: string) {
onUpdateCell(cell, { filename });
}

function runCell() {
Expand Down Expand Up @@ -102,7 +97,7 @@ export default function CodeCell(props: {
filename={cell.filename}
onUpdate={updateFilename}
onChange={() => setError(null)}
className="group-hover:border-input"
className="font-mono font-semibold text-xs border-transparent hover:border-input transition-colors group-hover:border-input"
/>
{error && <div className="text-red-600 text-sm">{error}</div>}
</div>
Expand Down Expand Up @@ -176,50 +171,48 @@ function CodeEditor({
function FilenameInput(props: {
filename: string;
className: string;
onUpdate: (filename: string) => Promise<void>;
onUpdate: (filename: string) => void;
onChange: (e: React.ChangeEvent<HTMLInputElement>) => void;
}) {
const filename = props.filename;
const onUpdate = props.onUpdate;
const onChange = props.onChange;

const inputRef = useRef<HTMLInputElement | null>(null);

const [filename, setFilename] = useState(props.filename);

useEffect(() => {
if (filename !== props.filename) {
setFilename(props.filename);
}

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [props.filename]);

function submit() {
if (props.filename !== filename) {
onUpdate(filename);
}
}

function blurOnEnter(e: React.KeyboardEvent<HTMLInputElement>) {
if (e.key === 'Enter') {
inputRef.current?.blur();
}
}

return (
<Input
onFocus={(e) => {
const input = e.target;
const value = input.value;
const dotIndex = value.lastIndexOf('.');
if (dotIndex !== -1) {
input.setSelectionRange(0, dotIndex);
} else {
input.select(); // In case there's no dot, select the whole value
}
}}
ref={inputRef}
onChange={onChange}
required
defaultValue={filename}
onBlur={() => {
if (!inputRef.current) {
return;
}

const updatedFilename = inputRef.current.value;
if (updatedFilename !== filename) {
onUpdate(updatedFilename);
}
}}
onKeyDown={(e) => {
if (e.key === 'Enter' && inputRef.current) {
inputRef.current.blur();
}
ref={inputRef}
value={filename}
onBlur={submit}
onKeyDown={blurOnEnter}
onChange={(e) => {
setFilename(e.target.value);
onChange(e);
}}
className={cn(
'font-mono font-semibold text-xs border-transparent hover:border-input transition-colors',
props.className,
)}
className={props.className}
/>
);
}
Loading