1
0
mirror of https://github.com/laurent22/joplin.git synced 2024-12-24 10:27:10 +02:00

Revert: Desktop: Fixes #5955: Changing the currently opened note from plugins or the data API does not refresh the note content

Causes an infinite rendering loop when creating a new note
This commit is contained in:
Laurent Cozic 2023-09-25 16:16:07 +01:00
parent 1f19072f8f
commit 34c4b832ba
4 changed files with 22 additions and 75 deletions

View File

@ -76,18 +76,11 @@ function NoteEditor(props: NoteEditorProps) {
}, []); }, []);
const effectiveNoteId = useEffectiveNoteId(props); const effectiveNoteId = useEffectiveNoteId(props);
const effectiveNote = props.notes.find(n => n.id === effectiveNoteId);
const { formNote, setFormNote, isNewNote, resourceInfos } = useFormNote({ const { formNote, setFormNote, isNewNote, resourceInfos } = useFormNote({
syncStarted: props.syncStarted, syncStarted: props.syncStarted,
decryptionStarted: props.decryptionStarted, decryptionStarted: props.decryptionStarted,
noteId: effectiveNoteId, noteId: effectiveNoteId,
// The effective updated_time property of the note. It may be different
// from the last time the note was saved, if it was modified outside the
// editor (eg. via API).
dbNote: effectiveNote ? { id: effectiveNote.id, updated_time: effectiveNote.updated_time } : { id: '', updated_time: 0 },
isProvisional: props.isProvisional, isProvisional: props.isProvisional,
titleInputRef: titleInputRef, titleInputRef: titleInputRef,
editorRef: editorRef, editorRef: editorRef,
@ -127,32 +120,12 @@ function NoteEditor(props: NoteEditorProps) {
return async function() { return async function() {
const note = await formNoteToNote(formNote); const note = await formNoteToNote(formNote);
reg.logger().debug('Saving note...', note); reg.logger().debug('Saving note...', note);
const noteUpdatedTime = Date.now(); const savedNote: any = await Note.save(note);
// First we set the formNote object, then we save the note. We
// do it in that order, otherwise `useFormNote` will be rendered
// with the newly saved note and the timestamp of that note will
// be more recent that the one in the editor, which will trigger
// an update. We do not want this since we already have the
// latest changes.
//
// It also means that we manually set the timestamp, so that we
// have it before the note is saved.
setFormNote((prev: FormNote) => { setFormNote((prev: FormNote) => {
return { return { ...prev, user_updated_time: savedNote.user_updated_time, hasChanged: false };
...prev,
user_updated_time: noteUpdatedTime,
updated_time: noteUpdatedTime,
hasChanged: false,
};
}); });
const savedNote = await Note.save({
...note,
updated_time: noteUpdatedTime,
}, { autoTimestamp: false });
void ExternalEditWatcher.instance().updateNoteFile(savedNote); void ExternalEditWatcher.instance().updateNoteFile(savedNote);
props.dispatch({ props.dispatch({

View File

@ -5,7 +5,6 @@ import { MarkupLanguage } from '@joplin/renderer';
import { RenderResult, RenderResultPluginAsset } from '@joplin/renderer/MarkupToHtml'; import { RenderResult, RenderResultPluginAsset } from '@joplin/renderer/MarkupToHtml';
import { MarkupToHtmlOptions } from './useMarkupToHtml'; import { MarkupToHtmlOptions } from './useMarkupToHtml';
import { Dispatch } from 'redux'; import { Dispatch } from 'redux';
import { NoteEntity } from '@joplin/lib/services/database/types';
export interface AllAssetsOptions { export interface AllAssetsOptions {
contentMaxWidthTarget?: string; contentMaxWidthTarget?: string;
@ -21,7 +20,7 @@ export interface NoteEditorProps {
dispatch: Dispatch; dispatch: Dispatch;
selectedNoteIds: string[]; selectedNoteIds: string[];
selectedFolderId: string; selectedFolderId: string;
notes: NoteEntity[]; notes: any[];
watchedNoteFiles: string[]; watchedNoteFiles: string[];
isProvisional: boolean; isProvisional: boolean;
editorNoteStatuses: any; editorNoteStatuses: any;
@ -105,7 +104,6 @@ export interface FormNote {
markup_language: number; markup_language: number;
user_updated_time: number; user_updated_time: number;
encryption_applied: number; encryption_applied: number;
updated_time: number;
hasChanged: boolean; hasChanged: boolean;
@ -156,7 +154,6 @@ export function defaultFormNote(): FormNote {
hasChanged: false, hasChanged: false,
user_updated_time: 0, user_updated_time: 0,
encryption_applied: 0, encryption_applied: 0,
updated_time: 0,
}; };
} }

View File

@ -12,7 +12,6 @@ const defaultFormNoteProps: HookDependencies = {
editorRef: null, editorRef: null,
onBeforeLoad: ()=>{}, onBeforeLoad: ()=>{},
onAfterLoad: ()=>{}, onAfterLoad: ()=>{},
dbNote: { id: '', updated_time: 0 },
}; };
describe('useFormNote', () => { describe('useFormNote', () => {

View File

@ -7,30 +7,21 @@ import { splitHtml } from '@joplin/renderer/HtmlToHtml';
import Setting from '@joplin/lib/models/Setting'; import Setting from '@joplin/lib/models/Setting';
import usePrevious from '../../hooks/usePrevious'; import usePrevious from '../../hooks/usePrevious';
import ResourceEditWatcher from '@joplin/lib/services/ResourceEditWatcher/index'; import ResourceEditWatcher from '@joplin/lib/services/ResourceEditWatcher/index';
import { MarkupToHtml } from '@joplin/renderer';
const { MarkupToHtml } = require('@joplin/renderer');
import Note from '@joplin/lib/models/Note'; import Note from '@joplin/lib/models/Note';
import { reg } from '@joplin/lib/registry';
import ResourceFetcher from '@joplin/lib/services/ResourceFetcher'; import ResourceFetcher from '@joplin/lib/services/ResourceFetcher';
import DecryptionWorker from '@joplin/lib/services/DecryptionWorker'; import DecryptionWorker from '@joplin/lib/services/DecryptionWorker';
import { NoteEntity } from '@joplin/lib/services/database/types';
import Logger from '@joplin/utils/Logger';
const logger = Logger.create('useFormNote');
export interface OnLoadEvent { export interface OnLoadEvent {
formNote: FormNote; formNote: FormNote;
updated_time: number;
}
export interface DbNote {
id: string;
updated_time: number;
} }
export interface HookDependencies { export interface HookDependencies {
syncStarted: boolean; syncStarted: boolean;
decryptionStarted: boolean; decryptionStarted: boolean;
noteId: string; noteId: string;
dbNote: DbNote;
isProvisional: boolean; isProvisional: boolean;
titleInputRef: any; titleInputRef: any;
editorRef: any; editorRef: any;
@ -72,7 +63,7 @@ function resourceInfosChanged(a: ResourceInfos, b: ResourceInfos): boolean {
export default function useFormNote(dependencies: HookDependencies) { export default function useFormNote(dependencies: HookDependencies) {
const { const {
syncStarted, decryptionStarted, noteId, isProvisional, titleInputRef, editorRef, onBeforeLoad, onAfterLoad, dbNote, syncStarted, decryptionStarted, noteId, isProvisional, titleInputRef, editorRef, onBeforeLoad, onAfterLoad,
} = dependencies; } = dependencies;
const [formNote, setFormNote] = useState<FormNote>(defaultFormNote()); const [formNote, setFormNote] = useState<FormNote>(defaultFormNote());
@ -86,7 +77,7 @@ export default function useFormNote(dependencies: HookDependencies) {
// a new refresh. // a new refresh.
const [formNoteRefeshScheduled, setFormNoteRefreshScheduled] = useState<number>(0); const [formNoteRefeshScheduled, setFormNoteRefreshScheduled] = useState<number>(0);
async function initNoteState(n: NoteEntity) { async function initNoteState(n: any) {
let originalCss = ''; let originalCss = '';
if (n.markup_language === MarkupToHtml.MARKUP_LANGUAGE_HTML) { if (n.markup_language === MarkupToHtml.MARKUP_LANGUAGE_HTML) {
@ -94,7 +85,7 @@ export default function useFormNote(dependencies: HookDependencies) {
originalCss = splitted.css; originalCss = splitted.css;
} }
const newFormNote: FormNote = { const newFormNote = {
id: n.id, id: n.id,
title: n.title, title: n.title,
body: n.body, body: n.body,
@ -108,7 +99,6 @@ export default function useFormNote(dependencies: HookDependencies) {
hasChanged: false, hasChanged: false,
user_updated_time: n.user_updated_time, user_updated_time: n.user_updated_time,
encryption_applied: n.encryption_applied, encryption_applied: n.encryption_applied,
updated_time: n.updated_time,
}; };
// Note that for performance reason,the call to setResourceInfos should // Note that for performance reason,the call to setResourceInfos should
@ -126,7 +116,7 @@ export default function useFormNote(dependencies: HookDependencies) {
useEffect(() => { useEffect(() => {
if (formNoteRefeshScheduled <= 0) return () => {}; if (formNoteRefeshScheduled <= 0) return () => {};
logger.info('Sync has finished and note has never been changed - reloading it'); reg.logger().info('Sync has finished and note has never been changed - reloading it');
let cancelled = false; let cancelled = false;
@ -138,7 +128,7 @@ export default function useFormNote(dependencies: HookDependencies) {
// it would not have been loaded in the editor (due to note selection changing // it would not have been loaded in the editor (due to note selection changing
// on delete) // on delete)
if (!n) { if (!n) {
logger.warn('Trying to reload note that has been deleted:', noteId); reg.logger().warn('Trying to reload note that has been deleted:', noteId);
return; return;
} }
@ -160,19 +150,19 @@ export default function useFormNote(dependencies: HookDependencies) {
}, [formNoteRefeshScheduled]); }, [formNoteRefeshScheduled]);
useEffect(() => { useEffect(() => {
// Check that synchronisation has just finished - and if the note has // Check that synchronisation has just finished - and
// never been changed, we reload it. If the note has already been // if the note has never been changed, we reload it.
// changed, it's a conflict that's already been handled by the // If the note has already been changed, it's a conflict
// synchronizer. // that's already been handled by the synchronizer.
const decryptionJustEnded = prevDecryptionStarted && !decryptionStarted; const decryptionJustEnded = prevDecryptionStarted && !decryptionStarted;
const syncJustEnded = prevSyncStarted && !syncStarted; const syncJustEnded = prevSyncStarted && !syncStarted;
if (!decryptionJustEnded && !syncJustEnded) return; if (!decryptionJustEnded && !syncJustEnded) return;
if (formNote.hasChanged) return; if (formNote.hasChanged) return;
// Refresh the form note. This is kept separate from the above logic so // Refresh the form note.
// that when prevSyncStarted is changed from true to false, it doesn't // This is kept separate from the above logic so that when prevSyncStarted is changed
// cancel the note from loading. // from true to false, it doesn't cancel the note from loading.
refreshFormNote(); refreshFormNote();
}, [ }, [
prevSyncStarted, syncStarted, prevSyncStarted, syncStarted,
@ -180,18 +170,6 @@ export default function useFormNote(dependencies: HookDependencies) {
formNote.hasChanged, refreshFormNote, formNote.hasChanged, refreshFormNote,
]); ]);
useEffect(() => {
// Something's not fully initialised - we skip the check
if (!dbNote.id || !dbNote.updated_time || !formNote.updated_time) return;
// If the note in the database is more recent that the note in editor,
// it was modified outside the editor, so we refresh it.
if (dbNote.id === formNote.id && dbNote.updated_time > formNote.updated_time) {
logger.info('Note has been changed outside the editor - reloading it');
refreshFormNote();
}
}, [dbNote.id, dbNote.updated_time, formNote.updated_time, formNote.id, refreshFormNote]);
useEffect(() => { useEffect(() => {
if (!noteId) { if (!noteId) {
if (formNote.id) setFormNote(defaultFormNote()); if (formNote.id) setFormNote(defaultFormNote());
@ -202,7 +180,7 @@ export default function useFormNote(dependencies: HookDependencies) {
let cancelled = false; let cancelled = false;
logger.debug('Loading existing note', noteId); reg.logger().debug('Loading existing note', noteId);
function handleAutoFocus(noteIsTodo: boolean) { function handleAutoFocus(noteIsTodo: boolean) {
if (!isProvisional) return; if (!isProvisional) return;
@ -222,15 +200,15 @@ export default function useFormNote(dependencies: HookDependencies) {
const n = await Note.load(noteId); const n = await Note.load(noteId);
if (cancelled) return; if (cancelled) return;
if (!n) throw new Error(`Cannot find note with ID: ${noteId}`); if (!n) throw new Error(`Cannot find note with ID: ${noteId}`);
logger.debug('Loaded note:', n); reg.logger().debug('Loaded note:', n);
await onBeforeLoad({ formNote, updated_time: 0 }); await onBeforeLoad({ formNote });
const newFormNote = await initNoteState(n); const newFormNote = await initNoteState(n);
setIsNewNote(isProvisional); setIsNewNote(isProvisional);
await onAfterLoad({ formNote: newFormNote, updated_time: n.updated_time }); await onAfterLoad({ formNote: newFormNote });
handleAutoFocus(!!n.is_todo); handleAutoFocus(!!n.is_todo);
} }