Merge pull request #2953 from opral/lorissigrist/mesdk-120-v2-persist…

…ence-experiment-fswatch-project-directory-instead

MESDK - Better settings reactivity

inlang/source-code/plugins/inlang-message-format/src/plugin.ts

@@ -22,6 +22,7 @@ export const plugin: Plugin<{
 	settingsSchema: PluginSettings,
 	loadMessages: async ({ settings, nodeishFs }) => {
 		await maybeMigrateToV2({ settings, nodeishFs })
+		// TODO - Call fs.readDir to automatically add the directory to the watchlist
 
 		const result: Record<string, Message> = {}
 

inlang/source-code/sdk/src/createMessagesQuery.ts

@@ -107,7 +107,7 @@ export function createMessagesQuery({
 			nodeishFs: nodeishFs,
 			// this message is called whenever a file changes that was read earlier by this filesystem
 			// - the plugin loads messages -> reads the file messages.json -> start watching on messages.json -> updateMessages
-			updateMessages: () => {
+			onChange: () => {
 				// reload
 				loadMessagesViaPlugin(
 					fsWithWatcher,

inlang/source-code/sdk/src/createNodeishFsWithWatcher.test.ts

@@ -8,7 +8,7 @@ describe("watcher", () => {
 
 		const fs = createNodeishFsWithWatcher({
 			nodeishFs: createNodeishMemoryFs(),
-			updateMessages: () => {
+			onChange: () => {
 				counter++
 			},
 		})

inlang/source-code/sdk/src/createNodeishFsWithWatcher.ts

@@ -8,67 +8,78 @@ import type { NodeishFilesystem } from "@lix-js/fs"
  */
 export const createNodeishFsWithWatcher = (args: {
 	nodeishFs: NodeishFilesystem
-	updateMessages: () => void
+	onChange: () => void
 }): NodeishFilesystem & {
 	stopWatching: () => void
 } => {
-	const pathList: string[] = []
-	let abortControllers: AbortController[] = []
+	const pathList = new Set<string>()
+	const abortControllers = new Set<AbortController>()
 
 	const stopWatching = () => {
 		for (const ac of abortControllers) {
 			ac.abort()
+			abortControllers.delete(ac) // release reference
 		}
-		// release references
-		abortControllers = []
 	}
 
-	const makeWatcher = (path: string) => {
-		;(async () => {
-			try {
-				const ac = new AbortController()
-				abortControllers.push(ac)
-				const watcher = args.nodeishFs.watch(path, {
-					signal: ac.signal,
-					persistent: false,
-				})
-				if (watcher) {
-					//eslint-disable-next-line @typescript-eslint/no-unused-vars
-					for await (const event of watcher) {
-						args.updateMessages()
-					}
+	const makeWatcher = async (path: string) => {
+		try {
+			const ac = new AbortController()
+			abortControllers.add(ac)
+			const watcher = args.nodeishFs.watch(path, {
+				signal: ac.signal,
+				recursive: true,
+				persistent: false,
+			})
+			if (watcher) {
+				//eslint-disable-next-line @typescript-eslint/no-unused-vars
+				for await (const event of watcher) {
+					// whenever the watcher changes we need to update the messages
+					args.onChange()
 				}
-			} catch (err: any) {
-				if (err.name === "AbortError") return
-				// https://github.com/opral/monorepo/issues/1647
-				// the file does not exist (yet)
-				// this is not testable beacause the fs.watch api differs
-				// from node and lix. lenghty
-				else if (err.code === "ENOENT") return
-				throw err
 			}
-		})()
+		} catch (err: any) {
+			if (err.name === "AbortError") return
+			// https://github.com/opral/monorepo/issues/1647
+			// the file does not exist (yet)
+			// this is not testable beacause the fs.watch api differs
+			// from node and lix. lenghty
+			else if (err.code === "ENOENT") return
+			throw err
+		}
 	}
 
-	const readFileAndExtractPath = (path: string, options: { encoding: "utf-8" | "binary" }) => {
-		if (!pathList.includes(path)) {
-			makeWatcher(path)
-			pathList.push(path)
+	/**
+	 * Creates watchers on-the-fly for any file or directory that is not yet watched.
+	 *
+	 * We do this instead of recursively watching the entire project because fs.watch does not support
+	 * recursive watching on linux in node 18. Once node 18 support is dropped this can be drastically simplified.
+	 */
+	const watched = <T extends any[], R>(
+		fn: (path: string, ...rest: T) => R
+	): ((path: string, ...rest: T) => R) => {
+		return (path: string, ...rest: T): R => {
+			if (!pathList.has(path)) {
+				makeWatcher(path)
+				pathList.add(path)
+			}
+			return fn(path, ...rest)
 		}
-		return args.nodeishFs.readFile(path, options)
 	}
 
 	return {
+		...args.nodeishFs,
+		/**
+		 * Reads the file and automatically adds it to the list of watched files.
+		 * Any changes to the file will trigger a message update.
+		 */
 		// @ts-expect-error
-		readFile: (path: string, options: { encoding: "utf-8" | "binary" }) =>
-			readFileAndExtractPath(path, options),
-		rm: args.nodeishFs.rm,
-		readdir: args.nodeishFs.readdir,
-		mkdir: args.nodeishFs.mkdir,
-		rmdir: (args.nodeishFs as any).rmdir,
-		writeFile: args.nodeishFs.writeFile,
-		watch: args.nodeishFs.watch,
-		stat: args.nodeishFs.stat,
+		readFile: watched(args.nodeishFs.readFile),
+		/**
+		 * Reads the directory and automatically adds it to the list of watched files.
+		 * Any changes to the directory will trigger a message update.
+		 */
+		readdir: watched(args.nodeishFs.readdir),
 		stopWatching,
 	}
 }

inlang/source-code/sdk/src/loadProject.test.ts

@@ -355,9 +355,9 @@ describe("initialization", () => {
 		it("should not re-write the settings to disk when initializing", async () => {
 			const repo = await mockRepo()
 			const fs = repo.nodeishFs
-			const settingsWithDeifferentFormatting = JSON.stringify(settings, undefined, 4)
+			const settingsWithDifferentFormatting = JSON.stringify(settings, undefined, 4)
 			await fs.mkdir("/user/project.inlang", { recursive: true })
-			await fs.writeFile("/user/project.inlang/settings.json", settingsWithDeifferentFormatting)
+			await fs.writeFile("/user/project.inlang/settings.json", settingsWithDifferentFormatting)
 
 			const project = await loadProject({
 				projectPath: "/user/project.inlang",
@@ -368,7 +368,7 @@ describe("initialization", () => {
 			const settingsOnDisk = await fs.readFile("/user/project.inlang/settings.json", {
 				encoding: "utf-8",
 			})
-			expect(settingsOnDisk).toBe(settingsWithDeifferentFormatting)
+			expect(settingsOnDisk).toBe(settingsWithDifferentFormatting)
 
 			project.setSettings(project.settings())
 			// TODO: how can we await `setsettings` correctly
@@ -377,7 +377,7 @@ describe("initialization", () => {
 			const newsettingsOnDisk = await fs.readFile("/user/project.inlang/settings.json", {
 				encoding: "utf-8",
 			})
-			expect(newsettingsOnDisk).not.toBe(settingsWithDeifferentFormatting)
+			expect(newsettingsOnDisk).not.toBe(settingsWithDifferentFormatting)
 		})
 	})
 

inlang/source-code/sdk/src/loadProject.ts

@@ -21,6 +21,7 @@ import { ProjectSettings, type NodeishFilesystemSubset } from "./versionedInterf
 import { tryCatch, type Result } from "@inlang/result"
 import { migrateIfOutdated } from "@inlang/project-settings/migration"
 import { createNodeishFsWithAbsolutePaths } from "./createNodeishFsWithAbsolutePaths.js"
+import { createNodeishFsWithWatcher } from "./createNodeishFsWithWatcher.js"
 import { normalizePath } from "@lix-js/fs"
 import { assertValidProjectPath } from "./validateProjectPath.js"
 
@@ -90,43 +91,27 @@ export async function loadProject(args: {
 
 		const [initialized, markInitAsComplete, markInitAsFailed] = createAwaitable()
 		const [loadedSettings, markSettingsAsLoaded, markSettingsAsFailed] = createAwaitable()
+
+		const [resolvedModules, setResolvedModules] =
+			createSignal<Awaited<ReturnType<typeof resolveModules>>>()
 		// -- settings ------------------------------------------------------------
 
 		const [settings, _setSettings] = createSignal<ProjectSettings>()
 		let v2Persistence = false
 		let locales: string[] = []
 
-		// This effect currently has no signals
-		// TODO: replace createEffect with await loadSettings
-		// https://github.com/opral/inlang-message-sdk/issues/77
-		createEffect(() => {
-			// TODO:
-			// if (projectId) {
-			// 	telemetryBrowser.group("project", projectId, {
-			// 		name: projectId,
-			// 	})
-			// }
-
-			loadSettings({ settingsFilePath: projectPath + "/settings.json", nodeishFs })
-				.then((settings) => {
-					setSettings(settings)
-					markSettingsAsLoaded()
-				})
-				.catch((err) => {
-					markInitAsFailed(err)
-					markSettingsAsFailed(err)
-				})
-		})
-		// TODO: create FS watcher and update settings on change
-		// https://github.com/opral/inlang-message-sdk/issues/35
-
-		const writeSettingsToDisk = skipFirst((settings: ProjectSettings) =>
-			_writeSettingsToDisk({ nodeishFs, settings, projectPath })
-		)
+		// TODO:
+		// if (projectId) {
+		// 	telemetryBrowser.group("project", projectId, {
+		// 		name: projectId,
+		// 	})
+		// }
 
-		const setSettings = (settings: ProjectSettings): Result<void, ProjectSettingsInvalidError> => {
+		const setSettings = (
+			newSettings: ProjectSettings
+		): Result<ProjectSettings, ProjectSettingsInvalidError> => {
 			try {
-				const validatedSettings = parseSettings(settings)
+				const validatedSettings = parseSettings(newSettings)
 				v2Persistence = !!validatedSettings.experimental?.persistence
 				locales = validatedSettings.languageTags
 
@@ -136,23 +121,56 @@ export async function loadProject(args: {
 					_setSettings(validatedSettings)
 				})
 
-				writeSettingsToDisk(validatedSettings)
-				return { data: undefined }
+				return { data: validatedSettings }
 			} catch (error: unknown) {
 				if (error instanceof ProjectSettingsInvalidError) {
 					return { error }
 				}
 
 				throw new Error(
-					"Unhandled error in setSettings. This is an internal bug. Please file an issue."
+					"Unhandled error in setSettings. This is an internal bug. Please file an issue.",
+					{ cause: error }
 				)
 			}
 		}
 
-		// -- resolvedModules -----------------------------------------------------------
+		const nodeishFsWithWatchersForSettings = createNodeishFsWithWatcher({
+			nodeishFs: nodeishFs,
+			onChange: async () => {
+				const readSettingsResult = await tryCatch(
+					async () =>
+						await loadSettings({
+							settingsFilePath: projectPath + "/settings.json",
+							nodeishFs: nodeishFs,
+						})
+				)
 
-		const [resolvedModules, setResolvedModules] =
-			createSignal<Awaited<ReturnType<typeof resolveModules>>>()
+				if (readSettingsResult.error) return
+				const newSettings = readSettingsResult.data
+
+				if (JSON.stringify(newSettings) !== JSON.stringify(settings())) {
+					setSettings(newSettings)
+				}
+			},
+		})
+
+		const settingsResult = await tryCatch(
+			async () =>
+				await loadSettings({
+					settingsFilePath: projectPath + "/settings.json",
+					nodeishFs: nodeishFsWithWatchersForSettings,
+				})
+		)
+
+		if (settingsResult.error) {
+			markInitAsFailed(settingsResult.error)
+			markSettingsAsFailed(settingsResult.error)
+		} else {
+			setSettings(settingsResult.data)
+			markSettingsAsLoaded()
+		}
+
+		// -- resolvedModules -----------------------------------------------------------
 
 		createEffect(() => {
 			const _settings = settings()
@@ -318,7 +336,11 @@ export async function loadProject(args: {
 				//...(lintErrors() ?? []),
 			]),
 			settings: createSubscribable(() => settings() as ProjectSettings),
-			setSettings,
+			setSettings: (newSettings: ProjectSettings): Result<void, ProjectSettingsInvalidError> => {
+				const result = setSettings(newSettings)
+				if (!result.error) writeSettingsToDisk({ nodeishFs, settings: result.data, projectPath })
+				return result.error ? result : { data: undefined }
+			},
 			customApi: createSubscribable(() => resolvedModules()?.resolvedPluginApi.customApi || {}),
 			query: {
 				messages: messagesQuery,
@@ -355,6 +377,9 @@ const loadSettings = async (args: {
 	return parseSettings(json.data)
 }
 
+/**
+ * @throws If the settings are not valid
+ */
 const parseSettings = (settings: unknown) => {
 	const withMigration = migrateIfOutdated(settings as any)
 	if (settingsCompiler.Check(withMigration) === false) {
@@ -386,26 +411,24 @@ const parseSettings = (settings: unknown) => {
 	return withMigration
 }
 
-const _writeSettingsToDisk = async (args: {
+const writeSettingsToDisk = async (args: {
 	projectPath: string
 	nodeishFs: NodeishFilesystemSubset
 	settings: ProjectSettings
 }) => {
-	const { data: serializedSettings, error: serializeSettingsError } = tryCatch(() =>
+	const serializeResult = tryCatch(() =>
 		// TODO: this will probably not match the original formatting
 		JSON.stringify(args.settings, undefined, 2)
 	)
-	if (serializeSettingsError) {
-		throw serializeSettingsError
-	}
+	if (serializeResult.error) throw serializeResult.error
+	const serializedSettings = serializeResult.data
 
-	const { error: writeSettingsError } = await tryCatch(async () =>
-		args.nodeishFs.writeFile(args.projectPath + "/settings.json", serializedSettings)
+	const writeResult = await tryCatch(
+		async () =>
+			await args.nodeishFs.writeFile(args.projectPath + "/settings.json", serializedSettings)
 	)
 
-	if (writeSettingsError) {
-		throw writeSettingsError
-	}
+	if (writeResult.error) throw writeResult.error
 }
 
 // ------------------------------------------------------------------------------------------------
@@ -426,18 +449,6 @@ const createAwaitable = () => {
 	]
 }
 
-// Skip initial call, eg. to skip setup of a createEffect
-function skipFirst(func: (args: any) => any) {
-	let initial = false
-	return function (...args: any) {
-		if (initial) {
-			// @ts-ignore
-			return func.apply(this, args)
-		}
-		initial = true
-	}
-}
-
 export function createSubscribable<T>(signal: () => T): Subscribable<T> {
 	return Object.assign(signal, {
 		subscribe: (callback: (value: T) => void) => {