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

Desktop: Fixes #11445: Link watched files to the current window #11590

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
1 change: 1 addition & 0 deletions .eslintignore
Original file line number Diff line number Diff line change
Expand Up @@ -295,6 +295,7 @@ packages/app-desktop/gui/NoteEditor/utils/useNoteSearchBar.js
packages/app-desktop/gui/NoteEditor/utils/usePluginEditorView.test.js
packages/app-desktop/gui/NoteEditor/utils/usePluginEditorView.js
packages/app-desktop/gui/NoteEditor/utils/usePluginServiceRegistration.js
packages/app-desktop/gui/NoteEditor/utils/useResourceUnwatcher.js
packages/app-desktop/gui/NoteEditor/utils/useScheduleSaveCallbacks.js
packages/app-desktop/gui/NoteEditor/utils/useScrollWhenReadyOptions.js
packages/app-desktop/gui/NoteEditor/utils/useSearchMarkers.js
Expand Down
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,7 @@ packages/app-desktop/gui/NoteEditor/utils/useNoteSearchBar.js
packages/app-desktop/gui/NoteEditor/utils/usePluginEditorView.test.js
packages/app-desktop/gui/NoteEditor/utils/usePluginEditorView.js
packages/app-desktop/gui/NoteEditor/utils/usePluginServiceRegistration.js
packages/app-desktop/gui/NoteEditor/utils/useResourceUnwatcher.js
packages/app-desktop/gui/NoteEditor/utils/useScheduleSaveCallbacks.js
packages/app-desktop/gui/NoteEditor/utils/useScrollWhenReadyOptions.js
packages/app-desktop/gui/NoteEditor/utils/useSearchMarkers.js
Expand Down
5 changes: 3 additions & 2 deletions packages/app-desktop/app.reducer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,8 @@ export interface AppWindowState extends WindowState {
visibleDialogs: VisibleDialogs;
dialogs: AppStateDialog[];
devToolsVisible: boolean;
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
watchedResources: any;
}

interface BackgroundWindowStates {
Expand All @@ -62,8 +64,6 @@ export interface AppState extends State, AppWindowState {
modalOverlayMessage: string|null;

// Extra reducer keys go here
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
watchedResources: any;
mainLayout: LayoutItem;
isResettingLayout: boolean;
}
Expand All @@ -76,6 +76,7 @@ export const createAppDefaultWindowState = (): AppWindowState => {
noteVisiblePanes: ['editor', 'viewer'],
editorCodeView: true,
devToolsVisible: false,
watchedResources: {},
};
};

Expand Down
1 change: 1 addition & 0 deletions packages/app-desktop/app.ts
Original file line number Diff line number Diff line change
Expand Up @@ -653,6 +653,7 @@ class Application extends BaseApplication {
// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
(action: any) => { this.store().dispatch(action); },
(path: string) => bridge().openItem(path),
() => this.store().getState().windowId,
);

// Forwards the local event to the global event manager, so that it can
Expand Down
5 changes: 4 additions & 1 deletion packages/app-desktop/gui/NoteEditor/NoteEditor.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ import { EditorActivationCheckFilterObject } from '@joplin/lib/services/plugins/
import PluginService from '@joplin/lib/services/plugins/PluginService';
import WebviewController from '@joplin/lib/services/plugins/WebviewController';
import AsyncActionQueue, { IntervalType } from '@joplin/lib/AsyncActionQueue';
import useResourceUnwatcher from './utils/useResourceUnwatcher';

const debounce = require('debounce');

