editor: fix error if main.py does not exist

The editor saga was assuming that main.py always exists, which is not
the case for the first use of the app. This fixes the issue by checking
to see if the file exists before attempting to open it. This also
revealed a bug in fileStorage where the list of files was not populated
during initialization.
This commit is contained in:
David Lechner
2022-02-28 12:24:46 -06:00
parent 97363b4327
commit 31600c6f37
10 changed files with 153 additions and 37 deletions
+12 -1
View File
@@ -5,7 +5,7 @@ import { monaco } from 'react-monaco-editor';
import { createAction } from '../actions';
/**
* Sets the current (active) edit session.
* Requests to set the current (active) edit session.
* @param editSession The new edit session.
*/
export const setEditSession = createAction(
@@ -15,6 +15,17 @@ export const setEditSession = createAction(
}),
);
/**
* Indicates that setting the edit session has completed.
* @param editSession The new edit session.
*/
export const didSetEditSession = createAction(
(editSession: monaco.editor.ICodeEditor | undefined) => ({
type: 'editor.action.didSetEditSession',
editSession,
}),
);
/**
* Creates an action to save the current file
*/
+12 -4
View File
@@ -1,9 +1,9 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2021 The Pybricks Authors
// Copyright (c) 2021-2022 The Pybricks Authors
import { monaco } from 'react-monaco-editor';
import { AnyAction } from 'redux';
import { setEditSession } from './actions';
import { didSetEditSession, setEditSession } from './actions';
import reducers from './reducers';
type State = ReturnType<typeof reducers>;
@@ -18,10 +18,18 @@ test('initial state', () => {
test('current', () => {
const session = {} as monaco.editor.ICodeEditor;
// setEditSession doesn't change the state
expect(reducers({ current: null } as State, setEditSession(session)).current).toBe(
session,
null,
);
// only didSetEditSession changes the state
expect(
reducers({ current: session } as State, setEditSession(undefined)).current,
reducers({ current: null } as State, didSetEditSession(session)).current,
).toBe(session);
expect(
reducers({ current: session } as State, didSetEditSession(undefined)).current,
).toBe(null);
});
+2 -2
View File
@@ -3,10 +3,10 @@
import { monaco } from 'react-monaco-editor';
import { Reducer, combineReducers } from 'redux';
import { setEditSession } from './actions';
import { didSetEditSession } from './actions';
const current: Reducer<monaco.editor.ICodeEditor | null> = (state = null, action) => {
if (setEditSession.matches(action)) {
if (didSetEditSession.matches(action)) {
return action.editSession || null;
}
+65 -2
View File
@@ -1,11 +1,23 @@
// SPDX-License-Identifier: MIT
// Copyright (c) 2020-2021 The Pybricks Authors
// Copyright (c) 2020-2022 The Pybricks Authors
import FileSaver from 'file-saver';
import { mock } from 'jest-mock-extended';
import { monaco } from 'react-monaco-editor';
import { AsyncSaga } from '../../test';
import { didFailToSaveAs, didSaveAs, open, saveAs } from './actions';
import {
fileStorageDidInitialize,
fileStorageDidReadFile,
fileStorageReadFile,
} from '../fileStorage/actions';
import {
didFailToSaveAs,
didSaveAs,
didSetEditSession,
open,
saveAs,
setEditSession,
} from './actions';
import editor from './sagas';
jest.mock('react-monaco-editor');
@@ -123,3 +135,54 @@ describe('saveAs', () => {
mockFileSaverSaveAs.mockRestore();
});
});
describe('setEditSession', () => {
it('should wait for storage to be initialized', async () => {
const mockEditor = mock<monaco.editor.ICodeEditor>();
const saga = new AsyncSaga(editor, {
fileStorage: { isInitialized: false, fileNames: new Set() },
});
saga.put(setEditSession(mockEditor));
saga.put(fileStorageDidInitialize([]));
const action = await saga.take();
expect(action).toEqual(didSetEditSession(mockEditor));
await saga.end();
});
it('should load main.py', async () => {
const mockEditor = mock<monaco.editor.ICodeEditor>();
const saga = new AsyncSaga(editor, {
fileStorage: { isInitialized: true, fileNames: new Set(['main.py']) },
});
saga.put(setEditSession(mockEditor));
const action = await saga.take();
expect(action).toEqual(fileStorageReadFile('main.py'));
saga.put(fileStorageDidReadFile('main.py', '# test file'));
const action2 = await saga.take();
expect(action2).toEqual(didSetEditSession(mockEditor));
expect(mockEditor.setValue).toHaveBeenCalled();
await saga.end();
});
it('should not raise error if main.py does not exist', async () => {
const mockEditor = mock<monaco.editor.ICodeEditor>();
const saga = new AsyncSaga(editor, {
fileStorage: { isInitialized: true, fileNames: new Set() },
});
saga.put(setEditSession(mockEditor));
const action = await saga.take();
expect(action).toEqual(didSetEditSession(mockEditor));
await saga.end();
});
});
+23 -4
View File
@@ -19,7 +19,14 @@ import {
} from '../fileStorage/actions';
import { RootState } from '../reducers';
import { ensureError } from '../utils';
import { didFailToSaveAs, didSaveAs, open, saveAs, setEditSession } from './actions';
import {
didFailToSaveAs,
didSaveAs,
didSetEditSession,
open,
saveAs,
setEditSession,
} from './actions';
const decoder = new TextDecoder();
@@ -86,8 +93,10 @@ function* handleSaveAs(): Generator {
}
function* handleSetEditSession(action: ReturnType<typeof setEditSession>): Generator {
if (action.editSession === null) {
// there is not current edit session, nothing to do
if (action.editSession === undefined) {
// REVISIT: this should probably do something, but currently we don't
// expect this to happen
yield* put(didSetEditSession(action.editSession));
return;
}
@@ -104,6 +113,14 @@ function* handleSetEditSession(action: ReturnType<typeof setEditSession>): Gener
// TODO: get current file from state
const currentFileName = 'main.py';
const fileList = yield* select((s: RootState) => s.fileStorage.fileNames);
if (!fileList.has(currentFileName)) {
// The file doesn't exist in storage, so don't try to open it.
yield* put(didSetEditSession(action.editSession));
return;
}
// TODO: implement locking to ensure exclusive access to file
yield* put(fileStorageReadFile(currentFileName));
@@ -117,8 +134,10 @@ function* handleSetEditSession(action: ReturnType<typeof setEditSession>): Gener
});
if (result) {
action.editSession?.setValue(result.fileContents);
action.editSession.setValue(result.fileContents);
}
yield* put(didSetEditSession(action.editSession));
}
export default function* (): Generator {
+6 -2
View File
@@ -3,9 +3,13 @@
import { createAction } from '../actions';
/** Action that indicates that the storage backend is ready to use. */
export const fileStorageDidInitialize = createAction(() => ({
/**
* Action that indicates that the storage backend is ready to use.
* @param fileNames List of all files currently in storage.
*/
export const fileStorageDidInitialize = createAction((fileNames: string[]) => ({
type: 'fileStorage.action.didInitialize',
fileNames,
}));
/** Action that indicates that the storage backend failed to initialize. */
+9 -1
View File
@@ -22,7 +22,7 @@ test('initial state', () => {
test('isInitialized', () => {
expect(
reducers({ isInitialized: false } as State, fileStorageDidInitialize())
reducers({ isInitialized: false } as State, fileStorageDidInitialize([]))
.isInitialized,
).toBeTruthy();
});
@@ -30,6 +30,14 @@ test('isInitialized', () => {
test('fileNames', () => {
const testFileName = 'test.file';
// initialization populates file list
expect(
reducers(
{ fileNames: new Set() } as State,
fileStorageDidInitialize([testFileName]),
).fileNames,
).toEqual(new Set([testFileName]));
// if item is not in set, add it
expect(
reducers(
+4
View File
@@ -17,6 +17,10 @@ const isInitialized: Reducer<boolean> = (state = false, action) => {
};
const fileNames: Reducer<Set<string>> = (state = new Set(), action) => {
if (fileStorageDidInitialize.matches(action)) {
return new Set(action.fileNames);
}
if (fileStorageDidChangeItem.matches(action)) {
return new Set([...state, action.fileName]);
}
+6 -9
View File
@@ -29,15 +29,12 @@ it('should migrate old program from local storage during initialization', async
const saga = new AsyncSaga(fileStorage);
// initialization should remove the localStorage entry
let action = await saga.take();
expect(action).toEqual(fileStorageDidInitialize());
// initialization should remove the localStorage entry and add add it to
// new storage backend
const action = await saga.take();
expect(action).toEqual(fileStorageDidInitialize(['main.py']));
expect(localStorage.getItem(oldProgramKey)).toBeNull();
// and add it to the new storage backend
action = await saga.take();
expect(action).toEqual(fileStorageDidChangeItem('main.py'));
await saga.end();
});
@@ -46,7 +43,7 @@ it('should read and write files', async () => {
let action = await saga.take();
expect(action).toEqual(fileStorageDidInitialize());
expect(action).toEqual(fileStorageDidInitialize([]));
const testFileName = 'test.file';
const testFileContents = 'test file contents';
@@ -75,7 +72,7 @@ it('should dispatch fail action if file does not exist', async () => {
const saga = new AsyncSaga(fileStorage);
let action = await saga.take();
expect(action).toEqual(fileStorageDidInitialize());
expect(action).toEqual(fileStorageDidInitialize([]));
const testFileName = 'test.file';
+14 -12
View File
@@ -96,13 +96,23 @@ function* initialize(): Generator {
yield* call(() => files.ready());
// migrate from old storage
// Previous versions of pybricks code used local storage to save a single program.
const oldProgram = localStorage.getItem('program');
if (oldProgram !== null) {
yield* call(() => files.setItem('main.py', oldProgram));
localStorage.removeItem('program');
}
// wire storage observable to redux-sagas
files.configObservables({
crossTabNotification: true,
crossTabChangeDetection: true,
});
// wire storage observable to redux-sagas
const localForageChannel = eventChannel<LocalForageObservableChange>((emit) => {
const filesObservable = files.newObservable({
crossTabNotification: true,
@@ -121,17 +131,9 @@ function* initialize(): Generator {
yield* takeEvery(fileStorageReadFile, handleReadFile, files);
yield* takeEvery(fileStorageWriteFile, handleWriteFile, files);
// migrate from old storage
const fileNames = yield* call(() => files.keys());
// Previous versions of pybricks code used local storage to save a single program.
const oldProgram = localStorage.getItem('program');
if (oldProgram !== null) {
yield* call(() => files.setItem('main.py', oldProgram));
localStorage.removeItem('program');
}
yield* put(fileStorageDidInitialize());
yield* put(fileStorageDidInitialize(fileNames));
} catch (err) {
yield* put(fileStorageDidFailToInitialize(ensureError(err)));
}