Merge pull request #32 from lauslim12/31-fix-async-operations-bug

Fix Asynchronous Operations in Button Event Handlers

app/Editor.tsx

@@ -66,16 +66,12 @@ const SharedNote = ({ title, content }: SharedNoteProps) => {
 };
 
 const NoteEditorRoot = () => {
-  const [initialValue, setInitialValue] = useState<Data | null>(null);
   const storage = useStorage();
 
-  useEffect(() => {
-    setInitialValue(storage.getData());
-  }, [storage]);
-
-  if (initialValue === null) {
-    return null;
-  }
+  // Lazily initialize the initial value, this `useState` is identical
+  // to `useRef` and will not cause any subsequent re-renders.
+  // Ref: https://kentcdodds.com/blog/use-state-lazy-initialization-and-function-updates
+  const [initialValue] = useState(() => storage.getData());
 
   return <NoteEditor storage={storage} initialValue={initialValue} />;
 };
@@ -107,15 +103,15 @@ const NoteEditor = ({ storage, initialValue }: NoteEditorProps) => {
   const [lastChanges, setLastChanges] = useState('');
 
   // Special function to write to the data store.
-  const writeToStorage = () => {
-    storage.setData(state);
+  const writeToStorage = (updatedData: Data) => {
+    storage.setData(updatedData);
     setSave('saved');
   };
 
   // Special React optimized debounce which will write to the
   // `localStorage` once an interval has passed. This is to create
   // an 'autosave-like' behavior.
-  const debouncedSave = useDebounce(() => writeToStorage());
+  const debouncedSave = useDebounce(() => writeToStorage(state));
 
   const handleChangeTitle = (e: ChangeEvent<HTMLTextAreaElement>) => {
     setTitle(e.currentTarget.value, Date.now().toString());
@@ -133,13 +129,21 @@ const NoteEditor = ({ storage, initialValue }: NoteEditorProps) => {
     // Cancel pending debounce, set relevant state, then store immediately.
     debouncedSave.cancel();
     setLastChanges(state.notes.content);
-    setContent('', Date.now().toString());
-    writeToStorage();
+
+    // Rather than using `useEffect` and worrying about potential side effects to subscribe
+    // to the `localStorage`, it's better to explicitly pass in the new values to the function
+    // to write the data. We cannot just rely on `state` because this is so fast that the
+    // `state` hasn't yet finished updating and we have already written the data to the storage.
+    const lastUpdated = Date.now().toString();
+    setContent('', lastUpdated);
+    writeToStorage({
+      notes: { ...state.notes, content: '', lastUpdated },
+    });
   };
 
-  const handleFreezeNote = () => {
-    setFrozen(!state.notes.frozen);
-    writeToStorage();
+  const handleFreezeNote = (nextValue: boolean) => () => {
+    setFrozen(nextValue);
+    writeToStorage({ notes: { ...state.notes, frozen: nextValue } });
   };
 
   const handleShareNote = () => {
@@ -168,8 +172,12 @@ const NoteEditor = ({ storage, initialValue }: NoteEditorProps) => {
     // Cancel pending debounce, set the relevant state, then store immediately.
     debouncedSave.cancel();
     setLastChanges('');
-    setContent(lastChanges, Date.now().toString());
-    writeToStorage();
+
+    const lastUpdated = Date.now().toString();
+    setContent(lastChanges, lastUpdated);
+    writeToStorage({
+      notes: { ...state.notes, content: lastChanges, lastUpdated },
+    });
   };
 
   useEffect(() => {
@@ -230,7 +238,7 @@ const NoteEditor = ({ storage, initialValue }: NoteEditorProps) => {
           Clear content
         </Button>
 
-        <Button onClick={handleFreezeNote}>
+        <Button onClick={handleFreezeNote(!state.notes.frozen)}>
           {state.notes.frozen ? 'Unfreeze note' : 'Freeze note'}
         </Button>
 

app/schema.ts

@@ -54,15 +54,27 @@ const stringAsCompatibleBoolean = () => {
       return false;
     }
 
-    if (x !== 'false' && x !== 'true') {
-      return false;
+    // If it's a boolean then return as is, if it's a 'stringified'
+    // boolean, such as `false` or `true`, then return the appropriate values.
+    // If no match, return `false` as the default boolean value.
+    if (typeof x === 'boolean') {
+      return x;
     }
 
-    if (x === 'false') {
-      return false;
+    if (typeof x === 'string') {
+      switch (x) {
+        case 'true':
+          return true;
+
+        case 'false':
+          return false;
+
+        default:
+          return false;
+      }
     }
 
-    return true;
+    return false;
   }, z.boolean());
 };
 

e2e/Speednote.test.tsx

@@ -132,6 +132,13 @@ test('able to clear content and undo clear', async ({ page }) => {
   await clearContentButton.click();
   await expect(content).toHaveValue('');
 
+  // Verify that the data is already stored in the `localStorage`. This is
+  // an implementation detail, but it's better to be safe: https://github.com/lauslim12/speednote/issues/31.
+  const clearedValue = await page.evaluate(() =>
+    localStorage.getItem('speednote')
+  );
+  expect(clearedValue).toContain('"content":""');
+
   // Query the `undoClearButton` again here.
   const undoClearButton = page.getByRole('button', { name: 'Undo clear' });
   await expect(undoClearButton).toBeVisible();
@@ -140,6 +147,14 @@ test('able to clear content and undo clear', async ({ page }) => {
   await expect(content).toHaveValue(
     "Tears Don't Fall, Enchanted, Beautiful Trauma"
   );
+
+  // Verify the data is already stored in the `localStorage`.
+  const restoredValue = await page.evaluate(() =>
+    localStorage.getItem('speednote')
+  );
+  expect(restoredValue).toContain(
+    `"content":"Tears Don't Fall, Enchanted, Beautiful Trauma"`
+  );
 });
 
 test('able to switch color mode', async ({ page }) => {
@@ -169,6 +184,13 @@ test('able to freeze notes and unfreeze them', async ({ page }) => {
   await expect(title).not.toBeEditable();
   await expect(content).not.toBeEditable();
 
+  // Verify that the data is already stored in the `localStorage`. This is
+  // an implementation detail, but it's better to be safe: https://github.com/lauslim12/speednote/issues/31.
+  const frozenValue = await page.evaluate(() =>
+    localStorage.getItem('speednote')
+  );
+  expect(frozenValue).toContain('"frozen":true');
+
   // Try to type, but it also shouldn't be possible. That's why we try to use `force`.
   await title.fill('Hello', { force: true });
   await expect(title).toHaveValue('');
@@ -187,6 +209,12 @@ test('able to freeze notes and unfreeze them', async ({ page }) => {
   await freezeNoteButton.click();
   await expect(freezeNoteButton).toHaveText('Freeze note');
 
+  // Verify that the data is already stored in the `localStorage`.
+  const unfrozenValue = await page.evaluate(() =>
+    localStorage.getItem('speednote')
+  );
+  expect(unfrozenValue).toContain('"frozen":false');
+
   // `Clear content` should not be disabled.
   await expect(clearContentButton).toBeEnabled();