Skip to content

fix: switching between notes may override note #336

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

Open
redeck1 wants to merge 2 commits into
base: main
Choose a base branch
from
Open

fix: switching between notes may override note #336

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
50a118c
fix: When quickly switch between notes, the url and id of the note ma…
redeck1 Jan 7, 2026
fe9fb69
Update useNote.ts
redeck1 Jan 13, 2026
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
51 changes: 35 additions & 16 deletions src/application/services/useNote.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -201,13 +201,23 @@ export default function (options: UseNoteComposableOptions): UseNoteComposableSt
try {
const response = await noteService.getNoteById(id);

/**
* Id changed, loaded another note
Copy link
Member

@neSpecc neSpecc Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

please, explain the case more straightforward

*/
if (currentId.value !== id) {
return;
}

note.value = response.note;
canEdit.value = response.accessRights.canEdit;
noteTools.value = response.tools;
parentNote.value = response.parentNote;
noteParents.value = response.parents;
void getNoteHierarchy(id);
} catch (error) {
if (currentId.value !== id) {
return;
}
Comment on lines +218 to +220
Copy link
Collaborator

@e11sy e11sy Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't understand why we need to check this condition one more time here (you've checked it right above, what is the difference here?)

deleteOpenedPageByUrl(route.path);
if (error instanceof DomainError) {
void router.push(`/error/${error.statusCode}`);
Expand Down Expand Up @@ -255,9 +265,14 @@ export default function (options: UseNoteComposableOptions): UseNoteComposableSt
*/
const specifiedNoteTools = resolveToolsByContent(content);

/**
* Id may be changed
Copy link
Member

@neSpecc neSpecc Feb 7, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

JSdoc is not clear for me, please explain the case.

*/
const savedNoteId = currentId.value;

isNoteSaving.value = true;

if (currentId.value === null) {
if (savedNoteId === null) {
/**
* @todo try-catch domain errors
*/
Expand All @@ -273,25 +288,26 @@ export default function (options: UseNoteComposableOptions): UseNoteComposableSt
},
});

patchOpenedPageByUrl(
route.path,
{
title: noteTitle.value,
url: route.path,
});
patchOpenedPageByUrl(route.path, {
title: noteTitle.value,
url: route.path,
});

/**
* Get note Hierarchy when new Note is created
*/
void getNoteHierarchy(noteCreated.id);
} else {
await noteService.updateNoteContentAndTools(currentId.value, content, specifiedNoteTools);
await noteService.updateNoteContentAndTools(savedNoteId, content, specifiedNoteTools);
}

/**
* Store just saved content in memory
* If id changed, do not store content
*/
lastUpdateContent.value = content;
if (currentId.value === savedNoteId) {
lastUpdateContent.value = content;
}

isNoteSaving.value = false;
}
Expand Down Expand Up @@ -391,13 +407,16 @@ export default function (options: UseNoteComposableOptions): UseNoteComposableSt
});

watch(noteTitle, (currentNoteTitle) => {
if (route.name == 'note') {
patchOpenedPageByUrl(
route.path,
{
title: currentNoteTitle,
url: route.path,
});
if (route.name == 'note' && currentId.value !== null) {
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Use strict equality (===) instead of loose equality (==) for consistency with the rest of the codebase and to avoid potential type coercion issues.

Suggested change
if (route.name == 'note' && currentId.value !== null) {
if (route.name === 'note' && currentId.value !== null) {

Copilot uses AI. Check for mistakes.
/**
* URL may have changed, use note id
*/
const noteUrl = `/note/${currentId.value}`;

patchOpenedPageByUrl(noteUrl, {
title: currentNoteTitle,
url: noteUrl,
});
}
Comment on lines 409 to 420
Copy link

Copilot AI Jan 14, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The noteTitle watcher has a potential race condition. While currentId.value is checked, it's not captured at the time the watcher is triggered. If currentId changes between when noteTitle is recalculated and when this watcher callback runs, the title from one note could be used to update the opened page for a different note. Consider capturing currentId.value at the start of the watcher callback, similar to how savedNoteId is captured in the save function.

Copilot uses AI. Check for mistakes.
updateNoteHierarchyContent(noteHierarchy.value, currentNoteTitle);
});
Expand Down
Loading