Expand Down Expand Up @@ -358,6 +359,8 @@ function NoteEditorContent(props: NoteEditorProps) {
const windowId = useContext(WindowIdContext);
const onMessage = useMessageHandler(scrollWhenReady, clearScrollWhenReady, windowId, editorRef, setLocalSearchResultCount, props.dispatch, formNote, htmlToMarkdown, markupToHtml);

useResourceUnwatcher({ noteId: formNote.id, windowId });

// eslint-disable-next-line @typescript-eslint/no-explicit-any -- Old code before rule was applied
const externalEditWatcher_noteChange = useCallback((event: any) => {
if (event.id === formNote.id) {
Expand Down Expand Up @@ -729,7 +732,7 @@ const mapStateToProps = (state: AppState, ownProps: ConnectProps) => {
selectedSearchId: windowState.selectedSearchId,
customCss: state.customViewerCss,
noteVisiblePanes: windowState.noteVisiblePanes,
watchedResources: state.watchedResources,
watchedResources: windowState.watchedResources,
highlightedWords: state.highlightedWords,
plugins: state.pluginService.plugins,
pluginHtmlContents: state.pluginService.pluginHtmlContents,
Expand Down
21 changes: 21 additions & 0 deletions packages/app-desktop/gui/NoteEditor/utils/useResourceUnwatcher.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import ResourceEditWatcher from '@joplin/lib/services/ResourceEditWatcher';
import { useEffect } from 'react';

interface Props {
noteId: string;
windowId: string;
}

const useResourceUnwatcher = ({ noteId, windowId }: Props) => {
useEffect(() => {
// All resources associated with the current window should no longer be watched after:
// 1. The editor unloads, or
// 2. The note shown in the editor changes.
// Unwatching in a cleanup callback handles both cases.
return () => {
void ResourceEditWatcher.instance().stopWatchingAll(windowId);
};
}, [noteId, windowId]);
};

export default useResourceUnwatcher;
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { RefObject, useCallback, useEffect, useRef, useState } from 'react';
import { NoteBodyEditorRef, ScrollOptions, ScrollOptionTypes } from './types';
import usePrevious from '@joplin/lib/hooks/usePrevious';
import ResourceEditWatcher from '@joplin/lib/services/ResourceEditWatcher';
import type { EditorScrollPercents } from '../../../app.reducer';

interface Props {
Expand Down Expand Up @@ -30,8 +29,6 @@ const useScrollWhenReadyOptions = ({ noteId, selectedNoteHash, lastEditorScrollP
type: selectedNoteHash ? ScrollOptionTypes.Hash : ScrollOptionTypes.Percent,
value: selectedNoteHash ? selectedNoteHash : lastScrollPercent,
});

void ResourceEditWatcher.instance().stopWatchingAll();
}, [noteId, previousNoteId, selectedNoteHash, editorRef]);

const clearScrollWhenReady = useCallback(() => {
Expand Down
44 changes: 40 additions & 4 deletions packages/lib/services/ResourceEditWatcher/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,10 +9,14 @@ import { ResourceEntity } from '../database/types';
const EventEmitter = require('events');
const chokidar = require('chokidar');

type WindowId = string;

interface WatchedItem {
resourceId: string;
title: string;
lastFileUpdatedTime: number;
lastResourceUpdatedTime: number;
watchedByWindows: WindowId[];
path: string;
asyncSaveQueue: AsyncActionQueue;
size: number;
Expand All @@ -23,6 +27,7 @@ interface WatchedItems {
}

type OpenItemFn = (path: string)=> void;
type GetWindowIdFn = ()=> string;

export default class ResourceEditWatcher {

Expand All @@ -41,6 +46,7 @@ export default class ResourceEditWatcher {
private eventEmitter_: any;
private tempDir_ = '';
private openItem_: OpenItemFn;
private getActiveWindowId_: GetWindowIdFn;

public constructor() {
this.logger_ = new Logger();
Expand All @@ -51,10 +57,11 @@ export default class ResourceEditWatcher {
}

// eslint-disable-next-line @typescript-eslint/ban-types, @typescript-eslint/no-explicit-any -- Old code before rule was applied, Old code before rule was applied
public initialize(logger: any, dispatch: Function, openItem: OpenItemFn) {
public initialize(logger: any, dispatch: Function, openItem: OpenItemFn, getWindowId: GetWindowIdFn) {
this.logger_ = logger;
this.dispatch = dispatch;
this.openItem_ = openItem;
this.getActiveWindowId_ = getWindowId;
}

public static instance() {
Expand Down Expand Up @@ -239,15 +246,18 @@ export default class ResourceEditWatcher {
}

private async watch(resourceId: string): Promise<WatchedItem> {
const sourceWindowId = this.getActiveWindowId_();
let watchedItem = this.watchedItemByResourceId(resourceId);

if (!watchedItem) {
// Immediately create and push the item to prevent race conditions

watchedItem = {
resourceId: resourceId,
title: '',
lastFileUpdatedTime: 0,
lastResourceUpdatedTime: 0,
watchedByWindows: [sourceWindowId],
asyncSaveQueue: new AsyncActionQueue(1000),
path: '',
size: -1,
Expand All @@ -261,6 +271,10 @@ export default class ResourceEditWatcher {
watchedItem.lastFileUpdatedTime = stat.mtime.getTime();
watchedItem.lastResourceUpdatedTime = resource.updated_time;
watchedItem.size = stat.size;
watchedItem.title = resource.title;
// Reset the watching window list to handle the case where the active window
// was changed while loading the resource.
watchedItem.watchedByWindows = [this.getActiveWindowId_()];

this.watchFile(editFilePath);

Expand All @@ -269,6 +283,18 @@ export default class ResourceEditWatcher {
id: resource.id,
title: resource.title,
});
} else if (!watchedItem.watchedByWindows.includes(sourceWindowId)) {
watchedItem = {
...this.watchedItems_[resourceId],
watchedByWindows: [...watchedItem.watchedByWindows, sourceWindowId],
};
this.watchedItems_[resourceId] = watchedItem;

this.dispatch({
type: 'RESOURCE_EDIT_WATCHER_SET',
id: watchedItem.resourceId,
title: watchedItem.title,
});
}

this.logger().info(`ResourceEditWatcher: Started watching ${watchedItem.path}`);
Expand Down Expand Up @@ -318,16 +344,26 @@ export default class ResourceEditWatcher {
this.logger().info(`ResourceEditWatcher: Stopped watching ${item.path}`);
}

public async stopWatchingAll() {
public async stopWatchingAll(sourceWindow: string) {
const promises = [];
for (const resourceId in this.watchedItems_) {
const item = this.watchedItems_[resourceId];
promises.push(this.stopWatching(item.resourceId));
let item = this.watchedItems_[resourceId];

if (item.watchedByWindows.includes(sourceWindow)) {
const otherWatchingWindows = item.watchedByWindows.filter(id => id !== sourceWindow);
item = { ...item, watchedByWindows: otherWatchingWindows };
this.watchedItems_[resourceId] = item;
}

if (item.watchedByWindows.length === 0) {
promises.push(this.stopWatching(item.resourceId));
}
}
await Promise.all(promises);

this.dispatch({
type: 'RESOURCE_EDIT_WATCHER_CLEAR',
windowId: sourceWindow,
});
}

Expand Down
19 changes: 17 additions & 2 deletions packages/lib/services/ResourceEditWatcher/reducer.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import produce, { Draft } from 'immer';
import { defaultWindowId, stateUtils } from '../../reducer';

export const defaultState = {
watchedResources: {},
Expand All @@ -20,14 +21,28 @@ const reducer = produce((draft: Draft<any>, action: any) => {
break;

case 'RESOURCE_EDIT_WATCHER_REMOVE':
// RESOURCE_EDIT_WATCHER_REMOVE signals that a resource is no longer being watched.
// As such, it should be removed from all windows' resource lists:
for (const windowId in draft.backgroundWindows) {
// watchedResources is per-window only on desktop:
if ('watchedResources' in draft.backgroundWindows[windowId]) {
delete draft.backgroundWindows[windowId].watchedResources[action.id];
}
}

delete draft.watchedResources[action.id];
break;

case 'RESOURCE_EDIT_WATCHER_CLEAR':
case 'RESOURCE_EDIT_WATCHER_CLEAR': {
const windowState = stateUtils.windowStateById(draft, action.windowId ?? defaultWindowId);

draft.watchedResources = {};
// The window may have already been closed.
const windowExists = !!windowState;
if (windowExists) {
windowState.watchedResources = {};
}
break;
}

}
} catch (error) {
Expand Down
3 changes: 2 additions & 1 deletion packages/tools/cspell/dictionary4.txt
Original file line number Diff line number Diff line change
Expand Up @@ -155,4 +155,5 @@ tablist
Edubirdie
Useviral
ldaps
Bluesky
Bluesky
unwatcher
Loading